From 4b60645beb846904dd12af93e2a5d2a0beebc39d Mon Sep 17 00:00:00 2001 From: John Gemignani Date: Mon, 4 Mar 2024 13:15:17 -0800 Subject: [PATCH] Fix Issue 1630: MERGE using array not working in some cases This PR fixes the issue where some previous clause user variables were not getting passed to the next clause. Basically, there is a function, remove_unused_subquery_outputs, that tries to remove unnecessary variables by replacing them, in place, with a NULL Const. As these variables are needed, we need to wrap them with the volatile wrapper to stop that function from removing them. Added regression tests. --- regress/expected/cypher_merge.out | 140 +++++++++++++++++++++++++++++ regress/sql/cypher_merge.sql | 68 +++++++++++++- src/backend/parser/cypher_clause.c | 24 +++++ 3 files changed, 231 insertions(+), 1 deletion(-) diff --git a/regress/expected/cypher_merge.out b/regress/expected/cypher_merge.out index 39690e7c9..7690405cf 100644 --- a/regress/expected/cypher_merge.out +++ b/regress/expected/cypher_merge.out @@ -1263,12 +1263,141 @@ $$) as (a agtype); --- (0 rows) +--- +--- Issue 1630 MERGE using array not working in some cases +-- +SELECT * FROM create_graph('issue_1630'); +NOTICE: graph "issue_1630" has been created + create_graph +-------------- + +(1 row) + +SELECT * FROM cypher('issue_1630', $$ MATCH (u) RETURN (u) $$) AS (u agtype); + u +--- +(0 rows) + +-- will it create it? +SELECT * FROM cypher('issue_1630', + $$ WITH ['jon', 'snow'] AS cols + MERGE (v:PERSION {first: cols[0], last: cols[1]}) + RETURN v $$) AS (v agtype); + v +----------------------------------------------------------------------------------------------------- + {"id": 844424930131969, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex +(1 row) + +-- will it retrieve it, if it exists? +SELECT * FROM cypher('issue_1630', + $$ WITH ['jon', 'snow'] AS cols + MERGE (v:PERSION {first: cols[0], last: cols[1]}) + RETURN v $$) AS (v agtype); + v +----------------------------------------------------------------------------------------------------- + {"id": 844424930131969, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex +(1 row) + +SELECT * FROM cypher('issue_1630', $$ MATCH (u) RETURN (u) $$) AS (u agtype); + u +----------------------------------------------------------------------------------------------------- + {"id": 844424930131969, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex +(1 row) + +-- a whole array +SELECT * FROM cypher('issue_1630', + $$ WITH ['jon', 'snow'] AS cols + MERGE (v:PERSION {namea: cols}) + RETURN v $$) AS (v agtype); + v +----------------------------------------------------------------------------------------------- + {"id": 844424930131970, "label": "PERSION", "properties": {"namea": ["jon", "snow"]}}::vertex +(1 row) + +-- a whole object +SELECT * FROM cypher('issue_1630', + $$ WITH {first: 'jon', last: 'snow'} AS cols + MERGE (v:PERSION {nameo: cols}) + RETURN v $$) AS (v agtype); + v +---------------------------------------------------------------------------------------------------------------- + {"id": 844424930131971, "label": "PERSION", "properties": {"nameo": {"last": "snow", "first": "jon"}}}::vertex +(1 row) + +-- delete them to start over +SELECT * FROM cypher('issue_1630', $$ MATCH (u) DELETE(u) $$) AS (u agtype); + u +--- +(0 rows) + +SELECT * FROM cypher('issue_1630', + $$ WITH {first: 'jon', last: 'snow'} AS cols + MERGE (v:PERSION {first: cols.first, last: cols.last}) + RETURN v $$) AS (v agtype); + v +----------------------------------------------------------------------------------------------------- + {"id": 844424930131972, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex +(1 row) + +-- delete them to start over +-- check pushing through a few clauses +SELECT * FROM cypher('issue_1630', $$ MATCH (u) DELETE(u) $$) AS (u agtype); + u +--- +(0 rows) + +SELECT * FROM cypher('issue_1630', + $$ WITH {first: 'jon', last: 'snow'} AS jonsnow + WITH jonsnow AS name + WITH name AS cols + MERGE (v:PERSION {first: cols.first, last: cols.last}) + RETURN v $$) AS (v agtype); + v +----------------------------------------------------------------------------------------------------- + {"id": 844424930131973, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex +(1 row) + +-- will it retrieve the one created? +SELECT * FROM cypher('issue_1630', + $$ WITH {first: 'jon', last: 'snow'} AS jonsnow + WITH jonsnow AS name + WITH name AS cols + MERGE (v:PERSION {first: cols.first, last: cols.last}) + RETURN v $$) AS (v agtype); + v +----------------------------------------------------------------------------------------------------- + {"id": 844424930131973, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex +(1 row) + +-- delete them to start over +-- check pushing through a few clauses and returning both vars +SELECT * FROM cypher('issue_1630', $$ MATCH (u) DELETE(u) $$) AS (u agtype); + u +--- +(0 rows) + +SELECT * FROM cypher('issue_1630', + $$ WITH {first: 'jon', last: 'snow'} AS jonsnow + WITH jonsnow AS name + WITH name AS cols + MERGE (v:PERSION {first: cols.first, last: cols.last}) + RETURN v, cols $$) AS (v agtype, cols agtype); + v | cols +-----------------------------------------------------------------------------------------------------+---------------------------------- + {"id": 844424930131974, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex | {"last": "snow", "first": "jon"} +(1 row) + --clean up SELECT * FROM cypher('cypher_merge', $$MATCH (n) DETACH DELETE n $$) AS (a agtype); a --- (0 rows) +SELECT * FROM cypher('issue_1630', $$MATCH (n) DETACH DELETE n $$) AS (a agtype); + a +--- +(0 rows) + /* * Clean up graph */ @@ -1299,3 +1428,14 @@ NOTICE: graph "cypher_merge" has been dropped (1 row) +SELECT drop_graph('issue_1630', true); +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to table issue_1630._ag_label_vertex +drop cascades to table issue_1630._ag_label_edge +drop cascades to table issue_1630."PERSION" +NOTICE: graph "issue_1630" has been dropped + drop_graph +------------ + +(1 row) + diff --git a/regress/sql/cypher_merge.sql b/regress/sql/cypher_merge.sql index f6aceea82..fe9ee8184 100644 --- a/regress/sql/cypher_merge.sql +++ b/regress/sql/cypher_merge.sql @@ -606,11 +606,77 @@ SELECT * FROM cypher('cypher_merge', $$ CREATE (n), (m) WITH n AS r MERGE (m) $$) as (a agtype); +--- +--- Issue 1630 MERGE using array not working in some cases +-- +SELECT * FROM create_graph('issue_1630'); +SELECT * FROM cypher('issue_1630', $$ MATCH (u) RETURN (u) $$) AS (u agtype); +-- will it create it? +SELECT * FROM cypher('issue_1630', + $$ WITH ['jon', 'snow'] AS cols + MERGE (v:PERSION {first: cols[0], last: cols[1]}) + RETURN v $$) AS (v agtype); +-- will it retrieve it, if it exists? +SELECT * FROM cypher('issue_1630', + $$ WITH ['jon', 'snow'] AS cols + MERGE (v:PERSION {first: cols[0], last: cols[1]}) + RETURN v $$) AS (v agtype); +SELECT * FROM cypher('issue_1630', $$ MATCH (u) RETURN (u) $$) AS (u agtype); + +-- a whole array +SELECT * FROM cypher('issue_1630', + $$ WITH ['jon', 'snow'] AS cols + MERGE (v:PERSION {namea: cols}) + RETURN v $$) AS (v agtype); + +-- a whole object +SELECT * FROM cypher('issue_1630', + $$ WITH {first: 'jon', last: 'snow'} AS cols + MERGE (v:PERSION {nameo: cols}) + RETURN v $$) AS (v agtype); + +-- delete them to start over +SELECT * FROM cypher('issue_1630', $$ MATCH (u) DELETE(u) $$) AS (u agtype); +SELECT * FROM cypher('issue_1630', + $$ WITH {first: 'jon', last: 'snow'} AS cols + MERGE (v:PERSION {first: cols.first, last: cols.last}) + RETURN v $$) AS (v agtype); + +-- delete them to start over +-- check pushing through a few clauses +SELECT * FROM cypher('issue_1630', $$ MATCH (u) DELETE(u) $$) AS (u agtype); +SELECT * FROM cypher('issue_1630', + $$ WITH {first: 'jon', last: 'snow'} AS jonsnow + WITH jonsnow AS name + WITH name AS cols + MERGE (v:PERSION {first: cols.first, last: cols.last}) + RETURN v $$) AS (v agtype); + +-- will it retrieve the one created? +SELECT * FROM cypher('issue_1630', + $$ WITH {first: 'jon', last: 'snow'} AS jonsnow + WITH jonsnow AS name + WITH name AS cols + MERGE (v:PERSION {first: cols.first, last: cols.last}) + RETURN v $$) AS (v agtype); + +-- delete them to start over +-- check pushing through a few clauses and returning both vars +SELECT * FROM cypher('issue_1630', $$ MATCH (u) DELETE(u) $$) AS (u agtype); +SELECT * FROM cypher('issue_1630', + $$ WITH {first: 'jon', last: 'snow'} AS jonsnow + WITH jonsnow AS name + WITH name AS cols + MERGE (v:PERSION {first: cols.first, last: cols.last}) + RETURN v, cols $$) AS (v agtype, cols agtype); + + --clean up SELECT * FROM cypher('cypher_merge', $$MATCH (n) DETACH DELETE n $$) AS (a agtype); +SELECT * FROM cypher('issue_1630', $$MATCH (n) DETACH DELETE n $$) AS (a agtype); /* * Clean up graph */ SELECT drop_graph('cypher_merge', true); - +SELECT drop_graph('issue_1630', true); diff --git a/src/backend/parser/cypher_clause.c b/src/backend/parser/cypher_clause.c index 105e0178c..6798f3d2a 100644 --- a/src/backend/parser/cypher_clause.c +++ b/src/backend/parser/cypher_clause.c @@ -6533,6 +6533,7 @@ transform_merge_make_lateral_join(cypher_parsestate *cpstate, Query *query, ParseExprKind tmp; cypher_merge *self = (cypher_merge *)clause->self; cypher_path *path; + ListCell *lc; Assert(is_ag_node(self->path, cypher_path)); @@ -6624,6 +6625,29 @@ transform_merge_make_lateral_join(cypher_parsestate *cpstate, Query *query, query->targetList = list_concat(query->targetList, make_target_list_from_join(pstate, jnsitem->p_rte)); + /* + * Iterate through the targetList and wrap all user defined variables with a + * volatile wrapper. This is necessary for allowing variables from previous + * clauses to not be removed by the function remove_unused_subquery_outputs. + * That function may replace variables, in place, with a NULL Const. We need + * to fix them so that it doesn't. NOTE: Our hidden, internal vars, are not + * wrapped. + */ + foreach(lc, query->targetList) + { + TargetEntry *qte = (TargetEntry *)lfirst(lc); + + if (IsA(qte->expr, Var)) + { + if (qte->resname != NULL && + pg_strncasecmp(qte->resname, AGE_DEFAULT_VARNAME_PREFIX, + strlen(AGE_DEFAULT_VARNAME_PREFIX))) + { + qte->expr = add_volatile_wrapper(qte->expr); + } + } + } + /* * For the metadata need to create paths, find the tuple position that * will represent the entity in the execution phase.