Skip to content

Commit

Permalink
Fix Issue 1630: MERGE using array not working in some cases (#1636)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jrgemignani
jrgemignani authored Mar 6, 2024
1 parent 3b2b394 commit 001b005
Show file tree
Hide file tree
Showing 3 changed files with 231 additions and 1 deletion.
140 changes: 140 additions & 0 deletions regress/expected/cypher_merge.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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)

68 changes: 67 additions & 1 deletion regress/sql/cypher_merge.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
24 changes: 24 additions & 0 deletions src/backend/parser/cypher_clause.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 001b005

Please sign in to comment.