Fix MERGE variable reuse

Fixed the MERGE clause to allow for correct variable reuse in
both the transform and execution phase.

Fixed an incorrect usage in MATCH where a variable was compared
with pg_strcasecmp instead of strcmp.

Refactored some of the code using the volatile wrapper.

Verified and corrected old regression tests that were in error.

Added regression tests.

regress/expected/cypher_match.out

@@ -2174,6 +2174,15 @@ SELECT * FROM cypher('cypher_match', $$ CREATE () WITH * MATCH (x{n0:x.n1}) RETU
 ---
 (0 rows)
 
+-- these should fail due to multiple labels for a variable
+SELECT * FROM cypher('cypher_match', $$ MATCH p=(x)-[]->(x:R) RETURN p, x $$) AS (p agtype, x agtype);
+ERROR:  multiple labels for variable 'x' are not supported
+LINE 1: ...* FROM cypher('cypher_match', $$ MATCH p=(x)-[]->(x:R) RETUR...
+                                                             ^
+SELECT * FROM cypher('cypher_match', $$ MATCH p=(x:r)-[]->(x:R) RETURN p, x $$) AS (p agtype, x agtype);
+ERROR:  multiple labels for variable 'x' are not supported
+LINE 1: ...FROM cypher('cypher_match', $$ MATCH p=(x:r)-[]->(x:R) RETUR...
+                                                             ^
 --
 -- Clean up
 --

regress/sql/cypher_match.sql

@@ -977,6 +977,10 @@ SELECT * FROM cypher('cypher_match', $$ MATCH p=(a {name:a.name})-[u {relationsh
 
 SELECT * FROM cypher('cypher_match', $$ CREATE () WITH * MATCH (x{n0:x.n1}) RETURN 0 $$) as (a agtype);
 
+-- these should fail due to multiple labels for a variable
+SELECT * FROM cypher('cypher_match', $$ MATCH p=(x)-[]->(x:R) RETURN p, x $$) AS (p agtype, x agtype);
+SELECT * FROM cypher('cypher_match', $$ MATCH p=(x:r)-[]->(x:R) RETURN p, x $$) AS (p agtype, x agtype);
+
 --
 -- Clean up
 --

regress/sql/cypher_merge.sql

@@ -482,19 +482,48 @@ SELECT * FROM cypher('cypher_merge', $$ MATCH (n:node) RETURN n $$) AS (n agtype
 --
 -- Complex MERGE w/wo RETURN values
 --
--- These should each create a path, if it doesn't already exist.
--- TODO Until the issue with variable reuse of 'x' in MERGE is corrected,
---      these commands will each create a new path.
+-- The first one should create a path, the others should just return parts of it.
 SELECT * FROM cypher('cypher_merge', $$ MERGE ()-[:B]->(x:C)-[:E]->(x:C)<-[f:F]-(y:I) $$) AS (x agtype);
 SELECT * FROM cypher('cypher_merge', $$ MERGE ()-[:B]->(x:C)-[:E]->(x:C)<-[f:F]-(y:I) RETURN x $$) AS (x agtype);
 SELECT * FROM cypher('cypher_merge', $$ MERGE p=()-[:B]->(x:C)-[:E]->(x:C)<-[f:F]-(y:I) $$) AS (p agtype);
 SELECT * FROM cypher('cypher_merge', $$ MERGE p=()-[:B]->(x:C)-[:E]->(x:C)<-[f:F]-(y:I) RETURN p $$) AS (p agtype);
 SELECT * FROM cypher('cypher_merge', $$ MERGE p=()-[:B]->(x:C)-[:E]->(x:C)<-[f:F]-(y:I) RETURN p $$) AS (p agtype);
--- TODO This should only return 1 row, as the path should already exist.
---      However, we need to fix the variable reuse in MERGE. Until then,
---      this will always return 5 rows due to 'x' above not being the same node.
+
+-- This should only return 1 row, as the path should already exist.
 SELECT * FROM cypher('cypher_merge', $$ MATCH p=()-[:B]->(:C)-[:E]->(:C)<-[:F]-(:I) RETURN p $$) AS (p agtype);
 
+-- test variable reuse in MERGE - the first MERGE of each group should create,
+-- the second MERGE shouldn't.
+SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x:P)-[:E]->(x:P) RETURN p, x $$) AS (p agtype, x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE (x:P)-[:E]->(x:P) $$) AS (x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE (x:P)-[:E]->(x) $$) AS (x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x:P)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x:Q)-[:E]->(x:Q) RETURN p, x $$) AS (p agtype, x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE (x:Q)-[:E]->(x) $$) AS (x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE (x:Q)-[:E]->(x:Q) $$) AS (x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x:Q)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x:R)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x:R)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x:R)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x:R)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
+-- should return 4 rows
+SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
+-- should create 1 row
+SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x)-[:E1]->(x) RETURN p, x $$) AS (p agtype, x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x)-[:E1]->(x) RETURN p, x $$) AS (p agtype, x agtype);
+-- the following should fail due to multiple labels
+SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x)-[:E]->(x:R) RETURN p, x $$) AS (p agtype, x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x:r)-[:E]->(x:R) RETURN p, x $$) AS (p agtype, x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE (x)-[:E]->(x:R) $$) AS (x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE (x:r)-[:E]->(x:R) $$) AS (x agtype);
+-- the following should fail due to reuse issues
+SELECT * FROM cypher('cypher_merge', $$ MERGE (x:r)-[y:E]->(x)-[y]->(x) $$) AS (x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE (x:r)-[y:E]->(x)-[x]->(y) $$) AS (x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE (x:r)-[y:E]->(x)-[z:E]->(y) $$) AS (x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x:r)-[y:E]->(x)-[p]->(x) $$) AS (x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x:r)-[y:E]->(x)-[p:E]->(x) $$) AS (x agtype);
+SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x:r)-[y:E]->(p)-[x]->(y) $$) AS (x agtype);
+
 --clean up
 SELECT * FROM cypher('cypher_merge', $$MATCH (n) DETACH DELETE n $$) AS (a agtype);
 

