Skip to content

Commit 40dddbd

Browse files
committed
Address Copilot review: pre-init ExprState, add ON MATCH SET predecessor test
- Move ExecInitExpr for ON CREATE/MATCH SET items from per-row execution in apply_update_list() to plan initialization in begin_cypher_merge(). Follows the established pattern used by cypher_target_node (id_expr_state, prop_expr_state). - Add prop_expr_state field to cypher_update_item with serialization support in outfuncs/readfuncs/copyfuncs. - apply_update_list() uses pre-initialized state when available, falls back to per-row init for plain SET callers. - Fix misleading comment: "ON MATCH SET" → "ON CREATE SET" for Case 1 first-run test. - Add Case 1 second-run test that triggers ON MATCH SET with a predecessor clause (MATCH ... MERGE ... ON MATCH SET).
1 parent 0fd8992 commit 40dddbd

File tree

9 files changed

+70
-4
lines changed

9 files changed

+70
-4
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ __pycache__
1515
**/apache_age_python.egg-info
1616

1717
drivers/python/build
18+
*.bc

regress/expected/cypher_merge.out

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1944,7 +1944,7 @@ $$) AS (name agtype, created agtype, matched agtype);
19441944
"Bob" | true | true
19451945
(1 row)
19461946

1947-
-- ON MATCH SET with MERGE after MATCH (Case 1: has predecessor)
1947+
-- ON CREATE SET with MERGE after MATCH (Case 1: has predecessor, first run = create)
19481948
SELECT * FROM cypher('merge_actions', $$
19491949
MATCH (a:Person {name: 'Alice'})
19501950
MERGE (a)-[:KNOWS]->(b:Person {name: 'Charlie'})
@@ -1956,6 +1956,18 @@ $$) AS (a agtype, b agtype, source agtype);
19561956
"Alice" | "Charlie" | "merge_create"
19571957
(1 row)
19581958

