Support pattern expressions as boolean expressions

[gregfelice]

Summary

Adds support for openCypher pattern expressions as general boolean expressions. Pattern expressions are now accepted anywhere a regular expression is valid — in WHERE, RETURN, WITH, SET, CASE, and as operands of boolean combinators — not just in WHERE.

Note on scope: An earlier version of this PR was titled "Support pattern expressions in WHERE clause via GLR parser." While implementing, Copilot correctly pointed out that adding anonymous_path to expr_atom actually enables pattern expressions in every expression context, not only WHERE. Rather than restrict the grammar with a WHERE-specific nonterminal, this PR now explicitly embraces the broader surface area — matching what openCypher defines and what Neo4j users expect — and covers the new contexts with regression tests. The PR title and description have been updated accordingly.

// WHERE (original motivation — still works)
MATCH (a:Person), (b:Person)
WHERE (a)-[:KNOWS]->(b)
RETURN a.name, b.name

// NOT pattern expression in WHERE
MATCH (a:Person)
WHERE NOT (a)-[:KNOWS]->(:Person)
RETURN a.name

// RETURN — project a boolean
MATCH (a:Person)
RETURN a.name, (a)-[:KNOWS]->(:Person) AS knows_someone

// CASE WHEN
MATCH (a:Person)
RETURN a.name,
       CASE WHEN (a)-[:KNOWS]->(:Person) THEN 'social' ELSE 'loner' END

// Combined with AND / OR
MATCH (a:Person)
RETURN a.name,
       (a)-[:KNOWS]->(:Person) AND (a)-[:WORKS_WITH]->(:Person) AS has_both

// SET — persist a computed boolean as a property
MATCH (a:Person)
SET a.is_social = (a)-[:KNOWS]->(:Person)
RETURN a.name, a.is_social

// WITH pipeline
MATCH (a:Person)
WITH a.name AS name, (a)-[:KNOWS]->(:Person) AS knows
WHERE knows
RETURN name

Implementation

Grammar (cypher_gram.y)

• Switched the parser to GLR mode (%glr-parser) to handle the inherent ambiguity between parenthesized expressions and graph patterns. Both start with (, so a single-token lookahead cannot decide which production applies until several tokens later. GLR forks at the conflict point and discards the failing alternative.
• Added anonymous_path to expr_atom with a %dprec 1 annotation. A bare (a) prefers the expression-variable interpretation (%dprec 2 on '(' expr ')'), so single-node pattern expressions still resolve as plain variable references.
• Added make_exists_pattern_sublink() to wrap the pattern in an EXISTS subquery — a bare pattern (a)-[:KNOWS]->(b) in expression context is semantically equivalent to EXISTS((a)-[:KNOWS]->(b)).
• Added a helper make_explain_stmt() and placed it immediately above its docstring comment so the grammar file stays readable.
• Replaced (rhs)[1] with YYRHSLOC(rhs, 1) in the YYLLOC_DEFAULT macro — a required GLR-correctness fix, not stylistic. GLR's rhs stack has a different layout than LALR(1)'s; the raw index lands on the wrong slot under GLR and produces incorrect source locations.
• Conflict budget pinned with %expect/%expect-rr. GLR produces 7 shift/reduce and 3 reduce/reduce conflicts (all arising from the ( ambiguity between path_node and parenthesized expr, plus expr_var/var_name_opt overlap on )/}/=). These are handled correctly at runtime by GLR + %dprec. The grammar declares %expect 7 and %expect-rr 3 so any deviation up or down fails the build and forces an audit of the new conflicts. The Makefile keeps -Werror and scopes the relaxation to -Wno-error=conflicts-sr -Wno-error=conflicts-rr, so other Bison warning categories (unused rules, undeclared types, etc.) still fail the build. Earlier rounds of this PR tried %expect-less variants for "Bison version portability," but Bison's %expect is exact-match — so the right answer is to pin the totals and update them deliberately if a future Bison reports different counts, not to remove the alarm.

Parser (cypher_analyze.c, cypher_clause.c)

• Existing EXISTS sublink handling already covers the pattern case — no new transform code needed on the analyze/clause side.

Files changed

