diff --git a/src/query/interpret/eval.hpp b/src/query/interpret/eval.hpp index af72a0551..2a9fb289f 100644 --- a/src/query/interpret/eval.hpp +++ b/src/query/interpret/eval.hpp @@ -784,9 +784,6 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> { TypedValue Visit(PrimitiveLiteral &literal) override { // TODO: no need to evaluate constants, we can write it to frame in one // of the previous phases. - if (ctx_->scope.in_merge && literal.value_.IsNull()) { - throw QueryRuntimeException("Can't have null literal properties inside merge!"); - } return TypedValue(literal.value_, ctx_->memory); } diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 307456e1a..060fd3746 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -263,6 +263,15 @@ VertexAccessor &CreateLocalVertex(const NodeCreationInfo &node_info, Frame *fram properties.emplace(dba.NameToProperty(key), value); } } + if (context.evaluation_context.scope.in_merge) { + for (const auto &[k, v] : properties) { + if (v.IsNull()) { + throw QueryRuntimeException(fmt::format("Can't have null literal properties inside merge ({}.{})!", + node_info.symbol.name(), dba.PropertyToName(k))); + } + } + } + MultiPropsInitChecked(&new_node, properties); (*frame)[node_info.symbol] = new_node; @@ -346,7 +355,7 @@ CreateExpand::CreateExpandCursor::CreateExpandCursor(const CreateExpand &self, u namespace { EdgeAccessor CreateEdge(const EdgeCreationInfo &edge_info, DbAccessor *dba, VertexAccessor *from, VertexAccessor *to, - Frame *frame, ExpressionEvaluator *evaluator) { + Frame *frame, ExecutionContext &context, ExpressionEvaluator *evaluator) { auto maybe_edge = dba->InsertEdge(from, to, edge_info.edge_type); if (maybe_edge.HasValue()) { auto &edge = *maybe_edge; @@ -361,6 +370,14 @@ EdgeAccessor CreateEdge(const EdgeCreationInfo &edge_info, DbAccessor *dba, Vert properties.emplace(dba->NameToProperty(key), value); } } + if (context.evaluation_context.scope.in_merge) { + for (const auto &[k, v] : properties) { + if (v.IsNull()) { + throw QueryRuntimeException(fmt::format("Can't have null literal properties inside merge ({}.{})!", + edge_info.symbol.name(), dba->PropertyToName(k))); + } + } + } if (!properties.empty()) MultiPropsInitChecked(&edge, properties); (*frame)[edge_info.symbol] = edge; @@ -420,14 +437,14 @@ bool CreateExpand::CreateExpandCursor::Pull(Frame &frame, ExecutionContext &cont auto created_edge = [&] { switch (self_.edge_info_.direction) { case EdgeAtom::Direction::IN: - return CreateEdge(self_.edge_info_, dba, &v2, &v1, &frame, &evaluator); + return CreateEdge(self_.edge_info_, dba, &v2, &v1, &frame, context, &evaluator); case EdgeAtom::Direction::OUT: // in the case of an undirected CreateExpand we choose an arbitrary // direction. this is used in the MERGE clause // it is not allowed in the CREATE clause, and the semantic // checker needs to ensure it doesn't reach this point case EdgeAtom::Direction::BOTH: - return CreateEdge(self_.edge_info_, dba, &v1, &v2, &frame, &evaluator); + return CreateEdge(self_.edge_info_, dba, &v1, &v2, &frame, context, &evaluator); } }(); @@ -4328,9 +4345,10 @@ bool Merge::MergeCursor::Pull(Frame &frame, ExecutionContext &context) { // and merge_create (could have a Once at the beginning) merge_match_cursor_->Reset(); merge_create_cursor_->Reset(); - } else + } else { // input is exhausted, we're done return false; + } } // pull from the merge_match cursor diff --git a/tests/gql_behave/tests/memgraph_V1/features/merge.feature b/tests/gql_behave/tests/memgraph_V1/features/merge.feature index cd8ab4f2d..0fc9dc6d7 100644 --- a/tests/gql_behave/tests/memgraph_V1/features/merge.feature +++ b/tests/gql_behave/tests/memgraph_V1/features/merge.feature @@ -426,6 +426,14 @@ Feature: Merge feature """ Then an error should be raised + Scenario: Merge node with one null property error + Given an empty graph + When executing query: + """ + MERGE ({id2: 1, id: null}) + """ + Then an error should be raised + Scenario: Merge edge with null property error Given an empty graph When executing query: @@ -451,3 +459,51 @@ Feature: Merge feature MERGE ()-[:TYPE {id:x}]->() """ Then an error should be raised + + Scenario: Merge node with null in on match passes + Given an empty graph + And having executed: + """ + CREATE ({a: 1}) + """ + When executing query: + """ + MERGE (n {a: 1}) + ON MATCH SET n.prop = null + ON CREATE SET n.prop = null; + """ + Then the result should be empty + + Scenario: Merge edge with null in on match passes + Given an empty graph + And having executed: + """ + CREATE ()-[:TYPE {a: 1}]->() + """ + When executing query: + """ + MERGE ()-[r:TYPE {a: 1}]->() + ON MATCH SET r.prop = null + ON CREATE SET r.prop = null; + """ + Then the result should be empty + + Scenario: Merge node with null in on create passes + Given an empty graph + When executing query: + """ + MERGE (n {a: 1}) + ON MATCH SET n.prop = null + ON CREATE SET n.prop = null; + """ + Then the result should be empty + + Scenario: Merge edge with null in on create passes + Given an empty graph + When executing query: + """ + MERGE ()-[r:TYPE {a: 1}]->() + ON MATCH SET r.prop = null + ON CREATE SET r.prop = null; + """ + Then the result should be empty diff --git a/tests/gql_behave/tests/memgraph_V1_on_disk/features/merge.feature b/tests/gql_behave/tests/memgraph_V1_on_disk/features/merge.feature index cd8ab4f2d..0fc9dc6d7 100644 --- a/tests/gql_behave/tests/memgraph_V1_on_disk/features/merge.feature +++ b/tests/gql_behave/tests/memgraph_V1_on_disk/features/merge.feature @@ -426,6 +426,14 @@ Feature: Merge feature """ Then an error should be raised + Scenario: Merge node with one null property error + Given an empty graph + When executing query: + """ + MERGE ({id2: 1, id: null}) + """ + Then an error should be raised + Scenario: Merge edge with null property error Given an empty graph When executing query: @@ -451,3 +459,51 @@ Feature: Merge feature MERGE ()-[:TYPE {id:x}]->() """ Then an error should be raised + + Scenario: Merge node with null in on match passes + Given an empty graph + And having executed: + """ + CREATE ({a: 1}) + """ + When executing query: + """ + MERGE (n {a: 1}) + ON MATCH SET n.prop = null + ON CREATE SET n.prop = null; + """ + Then the result should be empty + + Scenario: Merge edge with null in on match passes + Given an empty graph + And having executed: + """ + CREATE ()-[:TYPE {a: 1}]->() + """ + When executing query: + """ + MERGE ()-[r:TYPE {a: 1}]->() + ON MATCH SET r.prop = null + ON CREATE SET r.prop = null; + """ + Then the result should be empty + + Scenario: Merge node with null in on create passes + Given an empty graph + When executing query: + """ + MERGE (n {a: 1}) + ON MATCH SET n.prop = null + ON CREATE SET n.prop = null; + """ + Then the result should be empty + + Scenario: Merge edge with null in on create passes + Given an empty graph + When executing query: + """ + MERGE ()-[r:TYPE {a: 1}]->() + ON MATCH SET r.prop = null + ON CREATE SET r.prop = null; + """ + Then the result should be empty