1959+
-- ON MATCH SET with MERGE after MATCH (Case 1: has predecessor, second run = match)
1960+
SELECT * FROM cypher('merge_actions', $$
1961+
MATCH (a:Person {name: 'Alice'})
1962+
MERGE (a)-[:KNOWS]->(b:Person {name: 'Charlie'})
1963+
ON MATCH SET b.visited = true
1964+
RETURN a.name, b.name, b.visited
1965+
$$) AS (a agtype, b agtype, visited agtype);
1966+
a | b | visited
1967+
---------+-----------+---------
1968+
"Alice" | "Charlie" | true
1969+
(1 row)
1970+
19591971
-- Multiple SET items in a single ON CREATE SET
19601972
SELECT * FROM cypher('merge_actions', $$
19611973
MERGE (n:Person {name: 'Dave'})

regress/sql/cypher_merge.sql

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -903,14 +903,22 @@ SELECT * FROM cypher('merge_actions', $$
903903
RETURN n.name, n.created, n.matched
904904
$$) AS (name agtype, created agtype, matched agtype);
905905

906-
-- ON MATCH SET with MERGE after MATCH (Case 1: has predecessor)
906+
-- ON CREATE SET with MERGE after MATCH (Case 1: has predecessor, first run = create)
907907
SELECT * FROM cypher('merge_actions', $$
908908
MATCH (a:Person {name: 'Alice'})
909909
MERGE (a)-[:KNOWS]->(b:Person {name: 'Charlie'})
910910
ON CREATE SET b.source = 'merge_create'
911911
RETURN a.name, b.name, b.source
912912
$$) AS (a agtype, b agtype, source agtype);
913913

914+
-- ON MATCH SET with MERGE after MATCH (Case 1: has predecessor, second run = match)
915+
SELECT * FROM cypher('merge_actions', $$
916+
MATCH (a:Person {name: 'Alice'})
917+
MERGE (a)-[:KNOWS]->(b:Person {name: 'Charlie'})
918+
ON MATCH SET b.visited = true
919+
RETURN a.name, b.name, b.visited
920+
$$) AS (a agtype, b agtype, visited agtype);
921+
914922
-- Multiple SET items in a single ON CREATE SET
915923
SELECT * FROM cypher('merge_actions', $$
916924
MERGE (n:Person {name: 'Dave'})

src/backend/executor/cypher_merge.c

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,35 @@ static void begin_cypher_merge(CustomScanState *node, EState *estate,
191191
}
192192
}
193193

194+
/*
195+
* Pre-initialize ExprStates for ON CREATE SET / ON MATCH SET items.
196+
* This must happen once at plan init time, not per-row.
197+
*/
198+
if (css->on_create_set_info != NULL)
199+
{
200+
foreach(lc, css->on_create_set_info->set_items)
201+
{
202+
cypher_update_item *item = (cypher_update_item *)lfirst(lc);
203+
if (item->prop_expr != NULL)
204+
{
205+
item->prop_expr_state = ExecInitExpr(
206+
(Expr *)item->prop_expr, (PlanState *)node);
207+
}
208+
}
209+
}
210+
if (css->on_match_set_info != NULL)
211+
{
212+
foreach(lc, css->on_match_set_info->set_items)
213+
{
214+
cypher_update_item *item = (cypher_update_item *)lfirst(lc);
215+
if (item->prop_expr != NULL)
216+
{
217+
item->prop_expr_state = ExecInitExpr(
218+
(Expr *)item->prop_expr, (PlanState *)node);
219+
}
220+
}
221+
}
222+
194223
/*
195224
* Postgres does not assign the es_output_cid in queries that do
196225
* not write to disk, ie: SELECT commands. We need the command id

src/backend/executor/cypher_set.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -504,8 +504,20 @@ void apply_update_list(CustomScanState *node,
504504
Datum val;
505505
bool isnull;
506506

507-
expr_state = ExecInitExpr((Expr *)update_item->prop_expr,
508-
(PlanState *)node);
507+
/*
508+
* Use the pre-initialized ExprState if available (set during
509+
* plan init in begin_cypher_merge). Fall back to per-row init
510+
* for callers that haven't pre-initialized (e.g. plain SET).
511+
*/
512+
if (update_item->prop_expr_state != NULL)
513+
{
514+
expr_state = update_item->prop_expr_state;
515+
}
516+
else
517+
{
518+
expr_state = ExecInitExpr((Expr *)update_item->prop_expr,
519+
(PlanState *)node);
520+
}
509521
val = ExecEvalExpr(expr_state, econtext, &isnull);
510522
remove_property = isnull;
511523

src/backend/nodes/cypher_copyfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ void copy_cypher_update_item(ExtensibleNode *newnode, const ExtensibleNode *from
137137
COPY_SCALAR_FIELD(remove_item);
138138
COPY_SCALAR_FIELD(is_add);
139139
COPY_NODE_FIELD(prop_expr);
140+
COPY_NODE_FIELD(prop_expr_state);
140141
}
141142

142143
/* copy function for cypher_delete_information */

src/backend/nodes/cypher_outfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,7 @@ void out_cypher_update_item(StringInfo str, const ExtensibleNode *node)
430430
WRITE_BOOL_FIELD(remove_item);
431431
WRITE_BOOL_FIELD(is_add);
432432
WRITE_NODE_FIELD(prop_expr);
433+
WRITE_NODE_FIELD(prop_expr_state);
433434
}
434435

435436
/* serialization function for the cypher_delete_information ExtensibleNode. */

src/backend/nodes/cypher_readfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ void read_cypher_update_item(struct ExtensibleNode *node)
270270
READ_BOOL_FIELD(remove_item);
271271
READ_BOOL_FIELD(is_add);
272272
READ_NODE_FIELD(prop_expr);
273+
READ_NODE_FIELD(prop_expr_state);
273274
}
274275

275276
/*

src/include/nodes/cypher_nodes.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,7 @@ typedef struct cypher_update_item
455455
bool is_add;
456456
Node *prop_expr; /* SET value expression, used by MERGE ON CREATE/MATCH SET
457457
* where the expression is not in the plan's target list */
458+
ExprState *prop_expr_state; /* initialized at plan init, not per-row */
458459
} cypher_update_item;
459460

460461
typedef struct cypher_delete_information

0 commit comments

Comments
 (0)