diff --git a/regress/expected/cypher_create.out b/regress/expected/cypher_create.out index 2388af8a0..efb77ed59 100644 --- a/regress/expected/cypher_create.out +++ b/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 -- diff --git a/regress/expected/cypher_set.out b/regress/expected/cypher_set.out index 239234ed6..b37d3a45f 100644 --- a/regress/expected/cypher_set.out +++ b/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 -- diff --git a/regress/sql/cypher_create.sql b/regress/sql/cypher_create.sql index 0093dc449..38dbb083f 100644 --- a/regress/sql/cypher_create.sql +++ b/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 -- diff --git a/regress/sql/cypher_set.sql b/regress/sql/cypher_set.sql index e745d5d6e..4a715a51d 100644 --- a/regress/sql/cypher_set.sql +++ b/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 -- diff --git a/src/backend/executor/cypher_create.c b/src/backend/executor/cypher_create.c index 36ef61b32..64a6edb98 100644 --- a/src/backend/executor/cypher_create.c +++ b/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; } diff --git a/src/backend/executor/cypher_set.c b/src/backend/executor/cypher_set.c index 7a0d48f0c..9b6bf51c5 100644 --- a/src/backend/executor/cypher_set.c +++ b/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; }