src/backend/executor/cypher_create.c

@@ -129,6 +129,12 @@ static void begin_cypher_create(CustomScanState *node, EState *estate,
                 cypher_node->id_expr_state =
                     ExecInitExpr(cypher_node->id_expr, (PlanState *)node);
             }
+
+            if (cypher_node->prop_expr != NULL)
+            {
+                cypher_node->prop_expr_state = ExecInitExpr(cypher_node->prop_expr,
+                                                            (PlanState *)node);
+            }
         }
     }
 
@@ -140,7 +146,9 @@ static void begin_cypher_create(CustomScanState *node, EState *estate,
      * that have modified the command id.
      */
     if (estate->es_output_cid == 0)
+    {
         estate->es_output_cid = estate->es_snapshot->curcid;
+    }
 
     Increment_Estate_CommandId(estate);
 }
@@ -210,15 +218,19 @@ static TupleTableSlot *exec_cypher_create(CustomScanState *node)
         Decrement_Estate_CommandId(estate)
         slot = ExecProcNode(node->ss.ps.lefttree);
         Increment_Estate_CommandId(estate)
+
         /* break when there are no tuples */
         if (TupIsNull(slot))
         {
             break;
         }
+
         /* setup the scantuple that the process_pattern needs */
         econtext->ecxt_scantuple =
             node->ss.ps.lefttree->ps_ProjInfo->pi_exprContext->ecxt_scantuple;
+
         process_pattern(css);
+
         /*
          * This may not be necessary. If we have an empty pattern, nothing was
          * inserted and the current command Id was not used. So, only flag it
@@ -230,6 +242,7 @@ static TupleTableSlot *exec_cypher_create(CustomScanState *node)
             used = true;
         }
     } while (terminal);
+
     /*
      * If the current command Id wasn't used, nothing was inserted and we're
      * done.
@@ -238,8 +251,10 @@ static TupleTableSlot *exec_cypher_create(CustomScanState *node)
     {
         return NULL;
     }
+
     /* update the current command Id */
     CommandCounterIncrement();
