Merge branch 'master' into forbid-merge-with-null-properties

src/query/context.hpp

@@ -35,6 +35,10 @@ enum class TransactionStatus {
   STARTED_ROLLBACK,
 };
 
+struct Scope {
+  bool in_merge{false};
+};
+
 struct EvaluationContext {
   /// Memory for allocations during evaluation of a *single* Pull call.
   ///
@@ -51,6 +55,7 @@ struct EvaluationContext {
   /// All counters generated by `counter` function, mutable because the function
   /// modifies the values
   mutable std::unordered_map<std::string, int64_t> counters{};
+  Scope scope{};
 };
 
 inline std::vector<storage::PropertyId> NamesToProperties(const std::vector<std::string> &property_names,

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);
     }
   }();
 
@@ -4317,6 +4334,9 @@ bool Merge::MergeCursor::Pull(Frame &frame, ExecutionContext &context) {
   OOMExceptionEnabler oom_exception;
   SCOPED_PROFILE_OP("Merge");
 
+  context.evaluation_context.scope.in_merge = true;
+  memgraph::utils::OnScopeExit merge_exit([&] { context.evaluation_context.scope.in_merge = false; });
+
   while (true) {
     if (pull_input_) {
       if (input_cursor_->Pull(frame, context)) {
@@ -4325,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

tests/gql_behave/tests/memgraph_V1/features/merge.feature

@@ -187,7 +187,7 @@ Feature: Merge feature
         Then the result should be:
             | r    |
             | [:X] |
- 
+
     Scenario: Merge relationship test02
         Given an empty graph
         And having executed:
@@ -417,4 +417,93 @@ Feature: Merge feature
             | ({a: 1}) |
             | ({a: 2}) |
             | ({a: 3)) |
- 
+
+    Scenario: Merge node with null property error
+        Given an empty graph
+        When executing query:
+            """
+            MERGE ({id: null})
+            """
+        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:
+            """
+            MERGE ()-[:TYPE {id:null}]->()
+            """
+        Then an error should be raised
+
+    Scenario: Merge node with unwind with null property error
+        Given an empty graph
+        When executing query:
+            """
+            UNWIND [1, 2, null, 3] as x
+            MERGE ({id: x})
+            """
+        Then an error should be raised
+
+    Scenario: Merge edge with unwind with null property error
+        Given an empty graph
+        When executing query:
+            """
+            UNWIND [1, 2, null, 3] as x
+            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

tests/gql_behave/tests/memgraph_V1_on_disk/features/merge.feature

@@ -417,3 +417,93 @@ Feature: Merge feature
             | ({a: 1}) |
             | ({a: 2}) |
             | ({a: 3)) |
+
+    Scenario: Merge node with null property error
+        Given an empty graph
+        When executing query:
+            """
+            MERGE ({id: null})
+            """
+        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:
+            """
+            MERGE ()-[:TYPE {id:null}]->()
+            """
+        Then an error should be raised
+
+    Scenario: Merge node with unwind with null property error
+        Given an empty graph
+        When executing query:
+            """
+            UNWIND [1, 2, null, 3] as x
+            MERGE ({id: x})
+            """
+        Then an error should be raised
+
+    Scenario: Merge edge with unwind with null property error
+        Given an empty graph
+        When executing query:
+            """
+            UNWIND [1, 2, null, 3] as x
+            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