Fix earlier clauses' writes being invisible to later clauses (#2493)

regress/expected/cypher_create.out

@@ -906,6 +906,115 @@ $$) as (m agtype);
  {"id": 281474976710689, "label": "", "properties": {}}::vertex
 (1 row)
 
+--
+-- Issue 2493: a clause that reads must see everything a preceding clause
+-- wrote, not only the rows written by that clause's first input row.
+--
+SELECT create_graph('issue_2493');
+NOTICE:  graph "issue_2493" has been created
+ create_graph 
+--------------
+
+(1 row)
+
+-- all three created vertices must be visible to the later MATCH
+SELECT * FROM cypher('issue_2493', $$
+    UNWIND [1, 2, 3] AS i
+    CREATE (:vis {id: i})
+    WITH count(*) AS ignored
+    MATCH (n:vis)
+    RETURN count(n)
+$$) as (visible agtype);
+ visible 
+---------
+ 3
+(1 row)
+
+-- pre-existing and newly created vertices are both visible
+SELECT * FROM cypher('issue_2493', $$ CREATE (:pre {id: 0}) $$) as (a agtype);
+ a 
+---
+(0 rows)
+
+SELECT * FROM cypher('issue_2493', $$
+    UNWIND [1, 2] AS i
+    CREATE (:pre {id: i})
+    WITH count(*) AS ignored
+    MATCH (n:pre)
+    RETURN count(n)
+$$) as (visible agtype);
+ visible 
+---------
+ 3
+(1 row)
+
+-- as originally reported: OPTIONAL MATCH binds every created vertex
+SELECT * FROM cypher('issue_2493', $$
+    UNWIND [1, 2, 3] AS i
+    CREATE (:opt {id: i})
+    WITH count(*) AS ignored
+    OPTIONAL MATCH (a:opt)
+    RETURN count(*) AS rows, count(a) AS bound
+$$) as (rows agtype, bound agtype);
+ rows | bound 
+------+-------
+ 3    | 3
+(1 row)
+
+-- vertices written earlier drive a later CREATE (6 ordered pairs of 3)
+SELECT * FROM cypher('issue_2493', $$
+    UNWIND [1, 2, 3] AS i
+    CREATE (:src {id: i})
+    WITH count(*) AS ignored
+    MATCH (a:src), (b:src) WHERE a.id <> b.id
+    CREATE (a)-[:rel]->(b)
+$$) as (a agtype);
+ a 
+---
+(0 rows)
+
+SELECT * FROM cypher('issue_2493', $$
+    MATCH ()-[r:rel]->() RETURN count(r)
+$$) as (edges agtype);
+ edges 
+-------
+ 6
+(1 row)
+
+-- a CREATE must still not see its own writes: this creates 3 vertices, not
+-- an unbounded number
+SELECT * FROM cypher('issue_2493', $$
+    MATCH (n:src)
+    CREATE (:copy {from: n.id})
+$$) as (a agtype);
+ a 
+---
+(0 rows)
+
+SELECT * FROM cypher('issue_2493', $$
+    MATCH (n:copy) RETURN count(n)
+$$) as (copies agtype);
+ copies 
+--------
+ 3
+(1 row)
+
+SELECT drop_graph('issue_2493', true);
+NOTICE:  drop cascades to 8 other objects
+DETAIL:  drop cascades to table issue_2493._ag_label_vertex
+drop cascades to table issue_2493._ag_label_edge
+drop cascades to table issue_2493.vis
+drop cascades to table issue_2493.pre
+drop cascades to table issue_2493.opt
+drop cascades to table issue_2493.src
+drop cascades to table issue_2493.rel
+drop cascades to table issue_2493.copy
+NOTICE:  graph "issue_2493" has been dropped
+ drop_graph 
+------------
+
+(1 row)
+
 --
 -- Clean up
 --

regress/expected/cypher_set.out

@@ -1227,6 +1227,96 @@ $$) AS (a agtype);
  {"id": 5066549580791809, "label": "TestE2", "properties": {"pathRels": [{"id": 5348024557502465, "label": "E2REL", "end_id": 5066549580791810, "start_id": 5066549580791809, "properties": {}}::edge], "pathNodes": [{"id": 5066549580791809, "label": "TestE2", "properties": {}}::vertex, {"id": 5066549580791810, "label": "TestE2", "properties": {}}::vertex]}}::vertex
 (1 row)
 
