Skip to content

Commit d05092d

Browse files
gregfeliceclaude
gregfelice
and
claude
committed
Address Copilot round 3: broaden scope, remove %expect fragility
- Pattern expressions are now accepted anywhere an expr is valid (RETURN, WITH, SET, CASE, boolean combinations), not only WHERE. This matches openCypher semantics and documents the broader surface area that was already implicitly enabled by adding anonymous_path to expr_atom. Added regression tests for each new context: RETURN projection (bare and AS-aliased), mixed with other projections, CASE WHEN, boolean AND/OR combinators, SET to persist a computed boolean property, and WITH ... WHERE pipeline. - Remove the hardcoded `%expect 7` / `%expect-rr 3` conflict budget from cypher_gram.y. The exact conflict counts can drift across Bison versions and distros, which would break builds even though the grammar is correct (GLR handles the conflicts at runtime via fork + %dprec). Instead, pass -Wno-conflicts-sr / -Wno-conflicts-rr via BISONFLAGS in the Makefile so the build stays clean without binding us to a specific Bison release. Kept a block comment in the grammar explaining why GLR conflicts are expected and how they resolve. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5d11080 commit d05092d

4 files changed

Lines changed: 190 additions & 13 deletions

File tree

‎Makefile‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,19 @@ src/include/parser/cypher_kwlist_d.h: src/include/parser/cypher_kwlist.h $(GEN_K
151151

152152
src/include/parser/cypher_gram_def.h: src/backend/parser/cypher_gram.c
153153

154-
src/backend/parser/cypher_gram.c: BISONFLAGS += --defines=src/include/parser/cypher_gram_def.h
154+
#
155+
# The Cypher grammar uses GLR mode with a number of inherent shift/reduce
156+
# and reduce/reduce conflicts arising from the ambiguity between
157+
# parenthesized expressions and graph patterns (both start with '(').
158+
# GLR handles these correctly at runtime by forking at the conflict
159+
# point; %dprec annotations resolve cases where both forks succeed.
160+
#
161+
# We suppress the conflict warnings rather than hard-coding a conflict
162+
# budget with %expect / %expect-rr, because the exact counts vary across
163+
# Bison versions and would otherwise make the build fragile across
164+
# distros and future Bison releases.
165+
#
166+
src/backend/parser/cypher_gram.c: BISONFLAGS += --defines=src/include/parser/cypher_gram_def.h -Wno-conflicts-sr -Wno-conflicts-rr
155167

156168
src/backend/parser/cypher_parser.o: src/backend/parser/cypher_gram.c src/include/parser/cypher_gram_def.h
157169
src/backend/parser/cypher_parser.bc: src/backend/parser/cypher_gram.c src/include/parser/cypher_gram_def.h

‎regress/expected/pattern_expression.out‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,106 @@ $$) AS (result agtype);
213213
"Alice"
214214
(1 row)
215215

