Skip to content

Commit

Permalink
Merge pull request #1882 from kuzudb/play
Browse files Browse the repository at this point in the history 
Enable read after update
  • Loading branch information
@andyfengHKU
andyfengHKU committed Aug 2, 2023
2 parents 48f4052 + 541b7ff commit 0317cef
Show file tree
Hide file tree
Showing 10 changed files with 218 additions and 70 deletions.
1 change: 0 additions & 1 deletion src/binder/bind/bind_query.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ std::unique_ptr<BoundRegularQuery> Binder::bindQuery(const RegularQuery& regular
for (auto& boundSingleQuery : boundSingleQueries) {
auto normalizedSingleQuery = QueryNormalizer::normalizeQuery(*boundSingleQuery);
validateReadNotFollowUpdate(*normalizedSingleQuery);
validateReturnNotFollowUpdate(*normalizedSingleQuery);
boundRegularQuery->addSingleQuery(std::move(normalizedSingleQuery));
}
validateIsAllUnionOrUnionAll(*boundRegularQuery);
Expand Down
12 changes: 2 additions & 10 deletions src/binder/binder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,21 +144,13 @@ void Binder::validateIsAllUnionOrUnionAll(const BoundRegularQuery& regularQuery)
}
}

void Binder::validateReturnNotFollowUpdate(const NormalizedSingleQuery& singleQuery) {
for (auto i = 0u; i < singleQuery.getNumQueryParts(); ++i) {
auto normalizedQueryPart = singleQuery.getQueryPart(i);
if (normalizedQueryPart->hasUpdatingClause() && normalizedQueryPart->hasProjectionBody()) {
throw BinderException("Return/With after update is not supported.");
}
}
}

