kuzudb · andyfengHKU · Aug 2, 2023 · Aug 1, 2023
diff --git a/src/binder/bind/bind_query.cpp b/src/binder/bind/bind_query.cpp
@@ -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);

diff --git a/src/binder/binder.cpp b/src/binder/binder.cpp
@@ -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();
     }

diff --git a/src/include/binder/binder.h b/src/include/binder/binder.h
@@ -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);

diff --git a/src/include/processor/operator/update/set_executor.h b/src/include/processor/operator/update/set_executor.h
@@ -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);
@@ -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;
 
@@ -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;
 
@@ -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);
@@ -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} {}
 
@@ -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} {}
 

diff --git a/src/processor/mapper/map_set.cpp b/src/processor/mapper/map_set.cpp
@@ -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()) {
@@ -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));
     }
 }
 
@@ -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;
@@ -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));
     }
 }
 

diff --git a/src/processor/operator/update/set_executor.cpp b/src/processor/operator/update/set_executor.cpp
@@ -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() {
@@ -58,10 +97,16 @@
     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

diff --git a/test/test_files/exceptions/binder/binder_error.test b/test/test_files/exceptions/binder/binder_error.test
@@ -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
@@ -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

diff --git a/test/test_files/tinysnb/exception/exception.test b/test/test_files/tinysnb/exception/exception.test
@@ -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