From ed50628c43b421f3bc110ea12bf0aa8412368309 Mon Sep 17 00:00:00 2001 From: xiyang Date: Sun, 20 Nov 2022 16:24:43 +0800 Subject: [PATCH] add delete rel planning --- src/binder/bind/bind_updating_clause.cpp | 25 ++++-- src/binder/include/binder.h | 2 + .../updating_clause/bound_create_clause.cpp | 10 +-- .../updating_clause/bound_delete_clause.cpp | 26 ++++++ .../include/bound_create_clause.h | 13 ++- .../include/bound_delete_clause.h | 58 +++++++----- .../include/array_extract_operation.h | 2 +- src/planner/include/query_planner.h | 8 +- src/planner/include/update_planner.h | 23 ++--- .../logical_plan/include/logical_plan.h | 8 +- .../include/base_logical_operator.h | 10 ++- .../logical_operator/include/logical_create.h | 68 +++----------- .../include/logical_create_delete.h | 64 +++++++++++++ .../logical_operator/include/logical_delete.h | 49 ++++------ src/planner/update_planner.cpp | 83 ++++++++--------- src/processor/mapper/include/plan_mapper.h | 4 +- src/processor/mapper/map_create.cpp | 10 +-- src/processor/mapper/map_delete.cpp | 47 ++++++---- src/processor/mapper/plan_mapper.cpp | 7 +- .../operator/include/physical_operator.h | 15 ++-- src/processor/operator/update/delete.cpp | 56 +++++++++--- .../operator/update/include/delete.h | 90 ++++++++++++++++--- src/storage/store/include/rel_table.h | 7 ++ test/binder/binder_error_test.cpp | 7 -- test/runner/e2e_exception_test.cpp | 8 ++ 25 files changed, 440 insertions(+), 260 deletions(-) create mode 100644 src/binder/query/updating_clause/bound_delete_clause.cpp create mode 100644 src/planner/logical_plan/logical_operator/include/logical_create_delete.h diff --git a/src/binder/bind/bind_updating_clause.cpp b/src/binder/bind/bind_updating_clause.cpp index 952b1a8156..b49415933f 100644 --- a/src/binder/bind/bind_updating_clause.cpp +++ b/src/binder/bind/bind_updating_clause.cpp @@ -31,25 +31,23 @@ unique_ptr Binder::bindCreateClause(const UpdatingClause& u auto prevVariablesInScope = variablesInScope; auto [queryGraphCollection, propertyCollection] = bindGraphPattern(createClause.getPatternElements()); - vector> boundCreateNodes; - vector> boundCreateRels; + auto boundCreateClause = make_unique(); for (auto i = 0u; i < queryGraphCollection->getNumQueryGraphs(); ++i) { auto queryGraph = queryGraphCollection->getQueryGraph(i); for (auto j = 0u; j < queryGraph->getNumQueryNodes(); ++j) { auto node = queryGraph->getQueryNode(j); if (!prevVariablesInScope.contains(node->getRawName())) { - boundCreateNodes.push_back(bindCreateNode(node, *propertyCollection)); + boundCreateClause->addCreateNode(bindCreateNode(node, *propertyCollection)); } } for (auto j = 0u; j < queryGraph->getNumQueryRels(); ++j) { auto rel = queryGraph->getQueryRel(j); if (!prevVariablesInScope.contains(rel->getRawName())) { - boundCreateRels.push_back(bindCreateRel(rel, *propertyCollection)); + boundCreateClause->addCreateRel(bindCreateRel(rel, *propertyCollection)); } } } - auto boundCreateClause = - make_unique(std::move(boundCreateNodes), std::move(boundCreateRels)); + return boundCreateClause; } @@ -117,15 +115,26 @@ unique_ptr Binder::bindDeleteClause(const UpdatingClause& u auto boundDeleteClause = make_unique(); for (auto i = 0u; i < deleteClause.getNumExpressions(); ++i) { auto boundExpression = expressionBinder.bindExpression(*deleteClause.getExpression(i)); - if (boundExpression->dataType.typeID != NODE) { + if (boundExpression->dataType.typeID == NODE) { + boundDeleteClause->addDeleteNode( + bindDeleteNode(static_pointer_cast(boundExpression))); + } else if (boundExpression->dataType.typeID == REL) { + boundDeleteClause->addDeleteRel(static_pointer_cast(boundExpression)); + } else { throw BinderException("Delete " + expressionTypeToString(boundExpression->expressionType) + " is not supported."); } - boundDeleteClause->addExpression(move(boundExpression)); } return boundDeleteClause; } +unique_ptr Binder::bindDeleteNode(shared_ptr node) { + auto nodeTableSchema = catalog.getReadOnlyVersion()->getNodeTableSchema(node->getTableID()); + auto primaryKey = nodeTableSchema->getPrimaryKey(); + auto primaryKeyExpression = expressionBinder.bindNodePropertyExpression(node, primaryKey); + return make_unique(node, primaryKeyExpression); +} + } // namespace binder } // namespace kuzu diff --git a/src/binder/include/binder.h b/src/binder/include/binder.h index 64d2af9c9a..6642793f39 100644 --- a/src/binder/include/binder.h +++ b/src/binder/include/binder.h @@ -16,6 +16,7 @@ namespace binder { class BoundCreateNode; class BoundCreateRel; +class BoundDeleteNode; class Binder { friend class ExpressionBinder; @@ -76,6 +77,7 @@ class Binder { shared_ptr node, const PropertyKeyValCollection& collection); unique_ptr bindCreateRel( shared_ptr rel, const PropertyKeyValCollection& collection); + unique_ptr bindDeleteNode(shared_ptr node); /*** bind projection clause ***/ unique_ptr bindWithClause(const WithClause& withClause); diff --git a/src/binder/query/updating_clause/bound_create_clause.cpp b/src/binder/query/updating_clause/bound_create_clause.cpp index 66ee153d2c..23d3841b12 100644 --- a/src/binder/query/updating_clause/bound_create_clause.cpp +++ b/src/binder/query/updating_clause/bound_create_clause.cpp @@ -29,16 +29,14 @@ expression_vector BoundCreateClause::getPropertiesToRead() const { } unique_ptr BoundCreateClause::copy() { - vector> copiedCreateNodes; - vector> copiedCreateRels; + auto result = make_unique(); for (auto& createNode : createNodes) { - copiedCreateNodes.push_back(createNode->copy()); + result->addCreateNode(createNode->copy()); } for (auto& createRel : createRels) { - copiedCreateRels.push_back(createRel->copy()); + result->addCreateRel(createRel->copy()); } - return make_unique( - std::move(copiedCreateNodes), std::move(copiedCreateRels)); + return result; } } // namespace binder diff --git a/src/binder/query/updating_clause/bound_delete_clause.cpp b/src/binder/query/updating_clause/bound_delete_clause.cpp new file mode 100644 index 0000000000..59e129d08b --- /dev/null +++ b/src/binder/query/updating_clause/bound_delete_clause.cpp @@ -0,0 +1,26 @@ +#include "include/bound_delete_clause.h" + +namespace kuzu { +namespace binder { + +expression_vector BoundDeleteClause::getPropertiesToRead() const { + expression_vector result; + for (auto& deleteNode : deleteNodes) { + result.push_back(deleteNode->getPrimaryKeyExpression()); + } + return result; +} + +unique_ptr BoundDeleteClause::copy() { + auto result = make_unique(); + for (auto& deleteNode : deleteNodes) { + result->addDeleteNode(deleteNode->copy()); + } + for (auto& deleteRel : deleteRels) { + result->addDeleteRel(deleteRel); + } + return result; +} + +} // namespace binder +} // namespace kuzu diff --git a/src/binder/query/updating_clause/include/bound_create_clause.h b/src/binder/query/updating_clause/include/bound_create_clause.h index f3ea7ee10c..21c6d5cd58 100644 --- a/src/binder/query/updating_clause/include/bound_create_clause.h +++ b/src/binder/query/updating_clause/include/bound_create_clause.h @@ -54,19 +54,24 @@ class BoundCreateRel : public BoundCreateNodeOrRel { class BoundCreateClause : public BoundUpdatingClause { public: - BoundCreateClause(vector> createNodes, - vector> createRels) - : BoundUpdatingClause{ClauseType::CREATE}, createNodes{std::move(createNodes)}, - createRels{std::move(createRels)} {}; + BoundCreateClause() : BoundUpdatingClause{ClauseType::CREATE} {}; ~BoundCreateClause() override = default; + inline void addCreateNode(unique_ptr createNode) { + createNodes.push_back(std::move(createNode)); + } inline bool hasCreateNode() const { return !createNodes.empty(); } inline uint32_t getNumCreateNodes() const { return createNodes.size(); } inline BoundCreateNode* getCreateNode(uint32_t idx) const { return createNodes[idx].get(); } + inline const vector>& getCreateNodes() const { return createNodes; } + inline void addCreateRel(unique_ptr createRel) { + createRels.push_back(std::move(createRel)); + } inline bool hasCreateRel() const { return !createRels.empty(); } inline uint32_t getNumCreateRels() const { return createRels.size(); } inline BoundCreateRel* getCreateRel(uint32_t idx) const { return createRels[idx].get(); } + inline const vector>& getCreateRels() const { return createRels; } vector getAllSetItems() const; expression_vector getPropertiesToRead() const override; diff --git a/src/binder/query/updating_clause/include/bound_delete_clause.h b/src/binder/query/updating_clause/include/bound_delete_clause.h index 0e2d38dd00..bd8f5193f5 100644 --- a/src/binder/query/updating_clause/include/bound_delete_clause.h +++ b/src/binder/query/updating_clause/include/bound_delete_clause.h @@ -2,40 +2,56 @@ #include "bound_updating_clause.h" +#include "src/binder/expression/include/rel_expression.h" + namespace kuzu { namespace binder { +class BoundDeleteNode { +public: + BoundDeleteNode(shared_ptr node, shared_ptr primaryKeyExpression) + : node{std::move(node)}, primaryKeyExpression{std::move(primaryKeyExpression)} {} + + inline shared_ptr getNode() const { return node; } + inline shared_ptr getPrimaryKeyExpression() const { return primaryKeyExpression; } + + inline unique_ptr copy() { + return make_unique(node, primaryKeyExpression); + } + +private: + shared_ptr node; + shared_ptr primaryKeyExpression; +}; + class BoundDeleteClause : public BoundUpdatingClause { public: BoundDeleteClause() : BoundUpdatingClause{ClauseType::DELETE} {}; ~BoundDeleteClause() override = default; - inline void addExpression(shared_ptr expression) { - expressions.push_back(move(expression)); - } - inline uint32_t getNumExpressions() const { return expressions.size(); } - inline shared_ptr getExpression(uint32_t idx) const { return expressions[idx]; } - - inline expression_vector getPropertiesToRead() const override { - expression_vector result; - for (auto& expression : expressions) { - for (auto& property : expression->getSubPropertyExpressions()) { - result.push_back(property); - } - } - return result; + inline void addDeleteNode(unique_ptr deleteNode) { + deleteNodes.push_back(std::move(deleteNode)); } + inline bool hasDeleteNode() const { return !deleteNodes.empty(); } + inline uint32_t getNumDeleteNodes() const { return deleteNodes.size(); } + inline BoundDeleteNode* getDeleteNode(uint32_t idx) const { return deleteNodes[idx].get(); } + inline const vector>& getDeleteNodes() const { return deleteNodes; } - inline unique_ptr copy() override { - auto result = make_unique(); - for (auto& expression : expressions) { - result->addExpression(expression); - } - return result; + inline void addDeleteRel(shared_ptr deleteRel) { + deleteRels.push_back(std::move(deleteRel)); } + inline bool hasDeleteRel() const { return !deleteRels.empty(); } + inline uint32_t getNumDeleteRels() const { return deleteRels.size(); } + inline shared_ptr getDeleteRel(uint32_t idx) const { return deleteRels[idx]; } + inline vector> getDeleteRels() const { return deleteRels; } + + expression_vector getPropertiesToRead() const override; + + unique_ptr copy() override; private: - vector> expressions; + vector> deleteNodes; + vector> deleteRels; }; } // namespace binder diff --git a/src/function/string/operations/include/array_extract_operation.h b/src/function/string/operations/include/array_extract_operation.h index 7851197512..d8d3a0feda 100644 --- a/src/function/string/operations/include/array_extract_operation.h +++ b/src/function/string/operations/include/array_extract_operation.h @@ -27,7 +27,7 @@ struct ArrayExtract { auto startPos = idxPos - 1; auto endPos = startPos + 1; bool isAscii = true; - for (auto i = 0u; i < min((uint64_t)idxPos + 1, stringVal.size()); i++) { + for (auto i = 0u; i < min((size_t)idxPos + 1, stringVal.size()); i++) { if (stringVal[i] & 0x80) { isAscii = false; break; diff --git a/src/planner/include/query_planner.h b/src/planner/include/query_planner.h index 590b59248a..cc8a06886d 100644 --- a/src/planner/include/query_planner.h +++ b/src/planner/include/query_planner.h @@ -20,11 +20,9 @@ class QueryPlanner { public: explicit QueryPlanner(const Catalog& catalog, - const NodesStatisticsAndDeletedIDs& nodesStatisticsAndDeletedIDs, - const RelsStatistics& relsStatistics) - : catalog{catalog}, joinOrderEnumerator{catalog, nodesStatisticsAndDeletedIDs, - relsStatistics, this}, - projectionPlanner{this}, updatePlanner{catalog, this} {} + const NodesStatisticsAndDeletedIDs& nodesStatistics, const RelsStatistics& relsStatistics) + : catalog{catalog}, joinOrderEnumerator{catalog, nodesStatistics, relsStatistics, this}, + projectionPlanner{this}, updatePlanner{} {} vector> getAllPlans(const BoundStatement& boundStatement); diff --git a/src/planner/include/update_planner.h b/src/planner/include/update_planner.h index 5098ecac50..a75ebcb744 100644 --- a/src/planner/include/update_planner.h +++ b/src/planner/include/update_planner.h @@ -4,20 +4,14 @@ #include "src/binder/query/updating_clause/include/bound_delete_clause.h" #include "src/binder/query/updating_clause/include/bound_set_clause.h" #include "src/binder/query/updating_clause/include/bound_updating_clause.h" -#include "src/catalog/include/catalog.h" #include "src/planner/logical_plan/include/logical_plan.h" namespace kuzu { namespace planner { -using namespace kuzu::catalog; - -class QueryPlanner; - class UpdatePlanner { public: - UpdatePlanner(const catalog::Catalog& catalog, QueryPlanner* queryPlanner) - : catalog{catalog}, queryPlanner{queryPlanner} {}; + UpdatePlanner() = default; inline void planUpdatingClause( BoundUpdatingClause& updatingClause, vector>& plans) { @@ -31,20 +25,19 @@ class UpdatePlanner { void planSetItem(expression_pair setItem, LogicalPlan& plan); void planCreate(BoundCreateClause& createClause, LogicalPlan& plan); - void appendCreateNode(BoundCreateClause& createClause, LogicalPlan& plan); - void appendCreateRel(BoundCreateClause& createClause, LogicalPlan& plan); + void appendCreateNode( + const vector>& createNodes, LogicalPlan& plan); + void appendCreateRel(const vector>& createRels, LogicalPlan& plan); inline void appendSet(BoundSetClause& setClause, LogicalPlan& plan) { appendSet(setClause.getSetItems(), plan); } void appendSet(vector setItems, LogicalPlan& plan); - void appendDelete(BoundDeleteClause& deleteClause, LogicalPlan& plan); - vector splitSetItems(vector setItems); - -private: - const Catalog& catalog; - QueryPlanner* queryPlanner; + void planDelete(BoundDeleteClause& deleteClause, LogicalPlan& plan); + void appendDeleteNode( + const vector>& deleteNodes, LogicalPlan& plan); + void appendDeleteRel(const vector>& deleteRels, LogicalPlan& plan); }; } // namespace planner diff --git a/src/planner/logical_plan/include/logical_plan.h b/src/planner/logical_plan/include/logical_plan.h index 70242d1761..611b234e2f 100644 --- a/src/planner/logical_plan/include/logical_plan.h +++ b/src/planner/logical_plan/include/logical_plan.h @@ -20,10 +20,10 @@ class LogicalPlan { inline bool isEmpty() const { return lastOperator == nullptr; } inline bool isReadOnly() const { - return !lastOperator->descendantsContainType( - unordered_set{LOGICAL_SET_NODE_PROPERTY, LOGICAL_CREATE_NODE, - LOGICAL_CREATE_REL, LOGICAL_DELETE, LOGICAL_CREATE_NODE_TABLE, - LOGICAL_CREATE_REL_TABLE, LOGICAL_COPY_CSV, LOGICAL_DROP_TABLE}); + return !lastOperator->descendantsContainType(unordered_set{ + LOGICAL_SET_NODE_PROPERTY, LOGICAL_CREATE_NODE, LOGICAL_CREATE_REL, LOGICAL_DELETE_NODE, + LOGICAL_DELETE_REL, LOGICAL_CREATE_NODE_TABLE, LOGICAL_CREATE_REL_TABLE, + LOGICAL_COPY_CSV, LOGICAL_DROP_TABLE}); } inline void setExpressionsToCollect(expression_vector expressions) { diff --git a/src/planner/logical_plan/logical_operator/include/base_logical_operator.h b/src/planner/logical_plan/logical_operator/include/base_logical_operator.h index 802807c836..08060a0218 100644 --- a/src/planner/logical_plan/logical_operator/include/base_logical_operator.h +++ b/src/planner/logical_plan/logical_operator/include/base_logical_operator.h @@ -31,7 +31,8 @@ enum LogicalOperatorType : uint8_t { LOGICAL_CREATE_NODE, LOGICAL_CREATE_REL, LOGICAL_SET_NODE_PROPERTY, - LOGICAL_DELETE, + LOGICAL_DELETE_NODE, + LOGICAL_DELETE_REL, LOGICAL_ACCUMULATE, LOGICAL_EXPRESSIONS_SCAN, LOGICAL_FTABLE_SCAN, @@ -47,9 +48,10 @@ const string LogicalOperatorTypeNames[] = {"LOGICAL_SCAN_NODE", "LOGICAL_INDEX_S "LOGICAL_CROSS_PRODUCT", "LOGICAL_SEMI_MASKER", "LOGICAL_HASH_JOIN", "LOGICAL_MULTIPLICITY_REDUCER", "LOGICAL_LIMIT", "LOGICAL_SKIP", "LOGICAL_AGGREGATE", "LOGICAL_ORDER_BY", "LOGICAL_UNION_ALL", "LOGICAL_DISTINCT", "LOGICAL_CREATE_NODE", - "LOGICAL_CREATE_REL", "LOGICAL_SET_NODE_PROPERTY", "LOGICAL_DELETE", "LOGICAL_ACCUMULATE", - "LOGICAL_EXPRESSIONS_SCAN", "LOGICAL_FTABLE_SCAN", "LOGICAL_CREATE_NODE_TABLE", - "LOGICAL_CREATE_REL_TABLE", "LOGICAL_COPY_CSV", "LOGICAL_DROP_TABLE"}; + "LOGICAL_CREATE_REL", "LOGICAL_SET_NODE_PROPERTY", "LOGICAL_DELETE_NODE", "LOGICAL_DELETE_REL", + "LOGICAL_ACCUMULATE", "LOGICAL_EXPRESSIONS_SCAN", "LOGICAL_FTABLE_SCAN", + "LOGICAL_CREATE_NODE_TABLE", "LOGICAL_CREATE_REL_TABLE", "LOGICAL_COPY_CSV", + "LOGICAL_DROP_TABLE"}; class LogicalOperator { public: diff --git a/src/planner/logical_plan/logical_operator/include/logical_create.h b/src/planner/logical_plan/logical_operator/include/logical_create.h index 90609903eb..2cd6eb0e2d 100644 --- a/src/planner/logical_plan/logical_operator/include/logical_create.h +++ b/src/planner/logical_plan/logical_operator/include/logical_create.h @@ -1,82 +1,37 @@ #pragma once -#include "base_logical_operator.h" - -#include "src/binder/expression/include/expression.h" -#include "src/binder/expression/include/rel_expression.h" +#include "logical_create_delete.h" namespace kuzu { namespace planner { -struct NodeAndPrimaryKey { - shared_ptr node; - shared_ptr primaryKey; - - NodeAndPrimaryKey(shared_ptr node, shared_ptr primaryKey) - : node{std::move(node)}, primaryKey{std::move(primaryKey)} {} - - inline unique_ptr copy() const { - return make_unique(node, primaryKey); - } -}; - -class LogicalCreateNode : public LogicalOperator { +class LogicalCreateNode : public LogicalCreateOrDeleteNode { public: LogicalCreateNode( - vector> nodeAndPrimaryKeys, shared_ptr child) - : LogicalOperator{std::move(child)}, nodeAndPrimaryKeys{std::move(nodeAndPrimaryKeys)} {} + vector, shared_ptr>> nodeAndPrimaryKeys, + shared_ptr child) + : LogicalCreateOrDeleteNode{std::move(nodeAndPrimaryKeys), std::move(child)} {} - inline LogicalOperatorType getLogicalOperatorType() const override { + LogicalOperatorType getLogicalOperatorType() const override { return LogicalOperatorType::LOGICAL_CREATE_NODE; } - inline string getExpressionsForPrinting() const override { - expression_vector expressions; - for (auto& nodeAndPrimaryKey : nodeAndPrimaryKeys) { - expressions.push_back(nodeAndPrimaryKey->node); - } - return ExpressionUtil::toString(expressions); - } - - inline uint32_t getNumNodeAndPrimaryKeys() const { return nodeAndPrimaryKeys.size(); } - inline NodeAndPrimaryKey* getNodeAndPrimaryKey(uint32_t idx) { - return nodeAndPrimaryKeys[idx].get(); - } - - inline unique_ptr copy() override { - vector> copiedNodeAndPrimaryKeys; - for (auto& nodeAndPrimaryKey : nodeAndPrimaryKeys) { - copiedNodeAndPrimaryKeys.push_back(nodeAndPrimaryKey->copy()); - } - return make_unique( - std::move(copiedNodeAndPrimaryKeys), children[0]->copy()); + unique_ptr copy() override { + return make_unique(nodeAndPrimaryKeys, children[0]->copy()); } - -private: - vector> nodeAndPrimaryKeys; }; -class LogicalCreateRel : public LogicalOperator { +class LogicalCreateRel : public LogicalCreateOrDeleteRel { public: LogicalCreateRel(vector> rels, vector> setItemsPerRel, shared_ptr child) - : LogicalOperator{std::move(child)}, rels{std::move(rels)}, setItemsPerRel{std::move( - setItemsPerRel)} {} + : LogicalCreateOrDeleteRel{std::move(rels), std::move(child)}, setItemsPerRel{std::move( + setItemsPerRel)} {} inline LogicalOperatorType getLogicalOperatorType() const override { return LogicalOperatorType::LOGICAL_CREATE_REL; } - inline string getExpressionsForPrinting() const override { - expression_vector expressions; - for (auto& rel : rels) { - expressions.push_back(rel); - } - return ExpressionUtil::toString(expressions); - } - - inline uint32_t getNumRels() const { return rels.size(); } - inline shared_ptr getRel(uint32_t idx) const { return rels[idx]; } inline vector getSetItems(uint32_t idx) const { return setItemsPerRel[idx]; } unique_ptr copy() override { @@ -84,7 +39,6 @@ class LogicalCreateRel : public LogicalOperator { } private: - vector> rels; vector> setItemsPerRel; }; diff --git a/src/planner/logical_plan/logical_operator/include/logical_create_delete.h b/src/planner/logical_plan/logical_operator/include/logical_create_delete.h new file mode 100644 index 0000000000..9f7cb30b73 --- /dev/null +++ b/src/planner/logical_plan/logical_operator/include/logical_create_delete.h @@ -0,0 +1,64 @@ +#pragma once + +#include "base_logical_operator.h" + +#include "src/binder/expression/include/rel_expression.h" + +namespace kuzu { +namespace planner { + +class LogicalCreateOrDeleteNode : public LogicalOperator { +public: + LogicalCreateOrDeleteNode( + vector, shared_ptr>> nodeAndPrimaryKeys, + shared_ptr child) + : LogicalOperator{std::move(child)}, nodeAndPrimaryKeys{std::move(nodeAndPrimaryKeys)} {} + + LogicalOperatorType getLogicalOperatorType() const override = 0; + + inline string getExpressionsForPrinting() const override { + expression_vector expressions; + for (auto& [node, primaryKey] : nodeAndPrimaryKeys) { + expressions.push_back(node); + } + return ExpressionUtil::toString(expressions); + } + + inline vector, shared_ptr>> + getNodeAndPrimaryKeys() const { + return nodeAndPrimaryKeys; + } + + unique_ptr copy() override = 0; + +protected: + vector, shared_ptr>> nodeAndPrimaryKeys; +}; + +class LogicalCreateOrDeleteRel : public LogicalOperator { +public: + LogicalCreateOrDeleteRel( + vector> rels, shared_ptr child) + : LogicalOperator{std::move(child)}, rels{std::move(rels)} {} + + LogicalOperatorType getLogicalOperatorType() const override = 0; + + inline string getExpressionsForPrinting() const override { + expression_vector expressions; + for (auto& rel : rels) { + expressions.push_back(rel); + } + return ExpressionUtil::toString(expressions); + } + + inline uint32_t getNumRels() const { return rels.size(); } + inline shared_ptr getRel(uint32_t idx) const { return rels[idx]; } + + unique_ptr copy() override = 0; + +protected: + vector> rels; +}; + +} // namespace planner +} // namespace kuzu diff --git a/src/planner/logical_plan/logical_operator/include/logical_delete.h b/src/planner/logical_plan/logical_operator/include/logical_delete.h index bb8c0131b9..bd866299eb 100644 --- a/src/planner/logical_plan/logical_operator/include/logical_delete.h +++ b/src/planner/logical_plan/logical_operator/include/logical_delete.h @@ -1,49 +1,38 @@ #pragma once -#include "base_logical_operator.h" - -#include "src/binder/expression/include/expression.h" - -using namespace kuzu::binder; +#include "logical_create_delete.h" namespace kuzu { namespace planner { -class LogicalDelete : public LogicalOperator { +class LogicalDeleteNode : public LogicalCreateOrDeleteNode { public: - LogicalDelete(expression_vector nodeExpressions, expression_vector primaryKeyExpressions, + LogicalDeleteNode( + vector, shared_ptr>> nodeAndPrimaryKeys, shared_ptr child) - : LogicalOperator{move(child)}, nodeExpressions{move(nodeExpressions)}, - primaryKeyExpressions{move(primaryKeyExpressions)} {} + : LogicalCreateOrDeleteNode{std::move(nodeAndPrimaryKeys), std::move(child)} {} - LogicalOperatorType getLogicalOperatorType() const override { - return LogicalOperatorType::LOGICAL_DELETE; + inline LogicalOperatorType getLogicalOperatorType() const override { + return LogicalOperatorType::LOGICAL_DELETE_NODE; } - string getExpressionsForPrinting() const override { - string result; - for (auto& nodeExpression : nodeExpressions) { - result += nodeExpression->getRawName() + ","; - } - return result; + inline unique_ptr copy() override { + return make_unique(nodeAndPrimaryKeys, children[0]->copy()); } +}; - inline uint32_t getNumExpressions() const { return nodeExpressions.size(); } - inline shared_ptr getNodeExpression(uint32_t idx) const { - return nodeExpressions[idx]; - } - inline shared_ptr getPrimaryKeyExpression(uint32_t idx) const { - return primaryKeyExpressions[idx]; - } +class LogicalDeleteRel : public LogicalCreateOrDeleteRel { +public: + LogicalDeleteRel(vector> rels, shared_ptr child) + : LogicalCreateOrDeleteRel{std::move(rels), std::move(child)} {} - unique_ptr copy() override { - return make_unique( - nodeExpressions, primaryKeyExpressions, children[0]->copy()); + inline LogicalOperatorType getLogicalOperatorType() const override { + return LogicalOperatorType::LOGICAL_DELETE_REL; } -private: - expression_vector nodeExpressions; - expression_vector primaryKeyExpressions; + inline unique_ptr copy() override { + return make_unique(rels, children[0]->copy()); + } }; } // namespace planner diff --git a/src/planner/update_planner.cpp b/src/planner/update_planner.cpp index 25d7688be4..45859ee992 100644 --- a/src/planner/update_planner.cpp +++ b/src/planner/update_planner.cpp @@ -5,7 +5,6 @@ #include "src/planner/logical_plan/logical_operator/include/logical_create.h" #include "src/planner/logical_plan/logical_operator/include/logical_delete.h" #include "src/planner/logical_plan/logical_operator/include/logical_set.h" -#include "src/planner/logical_plan/logical_operator/include/sink_util.h" namespace kuzu { namespace planner { @@ -33,7 +32,7 @@ void UpdatePlanner::planUpdatingClause(BoundUpdatingClause& updatingClause, Logi } case ClauseType::DELETE: { QueryPlanner::appendAccumulate(plan); - appendDelete((BoundDeleteClause&)updatingClause, plan); + planDelete((BoundDeleteClause&)updatingClause, plan); return; } default: @@ -69,25 +68,24 @@ void UpdatePlanner::planCreate(BoundCreateClause& createClause, LogicalPlan& pla QueryPlanner::appendFlattenIfNecessary(groupPos, plan); } if (createClause.hasCreateNode()) { - appendCreateNode(createClause, plan); + appendCreateNode(createClause.getCreateNodes(), plan); } if (createClause.hasCreateRel()) { - appendCreateRel(createClause, plan); + appendCreateRel(createClause.getCreateRels(), plan); } } -void UpdatePlanner::appendCreateNode(BoundCreateClause& createClause, LogicalPlan& plan) { +void UpdatePlanner::appendCreateNode( + const vector>& createNodes, LogicalPlan& plan) { auto schema = plan.getSchema(); vector setItems; - vector> nodeAndPrimaryKeyPairs; - for (auto i = 0; i < createClause.getNumCreateNodes(); ++i) { - auto createNode = createClause.getCreateNode(i); + vector, shared_ptr>> nodeAndPrimaryKeyPairs; + for (auto& createNode : createNodes) { auto node = createNode->getNode(); auto groupPos = schema->createGroup(); schema->insertToGroupAndScope(node->getNodeIDPropertyExpression(), groupPos); schema->flattenGroup(groupPos); // create output is always flat - nodeAndPrimaryKeyPairs.push_back( - make_unique(node, createNode->getPrimaryKeyExpression())); + nodeAndPrimaryKeyPairs.emplace_back(node, createNode->getPrimaryKeyExpression()); for (auto& setItem : createNode->getSetItems()) { setItems.push_back(setItem); } @@ -98,11 +96,11 @@ void UpdatePlanner::appendCreateNode(BoundCreateClause& createClause, LogicalPla appendSet(std::move(setItems), plan); } -void UpdatePlanner::appendCreateRel(BoundCreateClause& createClause, LogicalPlan& plan) { +void UpdatePlanner::appendCreateRel( + const vector>& createRels, LogicalPlan& plan) { vector> rels; vector> setItemsPerRel; - for (auto i = 0; i < createClause.getNumCreateRels(); ++i) { - auto createRel = createClause.getCreateRel(i); + for (auto& createRel : createRels) { rels.push_back(createRel->getRel()); setItemsPerRel.push_back(createRel->getSetItems()); } @@ -115,39 +113,44 @@ void UpdatePlanner::appendSet(vector setItems, LogicalPlan& pla for (auto& setItem : setItems) { planSetItem(setItem, plan); } - auto structuredSetItems = splitSetItems(setItems); - if (!structuredSetItems.empty()) { - plan.setLastOperator(make_shared( - std::move(structuredSetItems), plan.getLastOperator())); + if (!setItems.empty()) { + plan.setLastOperator( + make_shared(std::move(setItems), plan.getLastOperator())); } } -void UpdatePlanner::appendDelete(BoundDeleteClause& deleteClause, LogicalPlan& plan) { - expression_vector nodeExpressions; - expression_vector primaryKeyExpressions; - for (auto i = 0u; i < deleteClause.getNumExpressions(); ++i) { - auto expression = deleteClause.getExpression(i); - assert(expression->dataType.typeID == NODE); - auto& nodeExpression = (NodeExpression&)*expression; - auto pk = - catalog.getReadOnlyVersion()->getNodePrimaryKeyProperty(nodeExpression.getTableID()); - auto pkExpression = - make_shared(pk.dataType, pk.name, pk.propertyID, expression); - queryPlanner->appendScanNodePropIfNecessarySwitch(pkExpression, nodeExpression, plan); - nodeExpressions.push_back(expression); - primaryKeyExpressions.push_back(pkExpression); - } - auto deleteOperator = - make_shared(nodeExpressions, primaryKeyExpressions, plan.getLastOperator()); - plan.setLastOperator(deleteOperator); +void UpdatePlanner::planDelete(BoundDeleteClause& deleteClause, LogicalPlan& plan) { + if (deleteClause.hasDeleteRel()) { + appendDeleteRel(deleteClause.getDeleteRels(), plan); + } + if (deleteClause.hasDeleteNode()) { + appendDeleteNode(deleteClause.getDeleteNodes(), plan); + } +} + +void UpdatePlanner::appendDeleteNode( + const vector>& deleteNodes, LogicalPlan& plan) { + vector, shared_ptr>> nodeAndPrimaryKeyPairs; + for (auto& deleteNode : deleteNodes) { + nodeAndPrimaryKeyPairs.emplace_back( + deleteNode->getNode(), deleteNode->getPrimaryKeyExpression()); + } + auto deleteNode = + make_shared(std::move(nodeAndPrimaryKeyPairs), plan.getLastOperator()); + plan.setLastOperator(std::move(deleteNode)); } -vector UpdatePlanner::splitSetItems(vector setItems) { - vector result; - for (auto& [lhs, rhs] : setItems) { - result.emplace_back(lhs, rhs); +void UpdatePlanner::appendDeleteRel( + const vector>& deleteRels, LogicalPlan& plan) { + // Delete one rel at a time so we flatten for each rel. + for (auto& rel : deleteRels) { + auto srcNodeID = rel->getSrcNode()->getNodeIDPropertyExpression(); + QueryPlanner::appendFlattenIfNecessary(srcNodeID, plan); + auto dstNodeID = rel->getDstNode()->getNodeIDPropertyExpression(); + QueryPlanner::appendFlattenIfNecessary(dstNodeID, plan); } - return result; + auto deleteRel = make_shared(deleteRels, plan.getLastOperator()); + plan.setLastOperator(std::move(deleteRel)); } } // namespace planner diff --git a/src/processor/mapper/include/plan_mapper.h b/src/processor/mapper/include/plan_mapper.h index edff5ac96b..2f2338565a 100644 --- a/src/processor/mapper/include/plan_mapper.h +++ b/src/processor/mapper/include/plan_mapper.h @@ -82,7 +82,9 @@ class PlanMapper { LogicalOperator* logicalOperator, MapperContext& mapperContext); unique_ptr mapLogicalSetToPhysical( LogicalOperator* logicalOperator, MapperContext& mapperContext); - unique_ptr mapLogicalDeleteToPhysical( + unique_ptr mapLogicalDeleteNodeToPhysical( + LogicalOperator* logicalOperator, MapperContext& mapperContext); + unique_ptr mapLogicalDeleteRelToPhysical( LogicalOperator* logicalOperator, MapperContext& mapperContext); unique_ptr mapLogicalCreateNodeTableToPhysical( LogicalOperator* logicalOperator, MapperContext& mapperContext); diff --git a/src/processor/mapper/map_create.cpp b/src/processor/mapper/map_create.cpp index 97b089070c..173094202a 100644 --- a/src/processor/mapper/map_create.cpp +++ b/src/processor/mapper/map_create.cpp @@ -14,19 +14,17 @@ unique_ptr PlanMapper::mapLogicalCreateNodeToPhysical( auto& nodesStore = storageManager.getNodesStore(); auto catalogContent = catalog->getReadOnlyVersion(); vector> createNodeInfos; - for (auto i = 0u; i < logicalCreateNode->getNumNodeAndPrimaryKeys(); ++i) { - auto nodeAndPrimaryKey = logicalCreateNode->getNodeAndPrimaryKey(i); - auto nodeTableID = nodeAndPrimaryKey->node->getTableID(); + for (auto& [node, primaryKey] : logicalCreateNode->getNodeAndPrimaryKeys()) { + auto nodeTableID = node->getTableID(); auto table = nodesStore.getNodeTable(nodeTableID); - auto primaryKeyEvaluator = - expressionMapper.mapExpression(nodeAndPrimaryKey->primaryKey, mapperContext); + auto primaryKeyEvaluator = expressionMapper.mapExpression(primaryKey, mapperContext); vector relTablesToInit; for (auto& [relTableID, relTableSchema] : catalogContent->getRelTableSchemas()) { if (relTableSchema->edgeContainsNodeTable(nodeTableID)) { relTablesToInit.push_back(storageManager.getRelsStore().getRelTable(relTableID)); } } - auto outDataPos = mapperContext.getDataPos(nodeAndPrimaryKey->node->getIDProperty()); + auto outDataPos = mapperContext.getDataPos(node->getIDProperty()); createNodeInfos.push_back(make_unique( table, std::move(primaryKeyEvaluator), relTablesToInit, outDataPos)); } diff --git a/src/processor/mapper/map_delete.cpp b/src/processor/mapper/map_delete.cpp index 1f89de8b44..90c37e6cac 100644 --- a/src/processor/mapper/map_delete.cpp +++ b/src/processor/mapper/map_delete.cpp @@ -7,27 +7,40 @@ namespace kuzu { namespace processor { -unique_ptr PlanMapper::mapLogicalDeleteToPhysical( +unique_ptr PlanMapper::mapLogicalDeleteNodeToPhysical( LogicalOperator* logicalOperator, MapperContext& mapperContext) { - auto& logicalDelete = (LogicalDelete&)*logicalOperator; + auto logicalDeleteNode = (LogicalDeleteNode*)logicalOperator; auto prevOperator = mapLogicalOperatorToPhysical(logicalOperator->getChild(0), mapperContext); auto& nodesStore = storageManager.getNodesStore(); - vector nodeIDVectorPositions; - vector primaryKeyVectorPositions; - vector nodeTables; - for (auto i = 0u; i < logicalDelete.getNumExpressions(); ++i) { - auto& nodeExpression = (NodeExpression&)*logicalDelete.getNodeExpression(i); - auto nodeIDExpression = nodeExpression.getNodeIDPropertyExpression(); - nodeIDVectorPositions.push_back( - mapperContext.getDataPos(nodeIDExpression->getUniqueName())); - nodeTables.push_back(nodesStore.getNodeTable(nodeExpression.getTableID())); - auto primaryKeyExpression = logicalDelete.getPrimaryKeyExpression(i); - primaryKeyVectorPositions.push_back( - mapperContext.getDataPos(primaryKeyExpression->getUniqueName())); + vector> deleteNodeInfos; + for (auto& [node, primaryKey] : logicalDeleteNode->getNodeAndPrimaryKeys()) { + auto nodeTable = nodesStore.getNodeTable(node->getTableID()); + auto nodeIDPos = mapperContext.getDataPos(node->getIDProperty()); + auto primaryKeyPos = mapperContext.getDataPos(primaryKey->getUniqueName()); + deleteNodeInfos.push_back(make_unique(nodeTable, nodeIDPos, primaryKeyPos)); } - return make_unique(move(nodeIDVectorPositions), - move(primaryKeyVectorPositions), move(nodeTables), move(prevOperator), getOperatorID(), - logicalDelete.getExpressionsForPrinting()); + return make_unique(std::move(deleteNodeInfos), std::move(prevOperator), + getOperatorID(), logicalDeleteNode->getExpressionsForPrinting()); +} + +unique_ptr PlanMapper::mapLogicalDeleteRelToPhysical( + LogicalOperator* logicalOperator, MapperContext& mapperContext) { + auto logicalDeleteRel = (LogicalDeleteRel*)logicalOperator; + auto prevOperator = mapLogicalOperatorToPhysical(logicalOperator->getChild(0), mapperContext); + auto& relStore = storageManager.getRelsStore(); + vector> createRelInfos; + for (auto i = 0u; i < logicalDeleteRel->getNumRels(); ++i) { + auto rel = logicalDeleteRel->getRel(i); + auto table = relStore.getRelTable(rel->getTableID()); + auto srcNodePos = mapperContext.getDataPos(rel->getSrcNode()->getIDProperty()); + auto srcNodeTableID = rel->getSrcNode()->getTableID(); + auto dstNodePos = mapperContext.getDataPos(rel->getDstNode()->getIDProperty()); + auto dstNodeTableID = rel->getDstNode()->getTableID(); + createRelInfos.push_back(make_unique( + table, srcNodePos, srcNodeTableID, dstNodePos, dstNodeTableID)); + } + return make_unique(relStore.getRelsStatistics(), std::move(createRelInfos), + std::move(prevOperator), getOperatorID(), logicalOperator->getExpressionsForPrinting()); } } // namespace processor diff --git a/src/processor/mapper/plan_mapper.cpp b/src/processor/mapper/plan_mapper.cpp index 5bc2b368a4..003a7c844e 100644 --- a/src/processor/mapper/plan_mapper.cpp +++ b/src/processor/mapper/plan_mapper.cpp @@ -106,8 +106,11 @@ unique_ptr PlanMapper::mapLogicalOperatorToPhysical( case LOGICAL_SET_NODE_PROPERTY: { physicalOperator = mapLogicalSetToPhysical(logicalOperator.get(), mapperContext); } break; - case LOGICAL_DELETE: { - physicalOperator = mapLogicalDeleteToPhysical(logicalOperator.get(), mapperContext); + case LOGICAL_DELETE_NODE: { + physicalOperator = mapLogicalDeleteNodeToPhysical(logicalOperator.get(), mapperContext); + } break; + case LOGICAL_DELETE_REL: { + physicalOperator = mapLogicalDeleteRelToPhysical(logicalOperator.get(), mapperContext); } break; case LOGICAL_CREATE_NODE_TABLE: { physicalOperator = diff --git a/src/processor/operator/include/physical_operator.h b/src/processor/operator/include/physical_operator.h index 740c349f98..3e831f8f04 100644 --- a/src/processor/operator/include/physical_operator.h +++ b/src/processor/operator/include/physical_operator.h @@ -20,7 +20,8 @@ enum PhysicalOperatorType : uint8_t { CREATE_REL, CREATE_REL_TABLE, CROSS_PRODUCT, - DELETE, + DELETE_NODE, + DELETE_REL, DROP_TABLE, EXISTS, FACTORIZED_TABLE_SCAN, @@ -53,12 +54,12 @@ enum PhysicalOperatorType : uint8_t { const string PhysicalOperatorTypeNames[] = {"AGGREGATE", "AGGREGATE_SCAN", "COLUMN_EXTEND", "COPY_NODE_CSV", "COPY_REL_CSV", "CREATE_NODE", "CREATE_NODE_TABLE", "CREATE_REL", - "CREATE_REL_TABLE", "CROSS_PRODUCT", "DELETE", "DROP_TABLE", "EXISTS", "FACTORIZED_TABLE_SCAN", - "FILTER", "FLATTEN", "HASH_JOIN_BUILD", "HASH_JOIN_PROBE", "INDEX_SCAN", "INTERSECT_BUILD", - "INTERSECT", "LIMIT", "LIST_EXTEND", "MULTIPLICITY_REDUCER", "PROJECTION", "SCAN_REL_PROPERTY", - "RESULT_COLLECTOR", "SCAN_NODE_ID", "SCAN_STRUCTURED_PROPERTY", "SEMI_MASKER", - "SET_STRUCTURED_NODE_PROPERTY", "SKIP", "ORDER_BY", "ORDER_BY_MERGE", "ORDER_BY_SCAN", - "UNION_ALL_SCAN", "UNWIND"}; + "CREATE_REL_TABLE", "CROSS_PRODUCT", "DELETE_NODE", "DELETE_REL", "DROP_TABLE", "EXISTS", + "FACTORIZED_TABLE_SCAN", "FILTER", "FLATTEN", "HASH_JOIN_BUILD", "HASH_JOIN_PROBE", + "INDEX_SCAN", "INTERSECT_BUILD", "INTERSECT", "LIMIT", "LIST_EXTEND", "MULTIPLICITY_REDUCER", + "PROJECTION", "SCAN_REL_PROPERTY", "RESULT_COLLECTOR", "SCAN_NODE_ID", + "SCAN_STRUCTURED_PROPERTY", "SEMI_MASKER", "SET_STRUCTURED_NODE_PROPERTY", "SKIP", "ORDER_BY", + "ORDER_BY_MERGE", "ORDER_BY_SCAN", "UNION_ALL_SCAN", "UNWIND"}; struct OperatorMetrics { diff --git a/src/processor/operator/update/delete.cpp b/src/processor/operator/update/delete.cpp index 056c9ec805..79836aafa7 100644 --- a/src/processor/operator/update/delete.cpp +++ b/src/processor/operator/update/delete.cpp @@ -3,32 +3,66 @@ namespace kuzu { namespace processor { -shared_ptr DeleteNodeStructuredProperty::init(ExecutionContext* context) { +shared_ptr DeleteNode::init(ExecutionContext* context) { resultSet = PhysicalOperator::init(context); - for (auto& vectorPos : nodeIDVectorPositions) { - auto dataChunk = resultSet->dataChunks[vectorPos.dataChunkPos]; - nodeIDVectors.push_back(dataChunk->valueVectors[vectorPos.valueVectorPos].get()); - } - for (auto& vectorPos : primaryKeyVectorPositions) { - auto dataChunk = resultSet->dataChunks[vectorPos.dataChunkPos]; - primaryKeyVectors.push_back(dataChunk->valueVectors[vectorPos.valueVectorPos].get()); + for (auto& deleteNodeInfo : deleteNodeInfos) { + auto nodeIDPos = deleteNodeInfo->nodeIDPos; + auto nodeIDVector = + resultSet->dataChunks[nodeIDPos.dataChunkPos]->valueVectors[nodeIDPos.valueVectorPos]; + nodeIDVectors.push_back(nodeIDVector.get()); + auto pkPos = deleteNodeInfo->primaryKeyPos; + auto pkVector = + resultSet->dataChunks[pkPos.dataChunkPos]->valueVectors[pkPos.valueVectorPos]; + primaryKeyVectors.push_back(pkVector.get()); } return resultSet; } -bool DeleteNodeStructuredProperty::getNextTuples() { +bool DeleteNode::getNextTuples() { metrics->executionTime.start(); if (!children[0]->getNextTuples()) { metrics->executionTime.stop(); return false; } - for (auto i = 0u; i < nodeTables.size(); ++i) { - auto nodeTable = nodeTables[i]; + for (auto i = 0u; i < deleteNodeInfos.size(); ++i) { + auto nodeTable = deleteNodeInfos[i]->table; nodeTable->deleteNodes(nodeIDVectors[i], primaryKeyVectors[i]); } metrics->executionTime.stop(); return true; } +shared_ptr DeleteRel::init(ExecutionContext* context) { + resultSet = PhysicalOperator::init(context); + for (auto& deleteRelInfo : deleteRelInfos) { + auto srcNodePos = deleteRelInfo->srcNodePos; + auto srcNodeIDVector = + resultSet->dataChunks[srcNodePos.dataChunkPos]->valueVectors[srcNodePos.valueVectorPos]; + auto dstNodePos = deleteRelInfo->dstNodePos; + auto dstNodeIDVector = + resultSet->dataChunks[dstNodePos.dataChunkPos]->valueVectors[dstNodePos.valueVectorPos]; + srcDstNodeIDVectorPairs.emplace_back(srcNodeIDVector.get(), dstNodeIDVector.get()); + } + return resultSet; +} + +bool DeleteRel::getNextTuples() { + metrics->executionTime.start(); + if (!children[0]->getNextTuples()) { + metrics->executionTime.stop(); + return false; + } + for (auto i = 0u; i < deleteRelInfos.size(); ++i) { + auto createRelInfo = deleteRelInfos[i].get(); + auto [srcNodeIDVector, dstNodeIDVector] = srcDstNodeIDVectorPairs[i]; + // TODO(Ziyi): you need to update rel statistics here. Though I don't think this is best + // design, statistics update should be hidden inside deleteRel(). See how node create/delete + // works. You can discuss this with Semih and if you decide to change, change createRel too. + createRelInfo->table->deleteRel(srcNodeIDVector, dstNodeIDVector); + } + metrics->executionTime.stop(); + return true; +} + } // namespace processor } // namespace kuzu diff --git a/src/processor/operator/update/include/delete.h b/src/processor/operator/update/include/delete.h index 08c851ad23..38c1b4a01e 100644 --- a/src/processor/operator/update/include/delete.h +++ b/src/processor/operator/update/include/delete.h @@ -6,35 +6,97 @@ namespace kuzu { namespace processor { -class DeleteNodeStructuredProperty : public PhysicalOperator { +struct DeleteNodeInfo { + NodeTable* table; + DataPos nodeIDPos; + DataPos primaryKeyPos; + + DeleteNodeInfo(NodeTable* table, const DataPos& nodeIDPos, const DataPos& primaryKeyPos) + : table{table}, nodeIDPos{nodeIDPos}, primaryKeyPos{primaryKeyPos} {} + + inline unique_ptr clone() { + return make_unique(table, nodeIDPos, primaryKeyPos); + } +}; + +class DeleteNode : public PhysicalOperator { public: - DeleteNodeStructuredProperty(vector nodeIDVectorPositions, - vector primaryKeyVectorPositions, vector nodeTables, + DeleteNode(vector> deleteNodeInfos, unique_ptr child, uint32_t id, const string& paramsString) - : PhysicalOperator{move(child), id, paramsString}, nodeIDVectorPositions{move( - nodeIDVectorPositions)}, - primaryKeyVectorPositions{move(primaryKeyVectorPositions)}, nodeTables{move(nodeTables)} { - } + : PhysicalOperator{std::move(child), id, paramsString}, deleteNodeInfos{ + std::move(deleteNodeInfos)} {} - PhysicalOperatorType getOperatorType() override { return PhysicalOperatorType::DELETE; } + inline PhysicalOperatorType getOperatorType() override { + return PhysicalOperatorType::DELETE_NODE; + } shared_ptr init(ExecutionContext* context) override; bool getNextTuples() override; - unique_ptr clone() override { - return make_unique(nodeIDVectorPositions, - primaryKeyVectorPositions, nodeTables, children[0]->clone(), id, paramsString); + inline unique_ptr clone() override { + vector> clonedDeleteNodeInfos; + for (auto& deleteNodeInfo : deleteNodeInfos) { + clonedDeleteNodeInfos.push_back(deleteNodeInfo->clone()); + } + return make_unique( + std::move(clonedDeleteNodeInfos), children[0]->clone(), id, paramsString); } private: - vector nodeIDVectorPositions; - vector primaryKeyVectorPositions; - vector nodeTables; + vector> deleteNodeInfos; vector nodeIDVectors; vector primaryKeyVectors; }; +struct DeleteRelInfo { + RelTable* table; + DataPos srcNodePos; + table_id_t srcNodeTableID; + DataPos dstNodePos; + table_id_t dstNodeTableID; + + DeleteRelInfo(RelTable* table, const DataPos& srcNodePos, table_id_t srcNodeTableID, + const DataPos& dstNodePos, table_id_t dstNodeTableID) + : table{table}, srcNodePos{srcNodePos}, srcNodeTableID{srcNodeTableID}, + dstNodePos{dstNodePos}, dstNodeTableID{dstNodeTableID} {} + + inline unique_ptr clone() { + return make_unique( + table, srcNodePos, srcNodeTableID, dstNodePos, dstNodeTableID); + } +}; + +class DeleteRel : public PhysicalOperator { +public: + DeleteRel(RelsStatistics& relsStatistics, vector> deleteRelInfos, + unique_ptr child, uint32_t id, const string& paramsString) + : PhysicalOperator{std::move(child), id, paramsString}, relsStatistics{relsStatistics}, + deleteRelInfos{std::move(deleteRelInfos)} {} + + inline PhysicalOperatorType getOperatorType() override { + return PhysicalOperatorType::DELETE_REL; + } + + shared_ptr init(ExecutionContext* context) override; + + bool getNextTuples() override; + + unique_ptr clone() override { + vector> clonedDeleteRelInfos; + for (auto& deleteRelInfo : deleteRelInfos) { + clonedDeleteRelInfos.push_back(deleteRelInfo->clone()); + } + return make_unique(relsStatistics, std::move(clonedDeleteRelInfos), + children[0]->clone(), id, paramsString); + } + +private: + RelsStatistics& relsStatistics; + vector> deleteRelInfos; + vector> srcDstNodeIDVectorPairs; +}; + } // namespace processor } // namespace kuzu diff --git a/src/storage/store/include/rel_table.h b/src/storage/store/include/rel_table.h index b90ef50cb3..de9a3928ee 100644 --- a/src/storage/store/include/rel_table.h +++ b/src/storage/store/include/rel_table.h @@ -50,6 +50,13 @@ class RelTable { void insertRels(shared_ptr& srcNodeIDVector, shared_ptr& dstNodeIDVector, vector>& relPropertyVectors); + // TODO(Ziyi): Let's also use raw pointer for insert interface and I think this should be a + // general principal when writing storage APIs. There is no good reason to give unique/shared + // ptr from processor to storage. + inline void deleteRel(ValueVector* srcNodeVector, ValueVector* dstNodeVector) { + assert(srcNodeVector->state->isFlat()); + assert(dstNodeVector->state->isFlat()); + } void initEmptyRelsForNewNode(nodeID_t& nodeID); private: diff --git a/test/binder/binder_error_test.cpp b/test/binder/binder_error_test.cpp index 28127b798c..70a8dba501 100644 --- a/test/binder/binder_error_test.cpp +++ b/test/binder/binder_error_test.cpp @@ -262,13 +262,6 @@ TEST_F(BinderErrorTest, ReadAfterUpdate1) { ASSERT_STREQ(expectedException.c_str(), getBindingError(input).c_str()); } -TEST_F(BinderErrorTest, ReadAfterUpdate2) { - string expectedException = "Binder exception: Read after update is not supported."; - auto input = "MATCH (a:person) WHERE a.age = 35 DELETE a WITH a MATCH (a)-[:knows]->(b:person) " - "RETURN a.age"; - ASSERT_STREQ(expectedException.c_str(), getBindingError(input).c_str()); -} - TEST_F(BinderErrorTest, ReadAfterUpdate3) { string expectedException = "Binder exception: Return/With after update is not supported."; auto input = "MATCH (a:person) SET a.age=3 RETURN a.name"; diff --git a/test/runner/e2e_exception_test.cpp b/test/runner/e2e_exception_test.cpp index 6d1e711824..2aa83c968e 100644 --- a/test/runner/e2e_exception_test.cpp +++ b/test/runner/e2e_exception_test.cpp @@ -191,3 +191,11 @@ TEST_F(TinySnbExceptionTest, EmptyQuery) { auto result = conn->query(""); ASSERT_STREQ(result->getErrorMessage().c_str(), "Connection Exception: Query is empty."); } + +TEST_F(TinySnbExceptionTest, ReadAfterUpdate2) { + auto result = conn->query( + "MATCH (a:person) WHERE a.age = 35 DELETE a WITH a MATCH (a)-[:knows]->(b:person) " + "RETURN a.age"); + ASSERT_STREQ( + result->getErrorMessage().c_str(), "Binder exception: Read after update is not supported."); +}