216+
--
217+
-- Pattern expressions in RETURN (boolean projection)
218+
--
219+
-- Each person gets a column showing whether they know someone
220+
SELECT * FROM cypher('pattern_expr', $$
221+
MATCH (a:Person)
222+
RETURN a.name, (a)-[:KNOWS]->(:Person) AS knows_someone
223+
ORDER BY a.name
224+
$$) AS (name agtype, knows_someone agtype);
225+
name | knows_someone
226+
-----------+---------------
227+
"Alice" | true
228+
"Bob" | false
229+
"Charlie" | false
230+
"Dave" | false
231+
(4 rows)
232+
233+
-- Mix pattern expression with other projections
234+
SELECT * FROM cypher('pattern_expr', $$
235+
MATCH (a:Person)
236+
RETURN a.name, (a)-[:KNOWS]->(:Person), (a)-[:WORKS_WITH]->(:Person)
237+
ORDER BY a.name
238+
$$) AS (name agtype, knows agtype, works_with agtype);
239+
name | knows | works_with
240+
-----------+-------+------------
241+
"Alice" | true | true
242+
"Bob" | false | false
243+
"Charlie" | false | false
244+
"Dave" | false | false
245+
(4 rows)
246+
247+
--
248+
-- Pattern expressions in CASE WHEN
249+
--
250+
SELECT * FROM cypher('pattern_expr', $$
251+
MATCH (a:Person)
252+
RETURN a.name,
253+
CASE WHEN (a)-[:KNOWS]->(:Person) THEN 'social'
254+
ELSE 'loner'
255+
END
256+
ORDER BY a.name
257+
$$) AS (name agtype, kind agtype);
258+
name | kind
259+
-----------+----------
260+
"Alice" | "social"
261+
"Bob" | "loner"
262+
"Charlie" | "loner"
263+
"Dave" | "loner"
264+
(4 rows)
265+
266+
--
267+
-- Pattern expressions combined with boolean operators in RETURN
268+
--
269+
SELECT * FROM cypher('pattern_expr', $$
270+
MATCH (a:Person)
271+
RETURN a.name,
272+
(a)-[:KNOWS]->(:Person) AND (a)-[:WORKS_WITH]->(:Person) AS has_both,
273+
(a)-[:KNOWS]->(:Person) OR (a)-[:WORKS_WITH]->(:Person) AS has_either
274+
ORDER BY a.name
275+
$$) AS (name agtype, has_both agtype, has_either agtype);
276+
name | has_both | has_either
277+
-----------+----------+------------
278+
"Alice" | true | true
279+
"Bob" | false | false
280+
"Charlie" | false | false
281+
"Dave" | false | false
282+
(4 rows)
283+
284+
--
285+
-- Pattern expression in SET (store boolean as property)
286+
--
287+
SELECT * FROM cypher('pattern_expr', $$
288+
MATCH (a:Person)
289+
SET a.is_social = (a)-[:KNOWS]->(:Person)
290+
RETURN a.name, a.is_social
291+
ORDER BY a.name
292+
$$) AS (name agtype, is_social agtype);
293+
name | is_social
294+
-----------+-----------
295+
"Alice" | true
296+
"Bob" | false
297+
"Charlie" | false
298+
"Dave" | false
299+
(4 rows)
300+
301+
--
302+
-- Pattern expression in WITH (carry boolean through pipeline)
303+
--
304+
SELECT * FROM cypher('pattern_expr', $$
305+
MATCH (a:Person)
306+
WITH a.name AS name, (a)-[:KNOWS]->(:Person) AS knows
307+
WHERE knows
308+
RETURN name
309+
ORDER BY name
310+
$$) AS (result agtype);
311+
result
312+
---------
313+
"Alice"
314+
(1 row)
315+
216316
--
217317
-- Cleanup
218318
--

‎regress/sql/pattern_expression.sql‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,67 @@ SELECT * FROM cypher('pattern_expr', $$
153153
RETURN (n.name)
154154
$$) AS (result agtype);
155155