+--
+-- Issue 2493: a clause that reads must see every update a preceding SET
+-- made, not only the updates from that clause's first input row.
+--
+SELECT create_graph('issue_2493_set');
+NOTICE:  graph "issue_2493_set" has been created
+ create_graph 
+--------------
+
+(1 row)
+
+SELECT * FROM cypher('issue_2493_set', $$
+    UNWIND [1, 2, 3] AS i CREATE (:x {id: i})
+$$) as (a agtype);
+ a 
+---
+(0 rows)
+
+-- all three updates must be visible to the later MATCH
+SELECT * FROM cypher('issue_2493_set', $$
+    MATCH (n:x) SET n.marked = true
+    WITH count(*) AS ignored
+    MATCH (m:x) WHERE m.marked = true
+    RETURN count(m)
+$$) as (visible agtype);
+ visible 
+---------
+ 3
+(1 row)
+
+-- the same for REMOVE, which shares the SET executor
+SELECT * FROM cypher('issue_2493_set', $$
+    MATCH (n:x) REMOVE n.marked
+    WITH count(*) AS ignored
+    MATCH (m:x) WHERE m.marked = true
+    RETURN count(m)
+$$) as (still_marked agtype);
+ still_marked 
+--------------
+ 0
+(1 row)
+
+-- updates written earlier drive a later CREATE
+SELECT * FROM cypher('issue_2493_set', $$
+    MATCH (n:x) SET n.ready = true
+    WITH count(*) AS ignored
+    MATCH (m:x) WHERE m.ready = true
+    CREATE (:derived {from: m.id})
+$$) as (a agtype);
+ a 
+---
+(0 rows)
+
+SELECT * FROM cypher('issue_2493_set', $$
+    MATCH (d:derived) RETURN count(d)
+$$) as (derived agtype);
+ derived 
+---------
+ 3
+(1 row)
+
+-- a SET must still not see its own writes: this updates each vertex once
+SELECT * FROM cypher('issue_2493_set', $$
+    MATCH (n:x) SET n.pass = 1 RETURN count(*)
+$$) as (updated agtype);
+ updated 
+---------
+ 3
+(1 row)
+
+SELECT * FROM cypher('issue_2493_set', $$
+    MATCH (m:x) WHERE m.pass = 1 RETURN count(m)
+$$) as (persisted agtype);
+ persisted 
+-----------
+ 3
+(1 row)
+
+SELECT drop_graph('issue_2493_set', true);
+NOTICE:  drop cascades to 4 other objects
+DETAIL:  drop cascades to table issue_2493_set._ag_label_vertex
+drop cascades to table issue_2493_set._ag_label_edge
+drop cascades to table issue_2493_set.x
+drop cascades to table issue_2493_set.derived
+NOTICE:  graph "issue_2493_set" has been dropped
+ drop_graph 
+------------
+
+(1 row)
+
 --
 -- Clean up
 --

regress/sql/cypher_create.sql

@@ -475,6 +475,67 @@ SELECT * FROM cypher('cypher_create', $$
   CREATE (n), (m) WITH n AS r CREATE (m) RETURN m
 $$) as (m agtype);
 
+--
+-- Issue 2493: a clause that reads must see everything a preceding clause
+-- wrote, not only the rows written by that clause's first input row.
+--
+SELECT create_graph('issue_2493');
+
+-- all three created vertices must be visible to the later MATCH
+SELECT * FROM cypher('issue_2493', $$
+    UNWIND [1, 2, 3] AS i
+    CREATE (:vis {id: i})
+    WITH count(*) AS ignored
+    MATCH (n:vis)
+    RETURN count(n)
+$$) as (visible agtype);
+
+-- pre-existing and newly created vertices are both visible
+SELECT * FROM cypher('issue_2493', $$ CREATE (:pre {id: 0}) $$) as (a agtype);
+
+SELECT * FROM cypher('issue_2493', $$
+    UNWIND [1, 2] AS i
+    CREATE (:pre {id: i})
+    WITH count(*) AS ignored
+    MATCH (n:pre)
+    RETURN count(n)
+$$) as (visible agtype);
+
+-- as originally reported: OPTIONAL MATCH binds every created vertex
+SELECT * FROM cypher('issue_2493', $$
+    UNWIND [1, 2, 3] AS i
+    CREATE (:opt {id: i})
+    WITH count(*) AS ignored
+    OPTIONAL MATCH (a:opt)
+    RETURN count(*) AS rows, count(a) AS bound
+$$) as (rows agtype, bound agtype);
+
+-- vertices written earlier drive a later CREATE (6 ordered pairs of 3)
+SELECT * FROM cypher('issue_2493', $$
+    UNWIND [1, 2, 3] AS i
+    CREATE (:src {id: i})
+    WITH count(*) AS ignored
+    MATCH (a:src), (b:src) WHERE a.id <> b.id
+    CREATE (a)-[:rel]->(b)
+$$) as (a agtype);
+
+SELECT * FROM cypher('issue_2493', $$
+    MATCH ()-[r:rel]->() RETURN count(r)
+$$) as (edges agtype);
+
+-- a CREATE must still not see its own writes: this creates 3 vertices, not
+-- an unbounded number
+SELECT * FROM cypher('issue_2493', $$
+    MATCH (n:src)
+    CREATE (:copy {from: n.id})
+$$) as (a agtype);
+
+SELECT * FROM cypher('issue_2493', $$
+    MATCH (n:copy) RETURN count(n)
+$$) as (copies agtype);
+
+SELECT drop_graph('issue_2493', true);
+
 --
 -- Clean up
 --