File	Change
src/backend/parser/cypher_gram.y	%glr-parser, %dprec annotations, anonymous_path in expr_atom, make_exists_pattern_sublink(), GLR comment block explaining why conflicts are expected
Makefile	BISONFLAGS += -Werror -Wno-error=conflicts-sr -Wno-error=conflicts-rr — keep -Werror for other warning categories, scope relaxation to the two known-noise conflict categories; register pattern_expression regression test
regress/sql/pattern_expression.sql	New regression suite: WHERE, NOT, nested, multi-hop, RETURN projection, mixed projections, CASE WHEN, AND/OR combinators, SET, WITH pipeline, regression guards for parenthesized arithmetic
regress/expected/pattern_expression.out	Expected output for above

Test plan

• Basic pattern in WHERE: WHERE (a)-[:KNOWS]->(b)
• NOT pattern in WHERE: WHERE NOT (a)-[:KNOWS]->(:Person)
• Pattern in WHERE with label: WHERE (a)-[:KNOWS]->(:Person)
• Pattern via EXISTS subquery (original syntax still works)
• Pattern in RETURN with AS alias → boolean column
• Pattern in RETURN without alias (positional column)
• Multiple pattern expressions in same RETURN
• Pattern in CASE WHEN ... THEN ... ELSE ... END
• Pattern combined with AND and OR in RETURN
• Pattern in SET a.flag = (a)-[:R]->(:L)
• Pattern in WITH projection + subsequent WHERE filter
• Regression guards: RETURN (1 + 2), RETURN (n.name) — parenthesized expressions still work
• All 32 regression tests pass (31 existing + pattern_expression)
• Build is clean under -Werror with %expect 7 %expect-rr 3 pinned (Bison 3.8.2); any conflict-count drift now fails the build deliberately

Implementation notes (added during review)

• YYRHSLOC macro change is a required GLR correctness fix, not cosmetic. GLR uses a different RHS stack layout than LALR(1); the previous (rhs)[1] indexing reads the wrong slot under GLR. The macro update is load-bearing for correct error locations under %glr-parser.
• Pattern expressions in projection are EXISTS-sublink-per-row. A bare pattern in a RETURN/WITH/SET projection (e.g. RETURN a.name, (a)-[:KNOWS]->(:Person)) desugars to a correlated EXISTS sublink evaluated once per outer row — O(rows × subquery cost). This is the correct openCypher semantics, but distinct from a pattern in WHERE, where the planner may pull the sublink up into a semi-join. Regression tests use tiny datasets so this is invisible; on large data, projection-context pattern expressions cost more than WHERE-context ones.

Known limitation (pre-existing, tracked separately)

• Single-node labeled pattern expressions (a:Label) do not filter by label. Because this PR lets a bare pattern appear as an expression anywhere, (a:Label) is now syntactically accepted as a boolean (e.g. WHERE (a:Person) or RETURN (a:Person)). It does not behave as an openCypher label predicate: the single-vertex pattern desugars to an EXISTS subquery with no label constraint and so is trivially true. Root cause is pre-existing in the transform layer — make_path_join_quals() (src/backend/parser/cypher_clause.c:5454) early-returns for vertex-only patterns (list_length(entities) < 3), skipping the label-filter quals that are only emitted for relationship patterns. This is not introduced by this PR (the code is byte-identical on master, and this PR touches no transform code); the grammar change merely makes the limitation reachable via a new surface. Relationship patterns — the feature this PR targets — correlate and filter correctly. Tracked as a follow-up; use a relationship pattern or label(a) = 'Label' for label checks in the meantime.

[Copilot]

Pull request overview

Adds openCypher-compatible “pattern expressions” as boolean predicates in WHERE by resolving the (a) ambiguity (parenthesized expression vs node pattern) using Bison GLR parsing.

Changes:

• Switch cypher_gram.y to %glr-parser, add %dprec annotations, and introduce make_exists_pattern_sublink() to share EXISTS(pattern) wrapping logic.
• Allow anonymous_path to appear as an expr_atom, translating bare patterns into an EXISTS sublink boolean.
• Add regression coverage for pattern expressions (regress/sql + regress/expected) and register the new test in REGRESS.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
src/backend/parser/cypher_gram.y	Enables bare pattern parsing in expression context via GLR + %dprec, refactors EXISTS wrapping into a helper.
regress/sql/pattern_expression.sql	New regression test cases covering pattern expressions in WHERE and boolean combinations.
regress/expected/pattern_expression.out	Expected output for the new regression test.
Makefile	Adds pattern_expression to the regression test run list.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jrgemignani]

