From 4a2da5d3d0b765fa1a28e48c0b3def5eb0e60fa7 Mon Sep 17 00:00:00 2001 From: John Gemignani Date: Thu, 8 Aug 2024 10:00:13 -0700 Subject: [PATCH] Fix Issue 1907: SET on MERGE not storing edge properties (#2022) Fixed issue 1907: SET on MERGE not storing edge properties. This issue is the same as the following commit, except that it applies to edges. commit 99e7c625d97782849e39bca93daca07b35c60772 Author: John Gemignani Date: Wed Jul 6 16:26:14 2022 -0700 Edges were overlooked when fixing the vertices in that commit. This PR corrects that omission. Current regression tests weren't impacted. Added additional regression tests. --- regress/expected/cypher_merge.out | 80 +++++++++++++++++++++++++++++ regress/sql/cypher_merge.sql | 26 ++++++++++ src/backend/executor/cypher_merge.c | 41 ++++++++++++++- 3 files changed, 145 insertions(+), 2 deletions(-) diff --git a/regress/expected/cypher_merge.out b/regress/expected/cypher_merge.out index af5bd9ea4..238a4c472 100644 --- a/regress/expected/cypher_merge.out +++ b/regress/expected/cypher_merge.out @@ -1649,6 +1649,74 @@ SELECT * FROM cypher('issue_1709', $$ MATCH (u) DELETE u $$) AS (a agtype); --- (0 rows) +-- +-- Fix issue 1907: SET on MERGE not storing edge properties +-- +-- setup +SELECT * FROM create_graph('issue_1907'); +NOTICE: graph "issue_1907" has been created + create_graph +-------------- + +(1 row) + +SELECT * from cypher('issue_1907', $$ CREATE (n:Testnode {name: 'Test Node A'}) + RETURN n $$) as (n agtype); + n +--------------------------------------------------------------------------------------------- + {"id": 844424930131969, "label": "Testnode", "properties": {"name": "Test Node A"}}::vertex +(1 row) + +SELECT * from cypher('issue_1907', $$ CREATE (n:Testnode {name: 'Test Node B'}) + RETURN n $$) as (n agtype); + n +--------------------------------------------------------------------------------------------- + {"id": 844424930131970, "label": "Testnode", "properties": {"name": "Test Node B"}}::vertex +(1 row) + +SELECT * FROM cypher('issue_1907', $$ MATCH ()-[r]->() RETURN r $$) AS (r agtype); + r +--- +(0 rows) + +-- should return properties added +SELECT * FROM cypher('issue_1907', $$ MERGE (a {name: 'Test Node A'})-[r:RELATED_TO]->(b {name: 'Test Node B'}) + SET r = {property1: 'something', property2: 'else'} + RETURN r $$) AS (r agtype); + r +------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ + {"id": 1125899906842625, "label": "RELATED_TO", "end_id": 281474976710658, "start_id": 281474976710657, "properties": {"property1": "something", "property2": "else"}}::edge +(1 row) + +-- should return properties added +SELECT * FROM cypher('issue_1907', $$ MATCH ()-[r]->() RETURN r $$) AS (r agtype); + r +------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ + {"id": 1125899906842625, "label": "RELATED_TO", "end_id": 281474976710658, "start_id": 281474976710657, "properties": {"property1": "something", "property2": "else"}}::edge +(1 row) + +-- cleanup +SELECT * FROM cypher('issue_1907', $$ MATCH ()-[r]->() DELETE r $$) AS (r agtype); + r +--- +(0 rows) + +-- do it again, but a different way +SELECT * FROM cypher('issue_1907', $$ MERGE (a {name: 'Test Node A'})-[r:RELATED_TO]->(b {name: 'Test Node B'}) + SET r.property1 = 'something', r.property2 = 'else' + RETURN r $$) AS (r agtype); + r +------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ + {"id": 1125899906842626, "label": "RELATED_TO", "end_id": 281474976710660, "start_id": 281474976710659, "properties": {"property1": "something", "property2": "else"}}::edge +(1 row) + +-- should return properties added +SELECT * FROM cypher('issue_1907', $$ MATCH ()-[r]->() RETURN r $$) AS (r agtype); + r +------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ + {"id": 1125899906842626, "label": "RELATED_TO", "end_id": 281474976710660, "start_id": 281474976710659, "properties": {"property1": "something", "property2": "else"}}::edge +(1 row) + -- -- clean up graphs -- @@ -1670,6 +1738,18 @@ SELECT * FROM cypher('issue_1709', $$ MATCH (n) DETACH DELETE n $$) AS (a agtype -- -- delete graphs -- +SELECT drop_graph('issue_1907', true); +NOTICE: drop cascades to 4 other objects +DETAIL: drop cascades to table issue_1907._ag_label_vertex +drop cascades to table issue_1907._ag_label_edge +drop cascades to table issue_1907."Testnode" +drop cascades to table issue_1907."RELATED_TO" +NOTICE: graph "issue_1907" has been dropped + drop_graph +------------ + +(1 row) + SELECT drop_graph('cypher_merge', true); NOTICE: drop cascades to 19 other objects DETAIL: drop cascades to table cypher_merge._ag_label_vertex diff --git a/regress/sql/cypher_merge.sql b/regress/sql/cypher_merge.sql index 59efad1b7..02c9d21c2 100644 --- a/regress/sql/cypher_merge.sql +++ b/regress/sql/cypher_merge.sql @@ -760,6 +760,31 @@ SELECT * FROM cypher('issue_1709', $$ MATCH ()-[e]->() DELETE e $$) AS (a agtype -- clean up SELECT * FROM cypher('issue_1709', $$ MATCH (u) DELETE u $$) AS (a agtype); +-- +-- Fix issue 1907: SET on MERGE not storing edge properties +-- +-- setup +SELECT * FROM create_graph('issue_1907'); +SELECT * from cypher('issue_1907', $$ CREATE (n:Testnode {name: 'Test Node A'}) + RETURN n $$) as (n agtype); +SELECT * from cypher('issue_1907', $$ CREATE (n:Testnode {name: 'Test Node B'}) + RETURN n $$) as (n agtype); +SELECT * FROM cypher('issue_1907', $$ MATCH ()-[r]->() RETURN r $$) AS (r agtype); +-- should return properties added +SELECT * FROM cypher('issue_1907', $$ MERGE (a {name: 'Test Node A'})-[r:RELATED_TO]->(b {name: 'Test Node B'}) + SET r = {property1: 'something', property2: 'else'} + RETURN r $$) AS (r agtype); +-- should return properties added +SELECT * FROM cypher('issue_1907', $$ MATCH ()-[r]->() RETURN r $$) AS (r agtype); +-- cleanup +SELECT * FROM cypher('issue_1907', $$ MATCH ()-[r]->() DELETE r $$) AS (r agtype); +-- do it again, but a different way +SELECT * FROM cypher('issue_1907', $$ MERGE (a {name: 'Test Node A'})-[r:RELATED_TO]->(b {name: 'Test Node B'}) + SET r.property1 = 'something', r.property2 = 'else' + RETURN r $$) AS (r agtype); +-- should return properties added +SELECT * FROM cypher('issue_1907', $$ MATCH ()-[r]->() RETURN r $$) AS (r agtype); + -- -- clean up graphs -- @@ -770,6 +795,7 @@ SELECT * FROM cypher('issue_1709', $$ MATCH (n) DETACH DELETE n $$) AS (a agtype -- -- delete graphs -- +SELECT drop_graph('issue_1907', true); SELECT drop_graph('cypher_merge', true); SELECT drop_graph('issue_1630', true); SELECT drop_graph('issue_1691', true); diff --git a/src/backend/executor/cypher_merge.c b/src/backend/executor/cypher_merge.c index ccdd999a6..de5fd6192 100644 --- a/src/backend/executor/cypher_merge.c +++ b/src/backend/executor/cypher_merge.c @@ -1400,10 +1400,47 @@ static void merge_edge(cypher_merge_custom_scan_state *css, elemTupleSlot->tts_values[edge_tuple_properties] = prop; elemTupleSlot->tts_isnull[edge_tuple_properties] = isNull; - /* Insert the edge, if it is a new edge */ - if (should_insert) + /* + * Insert the new edge. + * + * Depending on the currentCommandId, we need to do this one of two + * different ways - + * + * 1) If they are equal, the currentCommandId hasn't been used for an + * update, or it hasn't been incremented after being used. In either + * case, we need to use the current one and then increment it so that + * the following commands will have visibility of this update. Note, + * it isn't our job to update the currentCommandId first and then do + * this check. + * + * 2) If they are not equal, the currentCommandId has been used and/or + * updated. In this case, we can't use it. Otherwise our update won't + * be visible to anything that follows, until the currentCommandId is + * updated again. Remember, visibility is, greater than but not equal + * to, the currentCommandID used for the update. So, in this case we + * need to use the original currentCommandId when begin_cypher_merge + * was initiated as everything under this instance of merge needs to + * be based off of that initial currentCommandId. This allows the + * following command to see the updates generated by this instance of + * merge. + */ + if (should_insert && + css->base_currentCommandId == GetCurrentCommandId(false)) { insert_entity_tuple(resultRelInfo, elemTupleSlot, estate); + + /* + * Increment the currentCommandId since we processed an update. We + * don't want to do this outside of this block because we don't want + * to inadvertently or unnecessarily update the commandCounterId of + * another command. + */ + CommandCounterIncrement(); + } + else if (should_insert) + { + insert_entity_tuple_cid(resultRelInfo, elemTupleSlot, estate, + css->base_currentCommandId); } /* restore the old result relation info */