You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
gfql/cypher: resolve Lark ambiguity between where_predicates and expr (grammar-level follow-up to #1125) #1194
PR #1193 (issue #1125) hardened WHERE label narrowing by replacing a fragile regex in `binder.py` with AST-derived narrowing plus a text-split fallback in `parser.py::generic_where_clause`.
The text-split fallback exists because of a Lark grammar ambiguity: `WHERE n:Admin AND n:Active` can match BOTH the structured rule `where_predicates: where_predicate ("AND"i where_predicate)*` (parser.py:127) AND the generic expression path `where_clause: "WHERE"i expr -> generic_where_clause` (parser.py:126). Lark's ambiguity resolution prefers the generic path, so the AND-joined label predicates land in `WhereClause.expr` as raw text rather than as structured `WhereClause.predicates`.
The current PR works around this at the transformer level — `generic_where_clause` re-parses the raw text and lifts it back into structured predicates via `_split_top_level_and_terms` + `_BARE_LABEL_PREDICATE_RE`. This is correct and well-tested, but architecturally backward: the grammar already declares the structured path we want; we should let it win.
Proposed follow-up
Fix the ambiguity at the grammar layer so AND-joined bare label predicates naturally route to `where_predicates`:
Investigate why `expr` currently wins the ambiguity contest — likely because `bare_label_predicate_expr` is reachable from `?predicate` inside `?and_expr`.
Either:
Remove the `bare_label_predicate_expr` alternative from the `expr` path, forcing label predicates to go through `where_predicates` exclusively, OR
Raise the priority of `where_predicates` (Lark rule priority annotation), OR
Introduce an explicit `where_clause` disambiguator rule.
Once the grammar disambiguates correctly, the text-split logic in `generic_where_clause` becomes dead code for AND-joined labels — simplify to the single-label case (still a valid generic fallback) or remove entirely.
Tests to keep green
All 10 label-narrowing tests in `test_binder.py` (single, double, triple AND, multi-alias, multi-label, lowercase-and, mixed label+property, XOR/OR/NOT conservative, string-literal false-positive guards).
All 4 parser-level AST tests in `test_parser.py`.
Constraints
Must not regress existing `where_predicates` behavior (structured comparison predicates, IS NULL, CONTAINS, etc.).
Must keep string-literal false-positive protection (the `fullmatch` invariant in the current fallback).
Priority
Low-to-medium. The current text-level workaround is correct and tested; this is an architectural cleanup that removes a duplicate implementation of "split on top-level AND" and lets the grammar be the source of truth.
Related follow-up: #1200 — once Lark's and_op/or_op tree is exposed structurally in WhereClause, the text-split logic in generic_where_clause this issue targets becomes even more clearly redundant. The two issues combine to eliminate text re-parsing in both parser and pushdown paths.
WHERE boolean-shape conformance matrix merged in #1204. 31 tests lock parser/binder/pushdown shape contracts for all 7 expression forms so grammar-disambiguation changes in this issue can't silently drift them.
Activity
lmeyerov commentedon Apr 24, 2026
Related follow-up: #1200 — once Lark's
and_op/or_optree is exposed structurally inWhereClause, the text-split logic ingeneric_where_clausethis issue targets becomes even more clearly redundant. The two issues combine to eliminate text re-parsing in both parser and pushdown paths.test(#1201): WHERE boolean-shape conformance matrix (parser + binder …
test(#1201): WHERE boolean-shape conformance matrix (#1204)
lmeyerov commentedon Apr 24, 2026
WHERE boolean-shape conformance matrix merged in #1204. 31 tests lock parser/binder/pushdown shape contracts for all 7 expression forms so grammar-disambiguation changes in this issue can't silently drift them.
test(gfql/conformance): update WHERE boolean-shape contract for #1200 …
feat(gfql/binder): walk expr_tree, emit one BoundPredicate per top-le…
Merge master (#1214 expr_tree invariant) into m4-1194-expr-tree-walk
fix(cypher): walk expr_tree to lift label-only WHERE predicates (#1194)…