@gregfelice Please see the above comments by Copilot

[gregfelice]

Addressed all 3 Copilot suggestions:

1. Misplaced comment — Moved "Helper function to create an ExplainStmt node" from above make_exists_pattern_sublink() to directly above make_explain_stmt() where it belongs.

2. %expect/%expect-rr documentation — Added block comment explaining the conflict budget: 7 shift/reduce from path extension vs arithmetic operators on -/<, 3 reduce/reduce from expr_var vs var_name_opt on )/}/=. All resolved by GLR forking + %dprec annotations. Noted to update counts if grammar rules change.

3. Misleading test comment — Changed "Regular expressions still work" to "Regular (non-pattern) expressions still work" to avoid confusion with regex.

Regression test passes (pattern_expression: ok).

[jrgemignani]

@gregfelice Did you push them?

[gregfelice]

@jrgemignani Pushed now — sorry about that! All 3 Copilot suggestions addressed in commit 5d11080.

And noted on the workflow — I'll reply directly to Copilot's comments going forward instead of posting standalone. Thanks for the heads up.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jrgemignani]

@gregfelice Please see Copilot above

[gregfelice]

I'm on holiday this week - will get to next week - Thanks

…

On Mon, Apr 6, 2026 at 6:17 PM John Gemignani ***@***.***> wrote: *jrgemignani* left a comment (apache/age#2360) <#2360 (comment)> @gregfelice <https://github.com/gregfelice> Please see Copilot above — Reply to this email directly, view it on GitHub <#2360 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABBXGAANBXB5W733GNLJAT4UPKCZAVCNFSM6AAAAACW6JESACVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DCOJTGQZDCMRZHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Greg Felice 415.343.5227 ***@***.*** linkedin.com/in/gregfelice calendly.com/gregfelice/30min

[gregfelice]

@jrgemignani — both Copilot items addressed and threads resolved. The %expect concern is moot (we use -Wno-conflicts-sr -Wno-conflicts-rr via BISONFLAGS instead), and the broader pattern-expression surface area is intentional, documented in the PR description, and covered by regression tests for WHERE, RETURN, CASE WHEN, SET, WITH, and boolean combinators. Ready for re-review. Thanks!

[gregfelice]

Rebased onto current master (new HEAD 908adc61) and force-pushed. Conflicts were all alphabetical-merge reconciliation with #2359: Makefile regression-test list, helper function declarations in cypher_gram.y, and the predicate-function body placement. Dropped one duplicate /* Helper function to create an ExplainStmt node */ comment that came along with the HEAD side of the conflict. No logic changes from the rebase.

Local PG18-cassert installcheck is green on all 34 AGE tests including the new pattern_expression and upstream's new predicate_functions (only pgvector fails, environmental — extension not installed on my cassert PG).

CI on the new SHA is in action_required pending maintainer approval. @MuhammadTahaNaveed / @jrgemignani — would appreciate a kick when convenient. Thanks!

[gregfelice]

Rebased onto current master (new HEAD 61426cf0) and force-pushed to clear mergeability drift. Three commits replay on top of master (84e29542). Only conflict was the Makefile bison-flags line: master still carries -Werror, this branch replaces it with -Wno-conflicts-sr -Wno-conflicts-rr plus the explanatory comment block (the whole reason for the PR, since pattern-expression grammar introduces inherent GLR conflicts). No logic changes from the rebase.

Local PG18-cassert installcheck green on all 34 AGE tests including pattern_expression (51 ms).

CI on the new SHA will sit in action_required pending maintainer approval — would you mind kicking it when you have a moment? Thanks.

[jrgemignani]

@gregfelice

From Opus 4.7 -

Deep review: PR #2360 — Pattern expressions as boolean expressions

I read the PR description and the actual diff (cypher_gram.y, Makefile, regression test). Status: 3 commits, force-pushed onto current master (84e29542), Copilot's 3+2+0 rounds resolved, jrgemignani approved. Here's my analysis.

What it does (correctly)

The change is fundamentally sound:

1. Switches cypher_gram.y to %glr-parser. Right call — the (a) ambiguity (parenthesized expr vs. node pattern) genuinely is unresolvable at one-token lookahead. LALR(1) can't see far enough to know whether ( introduces (1+2) or (a)-[:R]->(b). GLR is the textbook tool.
2. Adds anonymous_path %dprec 1 to expr_atom, alongside the existing '(' expr ')' %dprec 2. Bare (a) tied to both productions — %dprec 2 > %dprec 1 correctly disambiguates to expression-variable.
3. Refactors EXISTS-wrapping into make_exists_pattern_sublink() — and re-uses it for both the existing EXISTS(pattern) production and the new bare-pattern production. The refactor is byte-identical to what the inline code did (same cypher_sub_pattern, CSP_EXISTS, EXISTS_SUBLINK, node_to_agtype(..., "boolean", ...)). Zero semantic change for the existing EXISTS path. ✓
4. YYRHSLOC(rhs, 1) macro fix — silently correct and necessary. GLR has a different rhs stack layout than LALR(1); (rhs)[1] indexes the wrong slot under GLR. This isn't mentioned in the description but it's a real correctness fix the PR depends on.
5. Tests cover meaningful surface area — WHERE, NOT, AND/OR, RETURN (alias + positional), mixed projections, CASE WHEN, SET, WITH pipeline, backward-compat with EXISTS(...). Good coverage of the new contexts.

Concerns worth raising

1. Removing -Werror is heavier than needed

The Makefile change is:

- src/backend/parser/cypher_gram.c: BISONFLAGS += --defines=... -Werror
+ src/backend/parser/cypher_gram.c: BISONFLAGS += --defines=... -Wno-conflicts-sr -Wno-conflicts-rr

This drops -Werror entirely, not just the two conflict categories. Any other Bison warning (deprecated %name-prefix=, undeclared types, unused rules, etc.) will silently be ignored going forward.

Suggested: keep -Werror, add only the two no-error opts:

BISONFLAGS += --defines=... -Werror -Wno-error=conflicts-sr -Wno-error=conflicts-rr

Bison 3.4+ supports -Wno-error=<category>. Keeps the safety net on everything else.

2. No %expect budget at all is fragile

The description's argument — "exact counts can drift across Bison versions" — is correct. But going to "any number of conflicts is fine" loses the alarm bell that catches new, unintentional conflicts a future grammar change might introduce. A new rule that adds 12 more conflicts would silently slip in.

Suggested: %expect 7 / %expect-rr 3 with a code comment "expected to drift on Bison upgrades; update as needed." This is the standard PostgreSQL approach in gram.y (which has %expect 0 and updates when needed). The point of %expect is not to pin a version, it's to fail loudly when unexpected conflicts appear. If the worry is distro drift, the right answer is a wider %expect value (e.g., %expect 12 %expect-rr 5) — generous enough to absorb Bison-version churn, tight enough to flag a new grammar bug.

3. GLR runtime cost on the parse hot path

GLR forks parse stacks at every conflict point. On ambiguous prefixes it does several full reductions before discarding losers. AGE parses Cypher on every single cypher() call, often inside tight per-row contexts (UDF arguments, prepared statements re-planned, etc.). I don't see any parse-time benchmark in the PR.

Concretely: a MATCH (a) query enters the ( ambiguity at character 7, and GLR will explore both '(' expr ')' and anonymous_path until it sees the closing ) and what follows. For short read-only queries, parser time is non-trivial relative to plan+execute.

Recommended diligence before merge:

• Microbench: pgbench -f script.sql -T 30 -c 4 running 1000× of typical Cypher query (MATCH (a:Person {id: $1}) RETURN a) on LALR build vs. GLR build. Look for backend CPU% in parse.
• Spot-check EXPLAIN (ANALYZE, BUFFERS) SELECT cypher(...) planning time, before/after.

If parse time regresses meaningfully (say >5%) on simple queries, this is worth knowing and documenting, even if it's accepted as the cost of the feature. If it's noise (<1%), it's a non-issue.

4. Pattern expression semantics — coverage gaps

The grammar change makes anonymous_path legal anywhere an expr_atom is. That opens the door wider than the tests probe:

• Single-node labeled patterns in expression position: RETURN (a:Person) is, in openCypher, the pattern "does a have label Person?" — a boolean. Under this PR, (a:Person) would be parsed by GLR — does it pick the (expr) branch (which would fail because a:Person isn't an expression) or the anonymous_path branch (which would succeed)? Per %dprec, both fork; the failing fork is dropped; we get the pattern interpretation. The test suite doesn't include this case. Worth adding.
• In list/map literals: RETURN [(a)-[:R]->(b), (a)-[:S]->(c)] — list of booleans. RETURN {flag: (a)-[:R]->(b)} — map containing a boolean. The grammar now allows these. Untested.
• In function arguments: RETURN count((a)-[:R]->(b)). Should work (boolean is countable). Untested.
• In OPTIONAL MATCH WHERE: OPTIONAL MATCH (a) WHERE (a)-[:R]->(:L). Untested.
• Nested EXISTS: WHERE EXISTS((a)-[:R]->(b)) AND (a)-[:S]->(c). The PR's "mixed" test doesn't combine the two syntaxes in one predicate. Untested.

None of these is a blocker — they're follow-up coverage. But the PR description claims "matching what openCypher defines and what Neo4j users expect," and these are exactly the cases Neo4j users will try.

5. Performance of the resulting plan

Bare pattern → EXISTS sublink → planner builds a correlated subquery. In a RETURN a.name, (a)-[:KNOWS]->(:Person) AS knows over a million rows, the planner has to either:

• (a) Evaluate the EXISTS sublink per row (O(N) sublink evals), or
• (b) Convert it to a semi-join (PG's pull_up_sublinks).

For Cypher, the pattern is a SRF (cypher's own scan), not a SQL subquery the planner can semi-join in the usual way. So path (a) is what will happen, and on large data it will be slow.

That's the correct semantics. But it's worth documenting in the PR or a follow-up that pattern-expressions in projection contexts are O(rows × subquery cost), distinct from pattern-expressions in WHERE (where the planner might pull it up). Tests use 4 rows — invisible. Real workloads would notice.

6. The expr_func_subexpr: EXISTS '(' anonymous_path ')' production stays

It's now redundant with anonymous_path %dprec 1 in expr_atom. Both produce the same make_exists_pattern_sublink(...) output. The grammar will accept both spellings. Keeping the explicit form is fine for backward compat (users may have written WHERE EXISTS((...)) for years), but it does add to the conflict count. Could be noted in the description as "intentionally redundant for backward compatibility."

7. var_name_opt %dprec 1 on both alternatives

var_name_opt:
    /* empty */ %dprec 1
        { $$ = NULL; }
    | var_name %dprec 1
    ;

Equal priorities on both alternatives is a no-op for %dprec. Bison will treat this as "no preference" and discard the failing fork. That's fine, but the annotation is misleading — the priorities don't disambiguate, they're inert. Either remove them or set unequal values. Minor.

Verdict

Direction: correct. GLR is the right tool, the refactor preserves semantics exactly, the test coverage spans the new contexts, and Copilot's review rounds caught the small stuff.

Items I'd want addressed before merge:

1. Keep -Werror, scope down to -Wno-error=conflicts-{sr,rr} rather than nuking -Werror.
2. Re-introduce some %expect budget (generous, e.g. %expect 10 %expect-rr 5) so future grammar changes that add unintended conflicts get flagged.
3. Quick parse-time microbench (LALR vs. GLR) on a couple of representative read-only Cypher queries. Even a one-line "no measurable regression on MATCH (a:L {p: $1}) RETURN a" in the PR description would satisfy.

Items I'd address as a follow-up (not blocking):

4. Test coverage gaps: RETURN (a:Label), pattern in list/map literals, pattern in function args, OPTIONAL MATCH WHERE.
5. Document the EXISTS-sublink-per-row cost in projection contexts so users aren't surprised on large data.
6. Note the YYRHSLOC fix in the PR body — it's not just a stylistic change, it's required for GLR correctness.

Items that are fine as-is:

7. The make_exists_pattern_sublink refactor (semantics preserved byte-for-byte).
8. The conflict explanation block comment (genuinely helpful for future grammar maintainers).
9. The decision to broaden scope from "WHERE only" to "all expression contexts" — Copilot was right, the grammar change inherently does this and a WHERE-only restriction would have required a parallel grammar branch.

Overall, this is a high-quality feature PR that solves a real openCypher-compat gap, and the maintainer approval is well-earned. The Makefile -Werror removal and absent %expect budget are the two items I'd push back on; everything else is polish.

[jrgemignani]

Please see the Opus 4.7 review

[jrgemignani]

@gregfelice I'm being extra careful due to this modifying the grammar.

[gregfelice]

@jrgemignani — thanks for the careful review. Addressed all three blocking asks in 553e9761, with one factual correction to the underlying reasoning that I want to surface explicitly:

1. Makefile -Werror scope — kept -Werror, downgraded only the two conflict categories via -Wno-error=conflicts-sr -Wno-error=conflicts-rr. pgxs auto-adds -Wno-deprecated so the existing %name-prefix= / %pure-parser directives still build clean. Any other Bison warning (unused rules, undeclared types, etc.) now fails the build again — same coverage as master, minus the two known-noise categories.

2. %expect / %expect-rr budget — added %expect 7 and %expect-rr 3 matching the Bison 3.8.2 totals. Caveat: Bison treats %expect as exact-match, not as a ceiling, so the "wider budget to absorb Bison drift" framing in the review doesn't actually work — %expect 12 on a 7-conflict grammar fails the build with found 7, expected 12 (verified empirically). Exact pinning is actually a stronger signal than a generous bound: it catches both new conflicts AND silent disappearance of expected ones (either is a surprise worth auditing). The grammar comment documents that future Bison versions reporting different counts should bump these numbers explicitly with a version note in the commit message.

3. Parse-time microbench — Phase 3.5 release-build perf preflight — built a user-owned PG18 release cluster (-O2, no cassert) and ran a pgbench equivalent of the workload-mapping-prescribed smoke (W01 + W04 for parser-only changes), 5 iterations × 15 s × 1 client per side:

	LALR (master) mean	GLR (this PR) mean	delta
W04-style: MATCH (a:L {p: 42}) RETURN a (1000-node graph)	4944 TPS (4904–4976, ±1.5%)	4979 TPS (4961–4999, ±0.8%)	+0.7%
W01-style: CREATE (:L {p: 99}) (in rolled-back tx)	11064 TPS (10749–11353, ±5.6%)	11066 TPS (10920–11176, ±2.3%)	+0.02%

Both deltas well below the 5% gate, and W01's LALR-side range (5.6%) is itself wider than the LALR-vs-GLR delta. No measurable parse-time regression on representative read-only or write Cypher.

Cassert installcheck: 34/34 AGE tests green (pgvector skipped — vector extension not installed in my local cassert PG; unrelated to this PR).

I also updated the PR description to (a) note the YYRHSLOC(rhs, 1) replacement is a GLR-stack correctness fix, not stylistic, and (b) reflect the restored -Werror + %expect 7/3 approach.

Non-blocking follow-ups from the review — your call on bundling:

• Test coverage for RETURN (a:Label), list/map literals, function args, OPTIONAL MATCH WHERE — happy to fold into this PR's pattern_expression regression suite as a second commit if you want them here, otherwise I'll open a follow-up.
• Doc note on EXISTS-sublink-per-row cost in projection contexts — better as a standalone docs commit so it's discoverable from the Cypher reference, not just this PR's history.

[jrgemignani]

@gregfelice Ty. Just me - I prefer the builds to fail on anything, warnings included :) And grammars are tricky.

[jrgemignani]

@gregfelice I would add the non-blocking follow ups that you listed. Follow ups sometimes get forgotten. Otherwise, everything else looks good.

[gregfelice]

@jrgemignani — folded in the test-coverage follow-up (commit d372551a). New cases, all verified against a PG18 build (33/33 installcheck): pattern exprs in list literals, map literals, function args (collect()/count()), OPTIONAL MATCH … WHERE (null-preserving), and EXISTS() + bare pattern combined in one predicate.

One thing I want to flag rather than bury — the RETURN (a:Label) case from the review doesn't behave as openCypher's label predicate. On an already-bound variable it's a degenerate EXISTS existence check (a matching label is always true), and a non-matching label hits AGE's pre-existing "multiple labels for variable" restriction (errors rather than evaluating false). I captured both behaviours in the suite (including the error as expected output) so the current semantics are pinned — but if you'd rather reject single-node patterns in expression position than document the degenerate form, say the word and I'll add that instead.

Also documented that count() over a pattern boolean counts every row (boolean false is non-null) — expected SQL aggregate semantics, just noted so the value isn't mistaken for a bug.

The EXISTS-sublink-per-row cost note I'll do as a separate apache/age-website PR so it's discoverable from the Cypher reference, as discussed.

[gregfelice]

The EXISTS-sublink-per-row cost note is up as a separate docs PR: apache/age-website#364. It documents pattern expressions in RETURN/WITH projections with a verified example plus the per-row EXISTS-subquery cost note, and is marked as depending on this PR (shouldn't merge before it).

[jrgemignani]

@gregfelice We merged some big PRs and another 2 will hopefully be happening tomorrow. You may need to rebase this one too.

[gregfelice]

sg

…

On Mon, Jun 1, 2026 at 6:52 PM John Gemignani ***@***.***> wrote: *jrgemignani* left a comment (apache/age#2360) <#2360 (comment)> @gregfelice <https://github.com/gregfelice> We merged some big PRs and another 2 will hopefully be happening tomorrow. You may need to rebase this one too. — Reply to this email directly, view it on GitHub <#2360?email_source=notifications&email_token=AABBXGGWON6GV47MXZUVCP345YCLJA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINJZG4YTGMJRGUYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4597131150>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABBXGEKHKNMPHIE5KNNCGL45YCLJAVCNFSM6AAAAACW6JESACVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DKOJXGEZTCMJVGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Greg Felice 415.343.5227 ***@***.*** linkedin.com/in/gregfelice calendly.com/gregfelice/30min

[gregfelice]

Rebased onto current master (c59cba71) — clean replay, no logic changes (the only intervening master commit, the cassert Dockerfile, doesn't touch these files). Full cassert installcheck is green: 36/36 including pattern_expression, via the CI-exact EXTRA_TESTS="pgvector fuzzystrmatch pg_trgm". Notably the %expect 7/3 pin held under a different Bison version than the original 3.8.2, so the grammar-gen contract looks robust across distros.

All prior review items are addressed (-Werror/%expect rework in 553e9761, follow-up coverage in the latest commit). @jrgemignani — when you have a moment, could you kick the action_required CI and re-review / clear the changes-requested? (Docs companion age-website#364 is ready, gated to merge after this.)

[gregfelice]

Correction to my previous comment — that rebase was onto a stale fork base, which pulled an unrelated docker/Dockerfile.cassert into the diff and left the branch behind apache master. Re-rebased cleanly onto current master (12e2a31c): the PR is now just the grammar + test files (4 files, no stray Docker change), re-validated cassert-green 40/40 including pattern_expression via the CI-exact EXTRA_TESTS="pgvector fuzzystrmatch pg_trgm". @jrgemignani — ready for the action_required CI kick + re-review when you have a moment.

[gregfelice]

@jrgemignani — rebased onto current master (581d236e) and ready for re-review. The changes-requested items from your 05-13 review are all in:

• -Werror scope — kept -Werror, downgraded only the two conflict categories via -Wno-error=conflicts-{sr,rr} (553e9761). Any other Bison warning still fails the build.
• %expect budget — %expect 7 / %expect-rr 3, exact-match pins (553e9761).
• Parse-time microbench — LALR vs GLR, +0.7% / +0.02%, both well under the 5% gate.
• Follow-up coverage (your 06-01 ask) — list/map literals, function args, OPTIONAL MATCH … WHERE, EXISTS+bare-pattern, and the documented (a:Label) degenerate case (d372551a).

The rebase was a clean replay (no logic changes; the only overlap, #2437's Makefile rework, auto-merged cleanly — the REGRESS entry and BISONFLAGS are intact). Re-validated cassert PG18 green: 42/42 including pattern_expression, CI-exact EXTRA_TESTS="pgvector fuzzystrmatch pg_trgm", no regression diffs.

When you have a moment, could you kick the action_required CI and clear the stale changes-requested? Docs companion age-website#364 is ready and gated to merge after this.

[jrgemignani]

@gregfelice Going to set off Copilot and Opus 4.8 for one last check. Otherwise, everything looks good. I'm just extra careful with anything grammar related. That's not you, that's me.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[jrgemignani]

@gregfelice Looks good

[gregfelice]

@jrgemignani — closing out the Opus 4.7 review items.

1. -Werror scope — done (c6270db0): kept -Werror, downgraded only -Wno-error=conflicts-{sr,rr}. Any other Bison warning still fails the build.

2. %expect budget — one correction, because the suggested fix can't compile, then how I solved the real goal. Bison treats %expect / %expect-rr as exact-match, not a ceiling: %expect 10 against a real count of 7 fails with "expected 10 shift/reduce conflicts, found 7." So widening to a generous 10/5 reds the build immediately on our Bison (3.8.2 → 7 s/r, 3 r/r).

The underlying goal — don't let distro Bison drift silently break CI, but still catch genuinely new conflicts — is right, so I pinned the toolchain instead of loosening the alarm (just pushed):

• installcheck.yaml: runs-on: ubuntu-latest → ubuntu-24.04. ubuntu-latest floating to a newer release (and a newer Bison) was the actual drift source; freezing the image freezes Bison at 3.8.x.
• Added a "Verify Bison version" step that fails loudly with a pointer to cypher_gram.y if Bison ever leaves 3.8.x — so the next person gets "re-run bison, update %expect, bump the runner together," not a cryptic count mismatch deep in the build.

Net: %expect 7 / %expect-rr 3 stays exact (tightest alarm for a real new conflict), reproducibility comes from pinning the variable. Same posture as PostgreSQL's gram.y, which keeps an exact %expect and bumps it on intentional changes.

3. Parse-time microbench (GLR vs LALR) — ran it. Disposable PG18 container, identical AGE build per arm except the grammar, pgbench -M simple (simple protocol = full re-parse every txn — the regime GLR taxes), -c4, 1000-node graph, Bison 3.8.2.

query	LALR tps	GLR tps	Δ
RETURN 1 (parse floor)	123,849	123,815	−0.03%
MATCH (a:Person {id:$1}) RETURN a	25,170	25,463	+1.2%
RETURN (a)-[:KNOWS]->(:Person) (pattern expr)	parse error (LALR)	17,156	GLR-only

No measurable parse-time regression — the parse floor is identical and the point lookup is within run-to-run noise (GLR came out marginally ahead), well under the >5% bar. The pattern-expression query is GLR-only (new syntax); its lower tps is EXISTS-sublink execution cost, not parse overhead.

4. Follow-up test coverage — already added: pattern expressions in list literal, map literal, count(), OPTIONAL MATCH … WHERE (null-preserving), and mixed EXISTS((..)) AND (..).

Still open from the review — want me to fold these in, or take as a fast follow?

• One coverage gap remains: RETURN (a:Label) (single-node labeled pattern as a boolean). Quick to add.
• PR-body note that the YYRHSLOC change is a required GLR correctness fix, not cosmetic.
• PR-body note that pattern expressions in projection are EXISTS-sublink-per-row (O(rows × subquery)), distinct from WHERE where the planner may pull it up.

[gregfelice]

Follow-up on the remaining review items — PR body updated, plus one finding worth flagging.

Test coverage — RETURN (a:Label): while adding this case I found it isn't a missing test, it's a latent bug. A single-node labeled pattern as a boolean (WHERE (a:Person) / RETURN (a:Person)) is accepted but doesn't filter by label — it's trivially true. Root cause is pre-existing in the transform layer: make_path_join_quals() (cypher_clause.c:5454) early-returns for vertex-only patterns (list_length(entities) < 3), so the label-filter quals — only emitted for relationship patterns — are skipped. It's byte-identical on master and this PR touches no transform code; the grammar change just makes it reachable via the new (a:Label) surface. Relationship patterns (this PR's actual feature) correlate and filter correctly. I opened #2443 to track the transform fix rather than scope-creep it into the grammar PR, and documented it as a known limitation in the PR body. (Verified by building this branch in a container and running the repros.)

Doc notes added to the PR body:

• YYRHSLOC change is a required GLR correctness fix (different RHS stack layout under GLR), not cosmetic.
• Pattern expressions in projection are EXISTS-sublink-per-row (O(rows × subquery)), distinct from WHERE where the planner may pull them up.

So the review items net out as: #1 (-Werror) done, #2 (%expect) solved via toolchain pinning, #3 (parse-time) measured — no regression, #4 (coverage) done for relationship patterns with the single-node label case tracked in #2443. Ready for another look whenever.