void Binder::validateReadNotFollowUpdate(const NormalizedSingleQuery& singleQuery) {
bool hasSeenUpdateClause = false;
for (auto i = 0u; i < singleQuery.getNumQueryParts(); ++i) {
auto normalizedQueryPart = singleQuery.getQueryPart(i);
if (hasSeenUpdateClause && normalizedQueryPart->hasReadingClause()) {
throw BinderException("Read after update is not supported.");
throw BinderException(
"Read after update is not supported. Try query with multiple statements.");
}
hasSeenUpdateClause |= normalizedQueryPart->hasUpdatingClause();
}
Expand Down
7 changes: 2 additions & 5 deletions src/include/binder/binder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,8 @@ class Binder {

static void validateIsAllUnionOrUnionAll(const BoundRegularQuery& regularQuery);

// We don't support read (reading and projection clause) after write since this requires reading
// updated value and multiple property scan is needed which complicates our planning.
// e.g. MATCH (a:person) WHERE a.fName='A' SET a.fName='B' RETURN a.fName
// In the example above, we need to read fName both before and after SET.
static void validateReturnNotFollowUpdate(const NormalizedSingleQuery& singleQuery);
// We don't support read after write for simplicity. User should instead querying through
// multiple statement.
static void validateReadNotFollowUpdate(const NormalizedSingleQuery& singleQuery);

static void validateTableExist(const catalog::Catalog& _catalog, std::string& tableName);
Expand Down
65 changes: 38 additions & 27 deletions src/include/processor/operator/update/set_executor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ namespace processor {

class NodeSetExecutor {
public:
NodeSetExecutor(
const DataPos& nodeIDPos, std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator)
: nodeIDPos{nodeIDPos}, evaluator{std::move(evaluator)} {}
NodeSetExecutor(const DataPos& nodeIDPos, const DataPos& lhsVectorPos,
std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator)
: nodeIDPos{nodeIDPos}, lhsVectorPos{lhsVectorPos}, evaluator{std::move(evaluator)} {}
virtual ~NodeSetExecutor() = default;

void init(ResultSet* resultSet, ExecutionContext* context);
Expand All @@ -24,19 +24,25 @@ class NodeSetExecutor {

protected:
DataPos nodeIDPos;
DataPos lhsVectorPos;
std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator;

common::ValueVector* nodeIDVector;
common::ValueVector* rhsVector;
common::ValueVector* nodeIDVector = nullptr;
// E.g. SET a.age = b.age; a.age is the left-hand-side (lhs) and b.age is the right-hand-side
// (rhs)
common::ValueVector* lhsVector = nullptr;
common::ValueVector* rhsVector = nullptr;
};

class SingleLabelNodeSetExecutor : public NodeSetExecutor {
public:
SingleLabelNodeSetExecutor(storage::NodeColumn* column, const DataPos& nodeIDPos,
std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator)
: NodeSetExecutor{nodeIDPos, std::move(evaluator)}, column{column} {}
const DataPos& lhsVectorPos, std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator)
: NodeSetExecutor{nodeIDPos, lhsVectorPos, std::move(evaluator)}, column{column} {}

SingleLabelNodeSetExecutor(const SingleLabelNodeSetExecutor& other)
: NodeSetExecutor{other.nodeIDPos, other.evaluator->clone()}, column{other.column} {}
: NodeSetExecutor{other.nodeIDPos, other.lhsVectorPos, other.evaluator->clone()},
column{other.column} {}

void set() final;

Expand All @@ -52,12 +58,13 @@ class MultiLabelNodeSetExecutor : public NodeSetExecutor {
public:
MultiLabelNodeSetExecutor(
std::unordered_map<common::table_id_t, storage::NodeColumn*> tableIDToColumn,
const DataPos& nodeIDPos, std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator)
: NodeSetExecutor{nodeIDPos, std::move(evaluator)}, tableIDToColumn{
std::move(tableIDToColumn)} {}
const DataPos& nodeIDPos, const DataPos& lhsVectorPos,
std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator)
: NodeSetExecutor{nodeIDPos, lhsVectorPos, std::move(evaluator)}, tableIDToColumn{std::move(
tableIDToColumn)} {}
MultiLabelNodeSetExecutor(const MultiLabelNodeSetExecutor& other)
: NodeSetExecutor{other.nodeIDPos, other.evaluator->clone()}, tableIDToColumn{
other.tableIDToColumn} {}
: NodeSetExecutor{other.nodeIDPos, other.lhsVectorPos, other.evaluator->clone()},
tableIDToColumn{other.tableIDToColumn} {}

void set() final;

Expand All @@ -72,9 +79,10 @@ class MultiLabelNodeSetExecutor : public NodeSetExecutor {
class RelSetExecutor {
public:
RelSetExecutor(const DataPos& srcNodeIDPos, const DataPos& dstNodeIDPos,
const DataPos& relIDPos, std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator)
: srcNodeIDPos{srcNodeIDPos},
dstNodeIDPos{dstNodeIDPos}, relIDPos{relIDPos}, evaluator{std::move(evaluator)} {}
const DataPos& relIDPos, const DataPos& lhsVectorPos,
std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator)
: srcNodeIDPos{srcNodeIDPos}, dstNodeIDPos{dstNodeIDPos}, relIDPos{relIDPos},
lhsVectorPos{lhsVectorPos}, evaluator{std::move(evaluator)} {}
virtual ~RelSetExecutor() = default;

void init(ResultSet* resultSet, ExecutionContext* context);
Expand All @@ -87,23 +95,26 @@ class RelSetExecutor {
DataPos srcNodeIDPos;
DataPos dstNodeIDPos;
DataPos relIDPos;
DataPos lhsVectorPos;
std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator;

common::ValueVector* srcNodeIDVector;
common::ValueVector* dstNodeIDVector;
common::ValueVector* relIDVector;
common::ValueVector* rhsVector;
common::ValueVector* srcNodeIDVector = nullptr;
common::ValueVector* dstNodeIDVector = nullptr;
common::ValueVector* relIDVector = nullptr;
// See NodeSetExecutor for an example for lhsVector & rhsVector.
common::ValueVector* lhsVector = nullptr;
common::ValueVector* rhsVector = nullptr;
};

class SingleLabelRelSetExecutor : public RelSetExecutor {
public:
SingleLabelRelSetExecutor(storage::RelTable* table, common::property_id_t propertyID,
const DataPos& srcNodeIDPos, const DataPos& dstNodeIDPos, const DataPos& relIDPos,
std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator)
: RelSetExecutor{srcNodeIDPos, dstNodeIDPos, relIDPos, std::move(evaluator)}, table{table},
propertyID{propertyID} {}
const DataPos& lhsVectorPos, std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator)
: RelSetExecutor{srcNodeIDPos, dstNodeIDPos, relIDPos, lhsVectorPos, std::move(evaluator)},
table{table}, propertyID{propertyID} {}
SingleLabelRelSetExecutor(const SingleLabelRelSetExecutor& other)
: RelSetExecutor{other.srcNodeIDPos, other.dstNodeIDPos, other.relIDPos,
: RelSetExecutor{other.srcNodeIDPos, other.dstNodeIDPos, other.relIDPos, other.lhsVectorPos,
other.evaluator->clone()},
table{other.table}, propertyID{other.propertyID} {}

Expand All @@ -124,11 +135,11 @@ class MultiLabelRelSetExecutor : public RelSetExecutor {
std::unordered_map<common::table_id_t, std::pair<storage::RelTable*, common::property_id_t>>
tableIDToTableAndPropertyID,
const DataPos& srcNodeIDPos, const DataPos& dstNodeIDPos, const DataPos& relIDPos,
std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator)
: RelSetExecutor{srcNodeIDPos, dstNodeIDPos, relIDPos, std::move(evaluator)},
const DataPos& lhsVectorPos, std::unique_ptr<evaluator::BaseExpressionEvaluator> evaluator)
: RelSetExecutor{srcNodeIDPos, dstNodeIDPos, relIDPos, lhsVectorPos, std::move(evaluator)},
tableIDToTableAndPropertyID{std::move(tableIDToTableAndPropertyID)} {}
MultiLabelRelSetExecutor(const MultiLabelRelSetExecutor& other)
: RelSetExecutor{other.srcNodeIDPos, other.dstNodeIDPos, other.relIDPos,
: RelSetExecutor{other.srcNodeIDPos, other.dstNodeIDPos, other.relIDPos, other.lhsVectorPos,
other.evaluator->clone()},
tableIDToTableAndPropertyID{other.tableIDToTableAndPropertyID} {}

Expand Down
20 changes: 14 additions & 6 deletions src/processor/mapper/map_set.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@ static std::unique_ptr<NodeSetExecutor> getNodeSetExecutor(storage::NodesStore*
const LogicalSetPropertyInfo& info, const Schema& inSchema,
std::unique_ptr<BaseExpressionEvaluator> evaluator) {
auto node = (NodeExpression*)info.nodeOrRel.get();
auto property = (PropertyExpression*)info.setItem.first.get();
auto nodeIDPos = DataPos(inSchema.getExpressionPos(*node->getInternalIDProperty()));
auto property = (PropertyExpression*)info.setItem.first.get();
auto propertyPos = DataPos(INVALID_DATA_CHUNK_POS, INVALID_VALUE_VECTOR_POS);
if (inSchema.isExpressionInScope(*property)) {
propertyPos = DataPos(inSchema.getExpressionPos(*property));
}
if (node->isMultiLabeled()) {
std::unordered_map<common::table_id_t, storage::NodeColumn*> tableIDToColumn;
for (auto tableID : node->getTableIDs()) {
Expand All @@ -27,12 +31,12 @@ static std::unique_ptr<NodeSetExecutor> getNodeSetExecutor(storage::NodesStore*
tableIDToColumn.insert({tableID, column});
}
return std::make_unique<MultiLabelNodeSetExecutor>(
std::move(tableIDToColumn), nodeIDPos, std::move(evaluator));
std::move(tableIDToColumn), nodeIDPos, propertyPos, std::move(evaluator));
} else {
auto tableID = node->getSingleTableID();
auto column = store->getNodePropertyColumn(tableID, property->getPropertyID(tableID));
return std::make_unique<SingleLabelNodeSetExecutor>(
column, nodeIDPos, std::move(evaluator));
column, nodeIDPos, propertyPos, std::move(evaluator));
}
}

Expand All @@ -54,12 +58,16 @@ static std::unique_ptr<RelSetExecutor> getRelSetExecutor(storage::RelsStore* sto
const LogicalSetPropertyInfo& info, const Schema& inSchema,
std::unique_ptr<BaseExpressionEvaluator> evaluator) {
auto rel = (RelExpression*)info.nodeOrRel.get();
auto property = (PropertyExpression*)info.setItem.first.get();
auto srcNodePos =
DataPos(inSchema.getExpressionPos(*rel->getSrcNode()->getInternalIDProperty()));
auto dstNodePos =
DataPos(inSchema.getExpressionPos(*rel->getDstNode()->getInternalIDProperty()));
auto relIDPos = DataPos(inSchema.getExpressionPos(*rel->getInternalIDProperty()));
auto property = (PropertyExpression*)info.setItem.first.get();
auto propertyPos = DataPos(INVALID_DATA_CHUNK_POS, INVALID_VALUE_VECTOR_POS);
if (inSchema.isExpressionInScope(*property)) {
propertyPos = DataPos(inSchema.getExpressionPos(*property));
}
if (rel->isMultiLabeled()) {
std::unordered_map<common::table_id_t, std::pair<storage::RelTable*, common::property_id_t>>
tableIDToTableAndPropertyID;
Expand All @@ -72,13 +80,13 @@ static std::unique_ptr<RelSetExecutor> getRelSetExecutor(storage::RelsStore* sto
tableIDToTableAndPropertyID.insert({tableID, std::make_pair(table, propertyID)});
}
return std::make_unique<MultiLabelRelSetExecutor>(std::move(tableIDToTableAndPropertyID),
srcNodePos, dstNodePos, relIDPos, std::move(evaluator));
srcNodePos, dstNodePos, relIDPos, propertyPos, std::move(evaluator));
} else {
auto tableID = rel->getSingleTableID();
auto table = store->getRelTable(tableID);
auto propertyID = property->getPropertyID(tableID);
return std::make_unique<SingleLabelRelSetExecutor>(
table, propertyID, srcNodePos, dstNodePos, relIDPos, std::move(evaluator));
table, propertyID, srcNodePos, dstNodePos, relIDPos, propertyPos, std::move(evaluator));
}
}

Expand Down
57 changes: 51 additions & 6 deletions src/processor/operator/update/set_executor.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,55 +1,94 @@
#include "processor/operator/update/set_executor.h"

using namespace kuzu::common;

namespace kuzu {
namespace processor {

void NodeSetExecutor::init(ResultSet* resultSet, ExecutionContext* context) {
nodeIDVector = resultSet->getValueVector(nodeIDPos).get();
if (lhsVectorPos.dataChunkPos != INVALID_DATA_CHUNK_POS) {
lhsVector = resultSet->getValueVector(lhsVectorPos).get();
}
evaluator->init(*resultSet, context->memoryManager);
rhsVector = evaluator->resultVector.get();
}

static void writeToPropertyVector(ValueVector* propertyVector, uint32_t propertyVectorPos,
ValueVector* rhsVector, uint32_t rhsVectorPos) {
if (rhsVector->isNull(rhsVectorPos)) {
propertyVector->setNull(propertyVectorPos, true);
} else {
propertyVector->setNull(propertyVectorPos, false);
propertyVector->copyFromVectorData(propertyVectorPos, rhsVector, rhsVectorPos);
}
}

void SingleLabelNodeSetExecutor::set() {
evaluator->evaluate();
for (auto i = 0u; i < nodeIDVector->state->selVector->selectedSize; ++i) {
auto nodeIDPos = nodeIDVector->state->selVector->selectedPositions[i];
auto& nodeID = nodeIDVector->getValue<common::internalID_t>(nodeIDPos);
auto rhsPos = nodeIDPos;
auto lhsPos = nodeIDVector->state->selVector->selectedPositions[i];
auto& nodeID = nodeIDVector->getValue<common::internalID_t>(lhsPos);
auto rhsPos = lhsPos;
if (rhsVector->state->selVector->selectedSize == 1) {
rhsPos = rhsVector->state->selVector->selectedPositions[0];
}
column->write(nodeID.offset, rhsVector, rhsPos);
if (lhsVector != nullptr) {
writeToPropertyVector(lhsVector, lhsPos, rhsVector, rhsPos);
}
}
}

void MultiLabelNodeSetExecutor::set() {
evaluator->evaluate();
for (auto i = 0u; i < nodeIDVector->state->selVector->selectedSize; ++i) {
auto nodeIDPos = nodeIDVector->state->selVector->selectedPositions[i];
auto& nodeID = nodeIDVector->getValue<common::internalID_t>(nodeIDPos);
auto lhsPos = nodeIDVector->state->selVector->selectedPositions[i];
auto& nodeID = nodeIDVector->getValue<common::internalID_t>(lhsPos);
if (!tableIDToColumn.contains(nodeID.tableID)) {
if (lhsVector != nullptr) {
lhsVector->setNull(lhsPos, true);
}
continue;
}
auto column = tableIDToColumn.at(nodeID.tableID);
auto rhsPos = nodeIDPos;
auto rhsPos = lhsPos;
if (rhsVector->state->selVector->selectedSize == 1) {
rhsPos = rhsVector->state->selVector->selectedPositions[0];
}
column->write(nodeID.offset, rhsVector, rhsPos);
if (lhsVector != nullptr) {
writeToPropertyVector(lhsVector, lhsPos, rhsVector, rhsPos);
}
}
}

void RelSetExecutor::init(ResultSet* resultSet, ExecutionContext* context) {
srcNodeIDVector = resultSet->getValueVector(srcNodeIDPos).get();
dstNodeIDVector = resultSet->getValueVector(dstNodeIDPos).get();
relIDVector = resultSet->getValueVector(relIDPos).get();
if (lhsVectorPos.dataChunkPos != INVALID_DATA_CHUNK_POS) {
lhsVector = resultSet->getValueVector(lhsVectorPos).get();
}
evaluator->init(*resultSet, context->memoryManager);
rhsVector = evaluator->resultVector.get();
}

// Assume both input vectors are flat. Should be removed eventually.
static void writeToPropertyVector(ValueVector* propertyVector, ValueVector* rhsVector) {
assert(propertyVector->state->selVector->selectedSize == 1);
auto propertyVectorPos = propertyVector->state->selVector->selectedPositions[0];
assert(rhsVector->state->selVector->selectedSize == 1);
auto rhsVectorPos = rhsVector->state->selVector->selectedPositions[0];
writeToPropertyVector(propertyVector, propertyVectorPos, rhsVector, rhsVectorPos);
}

void SingleLabelRelSetExecutor::set() {
evaluator->evaluate();
table->updateRel(srcNodeIDVector, dstNodeIDVector, relIDVector, rhsVector, propertyID);
if (lhsVector != nullptr) {
writeToPropertyVector(lhsVector, rhsVector);
}
}

void MultiLabelRelSetExecutor::set() {
Expand All @@ -58,10 +97,16 @@ void MultiLabelRelSetExecutor::set() {
auto pos = relIDVector->state->selVector->selectedPositions[0];
auto relID = relIDVector->getValue<common::internalID_t>(pos);
if (!tableIDToTableAndPropertyID.contains(relID.tableID)) {
if (lhsVector != nullptr) {
lhsVector->setNull(pos, true);
}
return;
}
auto [table, propertyID] = tableIDToTableAndPropertyID.at(relID.tableID);
table->updateRel(srcNodeIDVector, dstNodeIDVector, relIDVector, rhsVector, propertyID);
if (lhsVector != nullptr) {
writeToPropertyVector(lhsVector, rhsVector);
}
}

} // namespace processor
Expand Down
18 changes: 8 additions & 10 deletions test/test_files/exceptions/binder/binder_error.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,14 @@ Binder exception: Union and union all can't be used together.
---- error
Binder exception: Lower and upper bound of a rel must be greater than 0.

-LOG ReadAfterUpdate
-STATEMENT MATCH (a:person) SET a.age = 35 WITH a MATCH (a)-[:knows]->(b:person) RETURN a.age
---- error
Binder exception: Read after update is not supported. Try query with multiple statements.
-STATEMENT MATCH (a:person) WHERE a.age = 35 DELETE a WITH a MATCH (a)-[:knows]->(b:person) RETURN a.age
---- error
Binder exception: Read after update is not supported. Try query with multiple statements.

-LOG SetDataTypeMisMatch
-STATEMENT MATCH (a:person) SET a.age = 'hh'
---- error
Expand All @@ -219,16 +227,6 @@ Binder exception: Expression hh has data type STRING but expect INT64. Implicit
---- error
Binder exception: Expression 12 has data type INT64 but expect DATE. Implicit cast is not supported.

-LOG ReadAfterUpdate1
-STATEMENT MATCH (a:person) SET a.age = 35 WITH a MATCH (a)-[:knows]->(b:person) RETURN a.age
---- error
Binder exception: Read after update is not supported.

-LOG ReadAfterUpdate3
-STATEMENT MATCH (a:person) SET a.age=3 RETURN a.fName
---- error
Binder exception: Return/With after update is not supported.

-LOG DeleteNodeProperty
-STATEMENT MATCH (a:person) DELETE a.age
---- error
Expand Down
5 changes: 0 additions & 5 deletions test/test_files/tinysnb/exception/exception.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ Runtime exception: Modulo by zero.
---- error
Connection Exception: Query is empty.

-CASE ReadAfterUpdate2
-STATEMENT MATCH (a:person) WHERE a.age = 35 DELETE a WITH a MATCH (a)-[:knows]->(b:person) RETURN a.age
---- error
Binder exception: Read after update is not supported.

-CASE Overflow
-STATEMENT RETURN to_int16(10000000000)
---- error
Expand Down

0 comments on commit 0317cef

Please sign in to comment.