156+
--
157+
-- Pattern expressions in RETURN (boolean projection)
158+
--
159+
-- Each person gets a column showing whether they know someone
160+
SELECT * FROM cypher('pattern_expr', $$
161+
MATCH (a:Person)
162+
RETURN a.name, (a)-[:KNOWS]->(:Person) AS knows_someone
163+
ORDER BY a.name
164+
$$) AS (name agtype, knows_someone agtype);
165+
166+
-- Mix pattern expression with other projections
167+
SELECT * FROM cypher('pattern_expr', $$
168+
MATCH (a:Person)
169+
RETURN a.name, (a)-[:KNOWS]->(:Person), (a)-[:WORKS_WITH]->(:Person)
170+
ORDER BY a.name
171+
$$) AS (name agtype, knows agtype, works_with agtype);
172+
173+
--
174+
-- Pattern expressions in CASE WHEN
175+
--
176+
SELECT * FROM cypher('pattern_expr', $$
177+
MATCH (a:Person)
178+
RETURN a.name,
179+
CASE WHEN (a)-[:KNOWS]->(:Person) THEN 'social'
180+
ELSE 'loner'
181+
END
182+
ORDER BY a.name
183+
$$) AS (name agtype, kind agtype);
184+
185+
--
186+
-- Pattern expressions combined with boolean operators in RETURN
187+
--
188+
SELECT * FROM cypher('pattern_expr', $$
189+
MATCH (a:Person)
190+
RETURN a.name,
191+
(a)-[:KNOWS]->(:Person) AND (a)-[:WORKS_WITH]->(:Person) AS has_both,
192+
(a)-[:KNOWS]->(:Person) OR (a)-[:WORKS_WITH]->(:Person) AS has_either
193+
ORDER BY a.name
194+
$$) AS (name agtype, has_both agtype, has_either agtype);
195+
196+
--
197+
-- Pattern expression in SET (store boolean as property)
198+
--
199+
SELECT * FROM cypher('pattern_expr', $$
200+
MATCH (a:Person)
201+
SET a.is_social = (a)-[:KNOWS]->(:Person)
202+
RETURN a.name, a.is_social
203+
ORDER BY a.name
204+
$$) AS (name agtype, is_social agtype);
205+
206+
--
207+
-- Pattern expression in WITH (carry boolean through pipeline)
208+
--
209+
SELECT * FROM cypher('pattern_expr', $$
210+
MATCH (a:Person)
211+
WITH a.name AS name, (a)-[:KNOWS]->(:Person) AS knows
212+
WHERE knows
213+
RETURN name
214+
ORDER BY name
215+
$$) AS (result agtype);
216+
156217
--
157218
-- Cleanup
158219
--

‎src/backend/parser/cypher_gram.y‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,20 +59,24 @@
5959
*/
6060
%glr-parser
6161
/*
62-
* Conflict budget for the GLR parser. Update these counts if grammar
63-
* rules change.
62+
* GLR conflicts are expected and correct for this grammar. They arise
63+
* from the inherent ambiguity between parenthesized expressions and
64+
* graph patterns: the shift/reduce conflicts on '-', '<', '{',
65+
* PARAMETER and ')' all come from path extension vs. arithmetic or
66+
* parenthesized-expression alternatives after a leading '(', and the
67+
* reduce/reduce conflicts on ')', '}' and '=' come from the overlap
68+
* between expr_var and var_name_opt. GLR handles all of these by
69+
* forking at the conflict point and discarding the failing alternative;
70+
* %dprec annotations on expr_var/var_name_opt and '(' expr ')' /
71+
* anonymous_path resolve cases where both forks succeed (bare (a)
72+
* prefers the expression interpretation).
6473
*
65-
* %expect 7 (shift/reduce) -- All arise from the ambiguity between
66-
* path extension ('-' '[' ... ']' '-' '>') and arithmetic operators
67-
* on '-' and '<'. GLR forks at these points and discards the
68-
* failing alternative.
69-
*
70-
* %expect-rr 3 (reduce/reduce) -- From the overlap between expr_var
71-
* and var_name_opt on ')' / '}' / '='. Resolved by %dprec
72-
* annotations that prefer the expression interpretation.
74+
* We intentionally do not use %expect / %expect-rr here because the
75+
* exact conflict counts can vary across Bison versions (and across
76+
* distros) as the generator's internals change. Instead, the Makefile
77+
* passes -Wno-conflicts-sr,-Wno-conflicts-rr via BISONFLAGS so the
78+
* build stays clean without binding us to a specific Bison release.
7379
*/
74-
%expect 7
75-
%expect-rr 3
7680

7781
%lex-param {ag_scanner_t scanner}
7882
%parse-param {ag_scanner_t scanner}

0 commit comments

Comments
 (0)