Copilot commented on code in PR #2347:
URL: https://github.com/apache/age/pull/2347#discussion_r2879755062
##########
src/backend/parser/cypher_gram.y:
##########
@@ -89,7 +93,7 @@
LIMIT
MATCH MERGE
NOT NULL_P
- OPERATOR OPTIONAL OR ORDER
+ ON OPERATOR OPTIONAL OR ORDER
Review Comment:
The `ON` token is added to the `%token` list here (making it a lexical
keyword), but it is NOT added to the `safe_keywords` production rule later in
the file (around lines 2431-2477). All other keywords added as tokens in this
file are also listed in `safe_keywords` so they can be used as identifiers,
property names, or labels via the `schema_name` → `reserved_keyword` →
`safe_keywords` path.
By omitting `ON` from `safe_keywords`, property keys or node labels named
`on` (e.g., `n.on`, `MATCH (n:on)`) will now fail to parse with a syntax error.
This is a breaking change for any graph using `on` as a property name or label.
Adding `| ON { $$ = KEYWORD_STRDUP($1); }` to the `safe_keywords` rule would
fix this.
##########
regress/sql/cypher_merge.sql:
##########
@@ -868,9 +868,95 @@ SELECT * FROM cypher('issue_1630', $$ MATCH (n) DETACH
DELETE n $$) AS (a agtype
SELECT * FROM cypher('issue_1709', $$ MATCH (n) DETACH DELETE n $$) AS (a
agtype);
SELECT * FROM cypher('issue_1446', $$ MATCH (n) DETACH DELETE n $$) AS (a
agtype);
+--
+-- ON CREATE SET / ON MATCH SET tests (issue #1619)
+--
+SELECT create_graph('merge_actions');
+
+-- Basic ON CREATE SET: first run creates the node
+SELECT * FROM cypher('merge_actions', $$
+ MERGE (n:Person {name: 'Alice'})
+ ON CREATE SET n.created = true
+ RETURN n.name, n.created
+$$) AS (name agtype, created agtype);
+
+-- ON MATCH SET: second run matches the existing node
+SELECT * FROM cypher('merge_actions', $$
+ MERGE (n:Person {name: 'Alice'})
+ ON MATCH SET n.found = true
+ RETURN n.name, n.created, n.found
+$$) AS (name agtype, created agtype, found agtype);
+
+-- Both ON CREATE SET and ON MATCH SET (first run = create)
+SELECT * FROM cypher('merge_actions', $$
+ MERGE (n:Person {name: 'Bob'})
+ ON CREATE SET n.created = true
+ ON MATCH SET n.matched = true
+ RETURN n.name, n.created, n.matched
+$$) AS (name agtype, created agtype, matched agtype);
+
+-- Both ON CREATE SET and ON MATCH SET (second run = match)
+SELECT * FROM cypher('merge_actions', $$
+ MERGE (n:Person {name: 'Bob'})
+ ON CREATE SET n.created = true
+ ON MATCH SET n.matched = true
+ RETURN n.name, n.created, n.matched
+$$) AS (name agtype, created agtype, matched agtype);
+
+-- ON CREATE SET with MERGE after MATCH (Case 1: has predecessor, first run =
create)
+SELECT * FROM cypher('merge_actions', $$
+ MATCH (a:Person {name: 'Alice'})
+ MERGE (a)-[:KNOWS]->(b:Person {name: 'Charlie'})
+ ON CREATE SET b.source = 'merge_create'
+ RETURN a.name, b.name, b.source
+$$) AS (a agtype, b agtype, source agtype);
+
+-- ON MATCH SET with MERGE after MATCH (Case 1: has predecessor, second run =
match)
+SELECT * FROM cypher('merge_actions', $$
+ MATCH (a:Person {name: 'Alice'})
+ MERGE (a)-[:KNOWS]->(b:Person {name: 'Charlie'})
+ ON MATCH SET b.visited = true
+ RETURN a.name, b.name, b.visited
+$$) AS (a agtype, b agtype, visited agtype);
+
+-- Multiple SET items in a single ON CREATE SET
+SELECT * FROM cypher('merge_actions', $$
+ MERGE (n:Person {name: 'Dave'})
+ ON CREATE SET n.a = 1, n.b = 2
+ RETURN n.name, n.a, n.b
+$$) AS (name agtype, a agtype, b agtype);
+
+-- Reverse order: ON MATCH before ON CREATE should work
+SELECT * FROM cypher('merge_actions', $$
+ MERGE (n:Person {name: 'Eve'})
+ ON MATCH SET n.seen = true
+ ON CREATE SET n.new = true
+ RETURN n.name, n.new
+$$) AS (name agtype, new agtype);
+
+-- Error: ON CREATE SET specified more than once
+SELECT * FROM cypher('merge_actions', $$
+ MERGE (n:Person {name: 'Bad'})
+ ON CREATE SET n.a = 1
+ ON CREATE SET n.b = 2
+ RETURN n
+$$) AS (n agtype);
+
+-- Error: ON MATCH SET specified more than once
+SELECT * FROM cypher('merge_actions', $$
+ MERGE (n:Person {name: 'Bad'})
+ ON MATCH SET n.a = 1
+ ON MATCH SET n.b = 2
+ RETURN n
+$$) AS (n agtype);
+
Review Comment:
There is no test covering the interaction of ON CREATE SET / ON MATCH SET
with chained (non-terminal) MERGE statements, which exercises the
eager-buffering code path added in PR #2344. The non-terminal MERGE path (lines
664-750 in `cypher_merge.c`) has ON CREATE SET and ON MATCH SET logic, but the
tests only cover Case 1 with a MATCH predecessor (not a MERGE-then-MERGE
chain). A test like:
```
MERGE (a:A {name: 'X'}) ON CREATE SET a.new = true
MERGE (b:B {name: 'Y'}) ON CREATE SET b.new = true
RETURN a, b
```
or with a chained terminal MERGE following a non-terminal MERGE with ON SET
would validate that the eager buffering path handles ON SET correctly.
```suggestion
-- Chained MERGE with ON CREATE SET in non-terminal and terminal clauses
SELECT * FROM cypher('merge_actions', $$
MERGE (a:Person {name: 'ChainCreateA'})
ON CREATE SET a.new = true
MERGE (b:Person {name: 'ChainCreateB'})
ON CREATE SET b.new = true
RETURN a.name, a.new, b.name, b.new
$$) AS (a_name agtype, a_new agtype, b_name agtype, b_new agtype);
-- Chained MERGE with non-terminal ON CREATE SET and terminal ON MATCH SET
-- Setup an existing node to be matched by the second MERGE
SELECT * FROM cypher('merge_actions', $$
CREATE (p:Person {name: 'ChainMatch', seen: false})
$$) AS (n agtype);
SELECT * FROM cypher('merge_actions', $$
MERGE (a:Person {name: 'ChainCreateOnce'})
ON CREATE SET a.created = true
MERGE (b:Person {name: 'ChainMatch'})
ON MATCH SET b.seen = true
RETURN a.name, a.created, b.name, b.seen
$$) AS (a_name agtype, a_created agtype, b_name agtype, b_seen agtype);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
