Skip to content

Commit

Permalink
Fix Issue 1630: MERGE using array not working in some cases (apache#1645
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 8, 2024
1 parent 3aca517 commit 146484f
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 @@ -6352,6 +6352,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 @@ -6440,6 +6441,29 @@ transform_merge_make_lateral_join(cypher_parsestate *cpstate, Query *query,
query->targetList = list_concat(query->targetList,
make_target_list_from_join(pstate, 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 146484f

Please sign in to comment.