+
     /* if this was a terminal CREATE just return NULL */
     if (terminal)
     {
@@ -256,19 +271,25 @@ static void end_cypher_create(CustomScanState *node)
         (cypher_create_custom_scan_state *)node;
     ListCell *lc;
 
+    // increment the command counter
+    CommandCounterIncrement();
+
     ExecEndNode(node->ss.ps.lefttree);
 
     foreach (lc, css->pattern)
     {
         cypher_create_path *path = lfirst(lc);
         ListCell *lc2;
+
         foreach (lc2, path->target_nodes)
         {
             cypher_target_node *cypher_node =
                 (cypher_target_node *)lfirst(lc2);
 
             if (!CYPHER_TARGET_NODE_INSERT_ENTITY(cypher_node->flags))
+            {
                 continue;
+            }
 
             // close all indices for the node
             ExecCloseIndices(cypher_node->resultRelInfo);
@@ -506,8 +527,7 @@ static Datum create_vertex(cypher_create_custom_scan_state *css,
             scantuple = ps->ps_ExprContext->ecxt_scantuple;
 
             // make the vertex agtype
-            result = make_vertex(
-                id, CStringGetDatum(node->label_name),
+            result = make_vertex(id, CStringGetDatum(node->label_name),
                 PointerGetDatum(scanTupleSlot->tts_values[node->prop_attr_num]));
 
             // append to the path list
@@ -546,9 +566,11 @@ static Datum create_vertex(cypher_create_custom_scan_state *css,
         v = get_ith_agtype_value_from_container(&a->root, 0);
 
         if (v->type != AGTV_VERTEX)
+        {
             ereport(ERROR,
                     (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                      errmsg("agtype must resolve to a vertex")));
+        }
 
         // extract the id agtype field
         id_value = GET_AGTYPE_VALUE_OBJECT_VALUE(v, "id");
@@ -570,10 +592,12 @@ static Datum create_vertex(cypher_create_custom_scan_state *css,
         if (!SAFE_TO_SKIP_EXISTENCE_CHECK(node->flags))
         {
             if (!entity_exists(estate, css->graph_oid, DATUM_GET_GRAPHID(id)))
+            {
                 ereport(ERROR,
                     (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                      errmsg("vertex assigned to variable %s was deleted",
                             node->variable_name)));
+            }
         }
 
         if (CYPHER_TARGET_NODE_IN_PATH(node->flags))

src/backend/executor/cypher_merge.c

@@ -198,27 +198,25 @@ static bool check_path(cypher_merge_custom_scan_state *css,
              */
             if (slot->tts_isnull[node->tuple_position - 1])
             {
+
                 return true;
             }
         }
-
     }
-
     return false;
 }
 
 static void process_path(cypher_merge_custom_scan_state *css)
 {
     cypher_create_path *path = css->path;
-
     ListCell *lc = list_head(path->target_nodes);
 
     /*
      * Create the first vertex. The create_vertex function will
      * create the rest of the path, if necessary.
      */
-    merge_vertex(css, lfirst(lc), lnext(lc));
 
+    merge_vertex(css, lfirst(lc), lnext(lc));
 
     /*
      * If this path is a variable, take the list that was accumulated
@@ -272,9 +270,13 @@ static void process_simple_merge(CustomScanState *node)
     if (TupIsNull(slot))
     {
         ExprContext *econtext = node->ss.ps.ps_ExprContext;
+        SubqueryScanState *sss = (SubqueryScanState *)node->ss.ps.lefttree;
+
+        /* our child execution node should be a subquery */
+        Assert(IsA(sss, SubqueryScanState));
 
         /* setup the scantuple that the process_path needs */
-        econtext->ecxt_scantuple = node->ss.ps.lefttree->ps_ResultTupleSlot;
+        econtext->ecxt_scantuple = sss->ss.ss_ScanTupleSlot;
         econtext->ecxt_scantuple->tts_isempty = false;
 
         process_path(css);
@@ -334,7 +336,7 @@ static TupleTableSlot *exec_cypher_merge(CustomScanState *node)
         /*
          * Case 1: MERGE is not the first clause in the cypher query.
          *
-         * For this case, we need to process all tuples give to us by the
+         * For this case, we need to process all tuples given to us by the
          * previous clause. When we receive a tuple from the previous clause:
          * check to see if the left lateral join found the pattern already. If
          * it did, we don't need to create the pattern. If the lateral join did
@@ -377,7 +379,6 @@ static TupleTableSlot *exec_cypher_merge(CustomScanState *node)
             return NULL;
         }
 
-        //return ExecProject(node->ss.ps.ps_ProjInfo);
         econtext->ecxt_scantuple = ExecProject(node->ss.ps.lefttree->ps_ProjInfo);
         return ExecProject(node->ss.ps.ps_ProjInfo);
 
@@ -783,13 +784,9 @@ static Datum merge_vertex(cypher_merge_custom_scan_state *css,
         Datum d;
         agtype_value *v = NULL;
         agtype_value *id_value = NULL;
-        TupleTableSlot *scantuple = NULL;
-        PlanState *ps = NULL;
-
-        ps = css->css.ss.ps.lefttree;
-        scantuple = ps->ps_ExprContext->ecxt_scantuple;
 
-        if (scantuple->tts_isnull[node->tuple_position - 1])
+        /* check that the variable isn't NULL */
+        if (scanTupleSlot->tts_isnull[node->tuple_position - 1])
         {
             ereport(ERROR,
                     (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -798,7 +795,7 @@ static Datum merge_vertex(cypher_merge_custom_scan_state *css,
         }
 
         /* get the vertex agtype in the scanTupleSlot */
-        d = scantuple->tts_values[node->tuple_position - 1];
+        d = scanTupleSlot->tts_values[node->tuple_position - 1];
         a = DATUM_GET_AGTYPE_P(d);
 
         /* Convert to an agtype value */