Skip to content

Commit 27268a1

Browse files
jrgemignanijrgemignani
committed
Fix Issue 1630: MERGE using array not working in some cases (apache#1636)
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.
1 parent 3aca517 commit 27268a1

3 files changed

Lines changed: 231 additions & 1 deletion

File tree

‎regress/expected/cypher_merge.out‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,12 +1263,141 @@ $$) as (a agtype);
12631263
---
12641264
(0 rows)
12651265

1266+
---
1267+
--- Issue 1630 MERGE using array not working in some cases
1268+
--
1269+
SELECT * FROM create_graph('issue_1630');
1270+
NOTICE: graph "issue_1630" has been created
1271+
create_graph
1272+
--------------
1273+
1274+
(1 row)
1275+
1276+
SELECT * FROM cypher('issue_1630', $$ MATCH (u) RETURN (u) $$) AS (u agtype);
1277+
u
1278+
---
1279+
(0 rows)
1280+
1281+
-- will it create it?
1282+
SELECT * FROM cypher('issue_1630',
1283+
$$ WITH ['jon', 'snow'] AS cols
1284+
MERGE (v:PERSION {first: cols[0], last: cols[1]})
1285+
RETURN v $$) AS (v agtype);
1286+
v
1287+
-----------------------------------------------------------------------------------------------------
1288+
{"id": 844424930131969, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex
1289+
(1 row)
1290+
1291+
-- will it retrieve it, if it exists?
1292+
SELECT * FROM cypher('issue_1630',
1293+
$$ WITH ['jon', 'snow'] AS cols
1294+
MERGE (v:PERSION {first: cols[0], last: cols[1]})
1295+
RETURN v $$) AS (v agtype);
1296+
v
1297+
-----------------------------------------------------------------------------------------------------
1298+
{"id": 844424930131969, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex
1299+
(1 row)
1300+
1301+
SELECT * FROM cypher('issue_1630', $$ MATCH (u) RETURN (u) $$) AS (u agtype);
1302+
u
1303+
-----------------------------------------------------------------------------------------------------
1304+
{"id": 844424930131969, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex
1305+
(1 row)
1306+
1307+
-- a whole array
1308+
SELECT * FROM cypher('issue_1630',
1309+
$$ WITH ['jon', 'snow'] AS cols
1310+
MERGE (v:PERSION {namea: cols})
1311+
RETURN v $$) AS (v agtype);
1312+
v
1313+
-----------------------------------------------------------------------------------------------
1314+
{"id": 844424930131970, "label": "PERSION", "properties": {"namea": ["jon", "snow"]}}::vertex
1315+
(1 row)
1316+
1317+
-- a whole object
1318+
SELECT * FROM cypher('issue_1630',
1319+
$$ WITH {first: 'jon', last: 'snow'} AS cols
1320+
MERGE (v:PERSION {nameo: cols})
1321+
RETURN v $$) AS (v agtype);
1322+
v
1323+
----------------------------------------------------------------------------------------------------------------
1324+
{"id": 844424930131971, "label": "PERSION", "properties": {"nameo": {"last": "snow", "first": "jon"}}}::vertex
1325+
(1 row)
1326+
1327+
-- delete them to start over
1328+
SELECT * FROM cypher('issue_1630', $$ MATCH (u) DELETE(u) $$) AS (u agtype);
1329+
u
1330+
---
1331+
(0 rows)
1332+
1333+
SELECT * FROM cypher('issue_1630',
1334+
$$ WITH {first: 'jon', last: 'snow'} AS cols
1335+
MERGE (v:PERSION {first: cols.first, last: cols.last})
1336+
RETURN v $$) AS (v agtype);
1337+
v
1338+
-----------------------------------------------------------------------------------------------------
1339+
{"id": 844424930131972, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex
1340+
(1 row)
1341+
1342+
-- delete them to start over
1343+
-- check pushing through a few clauses
1344+
SELECT * FROM cypher('issue_1630', $$ MATCH (u) DELETE(u) $$) AS (u agtype);
1345+
u
1346+
---
1347+
(0 rows)
1348+
1349+
SELECT * FROM cypher('issue_1630',
1350+
$$ WITH {first: 'jon', last: 'snow'} AS jonsnow
1351+
WITH jonsnow AS name
1352+
WITH name AS cols
1353+
MERGE (v:PERSION {first: cols.first, last: cols.last})
1354+
RETURN v $$) AS (v agtype);
1355+
v
1356+
-----------------------------------------------------------------------------------------------------
1357+
{"id": 844424930131973, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex
1358+
(1 row)
1359+
1360+
-- will it retrieve the one created?
1361+
SELECT * FROM cypher('issue_1630',
1362+
$$ WITH {first: 'jon', last: 'snow'} AS jonsnow
1363+
WITH jonsnow AS name
1364+
WITH name AS cols
1365+
MERGE (v:PERSION {first: cols.first, last: cols.last})
1366+
RETURN v $$) AS (v agtype);
1367+
v
1368+
-----------------------------------------------------------------------------------------------------
1369+
{"id": 844424930131973, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex
1370+
(1 row)
1371+
1372+
-- delete them to start over
1373+
-- check pushing through a few clauses and returning both vars
1374+
SELECT * FROM cypher('issue_1630', $$ MATCH (u) DELETE(u) $$) AS (u agtype);
1375+
u
1376+
---
1377+
(0 rows)
1378+
1379+
SELECT * FROM cypher('issue_1630',
1380+
$$ WITH {first: 'jon', last: 'snow'} AS jonsnow
1381+
WITH jonsnow AS name
1382+
WITH name AS cols
1383+
MERGE (v:PERSION {first: cols.first, last: cols.last})
1384+
RETURN v, cols $$) AS (v agtype, cols agtype);
1385+
v | cols
1386+
-----------------------------------------------------------------------------------------------------+----------------------------------
1387+
{"id": 844424930131974, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex | {"last": "snow", "first": "jon"}
1388+
(1 row)
1389+
12661390
--clean up
12671391
SELECT * FROM cypher('cypher_merge', $$MATCH (n) DETACH DELETE n $$) AS (a agtype);
12681392
a
12691393
---
12701394
(0 rows)
12711395

1396+
SELECT * FROM cypher('issue_1630', $$MATCH (n) DETACH DELETE n $$) AS (a agtype);
1397+
a
1398+
---
1399+
(0 rows)
1400+
12721401
/*
12731402
* Clean up graph
12741403
*/
@@ -1299,3 +1428,14 @@ NOTICE: graph "cypher_merge" has been dropped
12991428

13001429
(1 row)
13011430

1431+
SELECT drop_graph('issue_1630', true);
1432+
NOTICE: drop cascades to 3 other objects
1433+
DETAIL: drop cascades to table issue_1630._ag_label_vertex
1434+
drop cascades to table issue_1630._ag_label_edge
1435+
drop cascades to table issue_1630."PERSION"
1436+
NOTICE: graph "issue_1630" has been dropped
1437+
drop_graph
1438+
------------
1439+
1440+
(1 row)
1441+

‎regress/sql/cypher_merge.sql‎

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,11 +606,77 @@ SELECT * FROM cypher('cypher_merge', $$
606606
CREATE (n), (m) WITH n AS r MERGE (m)
607607
$$) as (a agtype);
608608

609+
---
610+
--- Issue 1630 MERGE using array not working in some cases
611+
--
612+
SELECT * FROM create_graph('issue_1630');
613+
SELECT * FROM cypher('issue_1630', $$ MATCH (u) RETURN (u) $$) AS (u agtype);
614+
-- will it create it?
615+
SELECT * FROM cypher('issue_1630',
616+
$$ WITH ['jon', 'snow'] AS cols
617+
MERGE (v:PERSION {first: cols[0], last: cols[1]})
618+
RETURN v $$) AS (v agtype);
619+
-- will it retrieve it, if it exists?
620+
SELECT * FROM cypher('issue_1630',
621+
$$ WITH ['jon', 'snow'] AS cols
622+
MERGE (v:PERSION {first: cols[0], last: cols[1]})
623+
RETURN v $$) AS (v agtype);
624+
SELECT * FROM cypher('issue_1630', $$ MATCH (u) RETURN (u) $$) AS (u agtype);
625+
626+
-- a whole array
627+
SELECT * FROM cypher('issue_1630',
628+
$$ WITH ['jon', 'snow'] AS cols
629+
MERGE (v:PERSION {namea: cols})
630+
RETURN v $$) AS (v agtype);
631+
632+
-- a whole object
633+
SELECT * FROM cypher('issue_1630',
634+
$$ WITH {first: 'jon', last: 'snow'} AS cols
635+
MERGE (v:PERSION {nameo: cols})
636+
RETURN v $$) AS (v agtype);
637+
638+
-- delete them to start over
639+
SELECT * FROM cypher('issue_1630', $$ MATCH (u) DELETE(u) $$) AS (u agtype);
640+
SELECT * FROM cypher('issue_1630',
641+
$$ WITH {first: 'jon', last: 'snow'} AS cols
642+
MERGE (v:PERSION {first: cols.first, last: cols.last})
643+
RETURN v $$) AS (v agtype);
644+
645+
-- delete them to start over
646+
-- check pushing through a few clauses
647+
SELECT * FROM cypher('issue_1630', $$ MATCH (u) DELETE(u) $$) AS (u agtype);
648+
SELECT * FROM cypher('issue_1630',
649+
$$ WITH {first: 'jon', last: 'snow'} AS jonsnow
650+
WITH jonsnow AS name
651+
WITH name AS cols
652+
MERGE (v:PERSION {first: cols.first, last: cols.last})
653+
RETURN v $$) AS (v agtype);
654+
655+
-- will it retrieve the one created?
656+
SELECT * FROM cypher('issue_1630',
657+
$$ WITH {first: 'jon', last: 'snow'} AS jonsnow
658+
WITH jonsnow AS name
659+
WITH name AS cols
660+
MERGE (v:PERSION {first: cols.first, last: cols.last})
661+
RETURN v $$) AS (v agtype);
662+
663+
-- delete them to start over
664+
-- check pushing through a few clauses and returning both vars
665+
SELECT * FROM cypher('issue_1630', $$ MATCH (u) DELETE(u) $$) AS (u agtype);
666+
SELECT * FROM cypher('issue_1630',
667+
$$ WITH {first: 'jon', last: 'snow'} AS jonsnow
668+
WITH jonsnow AS name
669+
WITH name AS cols
670+
MERGE (v:PERSION {first: cols.first, last: cols.last})
671+
RETURN v, cols $$) AS (v agtype, cols agtype);
672+
673+
609674
--clean up
610675
SELECT * FROM cypher('cypher_merge', $$MATCH (n) DETACH DELETE n $$) AS (a agtype);
676+
SELECT * FROM cypher('issue_1630', $$MATCH (n) DETACH DELETE n $$) AS (a agtype);
611677

612678
/*
613679
* Clean up graph
614680
*/
615681
SELECT drop_graph('cypher_merge', true);
616-
682+
SELECT drop_graph('issue_1630', true);

‎src/backend/parser/cypher_clause.c‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6352,6 +6352,7 @@ transform_merge_make_lateral_join(cypher_parsestate *cpstate, Query *query,
63526352
ParseExprKind tmp;
63536353
cypher_merge *self = (cypher_merge *)clause->self;
63546354
cypher_path *path;
6355+
ListCell *lc;
63556356

63566357
Assert(is_ag_node(self->path, cypher_path));
63576358

@@ -6440,6 +6441,29 @@ transform_merge_make_lateral_join(cypher_parsestate *cpstate, Query *query,
64406441
query->targetList = list_concat(query->targetList,
64416442
make_target_list_from_join(pstate, rte));
64426443

6444+
/*
6445+
* Iterate through the targetList and wrap all user defined variables with a
6446+
* volatile wrapper. This is necessary for allowing variables from previous
6447+
* clauses to not be removed by the function remove_unused_subquery_outputs.
6448+
* That function may replace variables, in place, with a NULL Const. We need
6449+
* to fix them so that it doesn't. NOTE: Our hidden, internal vars, are not
6450+
* wrapped.
6451+
*/
6452+
foreach(lc, query->targetList)
6453+
{
6454+
TargetEntry *qte = (TargetEntry *)lfirst(lc);
6455+
6456+
if (IsA(qte->expr, Var))
6457+
{
6458+
if (qte->resname != NULL &&
6459+
pg_strncasecmp(qte->resname, AGE_DEFAULT_VARNAME_PREFIX,
6460+
strlen(AGE_DEFAULT_VARNAME_PREFIX)))
6461+
{
6462+
qte->expr = add_volatile_wrapper(qte->expr);
6463+
}
6464+
}
6465+
}
6466+
64436467
/*
64446468
* For the metadata need to create paths, find the tuple position that
64456469
* will represent the entity in the execution phase.

0 commit comments

Comments
 (0)