regress/sql/cypher_set.sql

@@ -542,6 +542,55 @@ SELECT * FROM cypher('issue_1884', $$
     RETURN a
 $$) AS (a agtype);
 
+--
+-- Issue 2493: a clause that reads must see every update a preceding SET
+-- made, not only the updates from that clause's first input row.
+--
+SELECT create_graph('issue_2493_set');
+
+SELECT * FROM cypher('issue_2493_set', $$
+    UNWIND [1, 2, 3] AS i CREATE (:x {id: i})
+$$) as (a agtype);
+
+-- all three updates must be visible to the later MATCH
+SELECT * FROM cypher('issue_2493_set', $$
+    MATCH (n:x) SET n.marked = true
+    WITH count(*) AS ignored
+    MATCH (m:x) WHERE m.marked = true
+    RETURN count(m)
+$$) as (visible agtype);
+
+-- the same for REMOVE, which shares the SET executor
+SELECT * FROM cypher('issue_2493_set', $$
+    MATCH (n:x) REMOVE n.marked
+    WITH count(*) AS ignored
+    MATCH (m:x) WHERE m.marked = true
+    RETURN count(m)
+$$) as (still_marked agtype);
+
+-- updates written earlier drive a later CREATE
+SELECT * FROM cypher('issue_2493_set', $$
+    MATCH (n:x) SET n.ready = true
+    WITH count(*) AS ignored
+    MATCH (m:x) WHERE m.ready = true
+    CREATE (:derived {from: m.id})
+$$) as (a agtype);
+
+SELECT * FROM cypher('issue_2493_set', $$
+    MATCH (d:derived) RETURN count(d)
+$$) as (derived agtype);
+
+-- a SET must still not see its own writes: this updates each vertex once
+SELECT * FROM cypher('issue_2493_set', $$
+    MATCH (n:x) SET n.pass = 1 RETURN count(*)
+$$) as (updated agtype);
+
+SELECT * FROM cypher('issue_2493_set', $$
+    MATCH (m:x) WHERE m.pass = 1 RETURN count(m)
+$$) as (persisted agtype);
+
+SELECT drop_graph('issue_2493_set', true);
+
 --
 -- Clean up
 --

src/backend/executor/cypher_create.c

@@ -244,6 +244,29 @@ static TupleTableSlot *exec_cypher_create(CustomScanState *node)
      */
     if (!used)
     {
+        /*
+         * This clause is finished. Make everything it wrote visible to the
+         * clauses that read after it.
+         *
+         * Entities are inserted with the global command id (see
+         * insert_entity_tuple), and CommandCounterIncrement() above advances
+         * that id once per input row. es_snapshot->curcid does not follow it,
+         * so without this it stays one step past the command id used by the
+         * first input row, and everything written by the remaining rows is
+         * invisible for the rest of the statement (issue #2493).
+         *
+         * Syncing here, rather than after each row, is what keeps the clause
+         * from seeing its own writes: by this point the subtree is exhausted,
+         * so raising curcid cannot feed a created row back into the pattern
+         * that created it.
+         *
+         * Max() because Increment_Estate_CommandId can push curcid above the
+         * global command id, and lowering it would hide tuples that are
+         * already visible.
+         */
+        estate->es_snapshot->curcid = Max(estate->es_snapshot->curcid,
+                                          GetCurrentCommandId(false));
+
         return NULL;
     }
 

src/backend/executor/cypher_set.c

@@ -883,6 +883,29 @@ static TupleTableSlot *exec_cypher_set(CustomScanState *node)
 
     if (TupIsNull(slot))
     {
+        /*
+         * This clause is finished. Make everything it wrote visible to the
+         * clauses that read after it.
+         *
+         * Updated tuples are written with the global command id (see the cid
+         * in update_entity_tuple), and CommandCounterIncrement() advances that
+         * id once per input row. es_snapshot->curcid does not follow it, so
+         * without this it stays one step past the command id used by the first
+         * input row, and every later row's update is invisible for the rest of
+         * the statement. Same defect as the CREATE path (issue #2493).
+         *
+         * Syncing here, rather than after each row, is what keeps the clause
+         * from seeing its own writes: by this point the subtree is exhausted,
+         * so raising curcid cannot feed an updated row back into the pattern
+         * that updated it.
+         *
+         * Max() because Increment_Estate_CommandId can push curcid above the
+         * global command id, and lowering it would hide tuples that are
+         * already visible.
+         */
+        estate->es_snapshot->curcid = Max(estate->es_snapshot->curcid,
+                                          GetCurrentCommandId(false));
+
         return NULL;
     }