From d8b2bbfafff345825fe8001f7466ef40872bf903 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sun, 5 Jul 2026 15:23:12 -0700 Subject: [PATCH] refactor(gfql/cypher): consume WHERE directly from MatchClause (remove flatten seam) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The grammar bundles a trailing WHERE onto its MATCH clause. The transformer previously split it back out into a synthetic standalone WhereClause item and let the legacy clause-sequence assembler re-attach it — a temporary seam that kept the Earley→LALR switch (#1682) byte-identical without touching the gnarliest transformer method. Now the assembler consumes MatchClause.where directly: - primary MATCH keeps its WHERE on the clause (and it becomes the query's top-level `where`, last-clause-wins for per-clause scoping) — the grammar already set it, so this is a pure deletion of the split/re-attach round-trip; - a post-WITH re-entry MATCH's WHERE moves into `reentry_wheres` with the clause carrying none (the one case that still needs a split). The now-unreachable standalone-WhereClause branches in both query_body and graph_constructor are removed. Net −23 lines. Pure internal refactor, NO behavior change (closes #1687). Verified: - old-vs-new production AST differential over a 1,989-query repo corpus: 1989/1989 byte-identical, 0 AST diffs, 0 language diffs; - grammar invariants (zero conflicts / strict / semantic-ambiguity-zero) + full-repo LALR≡Earley differential pass; - full cypher suite on dgx-spark: 1737 passed / 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + graphistry/compute/gfql/cypher/parser.py | 97 +++++++++--------------- 2 files changed, 38 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9cb8aa45a..78d2efb9d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL `engine='polars'`/`'polars-gpu'` can now run off-engine analytic `call()`s (umap/hypergraph/compute_cugraph/…) — `call_mode='auto'` (default)**: a GFQL `call()` that invokes a Plottable-method analytic with **no native polars implementation** (and never will — `umap`, `hypergraph`, `compute_cugraph`, `compute_igraph`, `layout_*`, `collapse`, `get_topological_levels`, …) previously raised `NotImplementedError` under a polars engine, forcing users off polars for the whole pipeline. It now runs as a **mode-gated, warned modality switch**: `call_mode='auto'` (default) bridges the graph off-engine (`polars`→pandas, `polars-gpu`→cuDF **on-device**), runs the analytic, and coerces the result back to polars **losslessly via Arrow**, warning once per (process, function). `call_mode='strict'` keeps the honest `NotImplementedError` decline (for benchmark integrity / a hard memory ceiling). `polars-gpu` is **GPU-or-error**: it bridges to cuDF and declines rather than silently dropping a GPU analytic to host pandas. This is **deliberately narrower than CHAIN traversal/filter/row ops**, which stay parity-or-NIE (a bridge there would hide a missing impl and cheat a benchmark) — the split is mechanical (`is_row_pipeline_call`), and the chain and DAG surfaces bridge consistently. Configurable via `graphistry.compute.gfql.lazy.set_call_mode('auto'|'strict')` (Python) or `GFQL_POLARS_CALL_MODE` (env), resolving Python override > env > default('auto'), read live. Verified on dgx: `compute_cugraph` PageRank is byte-parity with the pandas oracle under both `polars` and `polars-gpu`. ### Changed +- **GFQL Cypher parser: internal cleanup — WHERE consumed directly from `MatchClause`**: the grammar bundles a trailing `WHERE` onto its `MATCH` clause; the transformer previously split it back out into a synthetic standalone item and re-attached it, so the legacy clause-assembler ran unchanged (a temporary seam that kept the LALR switch byte-identical). The assembler now consumes `MatchClause.where` directly (primary MATCH keeps its WHERE on the clause; a post-WITH re-entry MATCH's WHERE goes to `reentry_wheres`), deleting the split/re-attach round-trip and the now-unreachable standalone-WHERE handling in both `query_body` and `graph_constructor`. Pure internal refactor, **no behavior change**: verified byte-identical ASTs vs the prior parser across a 1,989-query repo corpus, and the full cypher suite passes. - **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. The grammar was then **purified to (near-)unambiguous**: WHERE binds to its preceding clause *in the grammar* (bundled into `match_clause`; the WITH..WHERE attachment ambiguity is eliminated, not tie-broken), and name-rooted dot chains derive only via `qualified_name` (the `property_access` redundancy is gone) — dropping the redundant `label_predicate_expr` rule (`(n:Admin)` parses via `grouped_expr`), and excluding a top-level `IN` from list-literal elements (`[x IN xs ...` is comprehension syntax; allowing a bare `IN` list element made it overlap). Net: the LALR conflict profile goes from 8 shift/reduce to **ZERO conflicts** — the grammar is now **provably unambiguous LALR(1)** and builds under Lark's `strict=True` (a build-time proof, machine-checked in CI as zero conflicts, plus a strict-mode build test where the optional `interegular` dep is present). Every input has a single derivation. **Machine-checked invariants** (`test_grammar_invariants.py`): (1) **zero LALR conflicts** (dependency-free, always-on in CI) plus a `strict=True` build test (skipped where the optional `interegular` dep is absent) — a grammar edit that introduces any ambiguity fails CI; (2) semantic ambiguity is ZERO — every corpus query has exactly one Earley derivation and it transforms to a single AST, no exceptions; (3) a rule-coverage gate (`test_every_grammar_rule_is_exercised_by_the_corpus`) forces every new grammar rule through the invariants; (4) differential tests run the production pipeline under both parsers (byte-identical ASTs, identical rejections). **Full-repo differentials** (1,850+ scraped queries, LALR-vs-Earley and old-vs-new production): **zero AST divergences**, and a small set of deliberate language fixes — accept-by-accident shapes with ill-defined semantics, now honest syntax errors and pinned as tests (`DELIBERATE_LANGUAGE_FIXES`): `RETURN DISTINCT` with no items (Earley re-lexed DISTINCT as a variable name), double WHERE (the old positional attachment kept *both* predicates in different AST fields), double post-WITH WHERE, WHERE after UNWIND, WHERE before MATCH in a graph constructor, and the invalid "list of IN-booleans" (`[x IN xs, y]`; use `[(x IN xs), y]` for a genuine bool list). The lift stays an internal optimization: only a flat `AND` chain over present columns lifts to `filter_dict`; parens / OR / XOR / NOT and any absent-column case stay on `where_rows` (a correctness boundary, since `where_rows` treats an absent property as null while `filter_dict` would raise). Full cypher suite (1,681 tests) passes. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. A pre-existing `<>`-over-null 3VL divergence between the two execution paths (surfaced by #1653's metamorphic test) is tracked separately in #1683. ### Fixed diff --git a/graphistry/compute/gfql/cypher/parser.py b/graphistry/compute/gfql/cypher/parser.py index e692fd5d40..d4814ad9a6 100644 --- a/graphistry/compute/gfql/cypher/parser.py +++ b/graphistry/compute/gfql/cypher/parser.py @@ -1636,18 +1636,9 @@ def query_item(self, meta: Any, items: Sequence[Any]) -> Any: return items[0] def query_body(self, meta: Any, items: Sequence[Any]) -> CypherQuery: - # The grammar bundles WHERE into its MATCH clause. Re-emit it as a - # follow-on item so the clause-sequence state machine below (which - # predates the bundling and encodes all ordering/support rules) - # processes the exact sequence the source text spells. - flat: List[Any] = [] - for item in items: - if isinstance(item, MatchClause) and item.where is not None: - flat.append(replace(item, where=None)) - flat.append(item.where) - else: - flat.append(item) - items = flat + # The grammar bundles WHERE onto its MATCH clause (there is no + # standalone WHERE item), so this state machine consumes + # ``MatchClause.where`` directly -- see the MATCH branch below. trailing_semicolon = any(str(item) == ";" for item in items) match_clauses: List[MatchClause] = [] reentry_match_clauses: List[MatchClause] = [] @@ -1690,40 +1681,30 @@ def query_body(self, meta: Any, items: Sequence[Any]) -> CypherQuery: line=item.span.line, column=item.span.column, ) - if seen_stage: - reentry_match_clauses.append(item) - reentry_where_clauses.append(None) - else: - match_clauses.append(item) - elif isinstance(item, WhereClause): - if call_clause is not None: + # WHERE is bundled onto its MATCH by the grammar. Route it as + # the source spells it: a primary MATCH keeps its WHERE on the + # clause (and it also becomes the query's top-level WHERE, + # last-clause-wins, so each MATCH scopes its own predicate); a + # post-WITH re-entry MATCH's WHERE lives in reentry_wheres with + # the clause itself carrying none. + match_where = item.where + if match_where is not None and call_clause is not None: raise _to_syntax_error( "Cypher WHERE is not supported with CALL in the current GFQL Cypher compiler; use YIELD/RETURN row expressions instead", - line=item.span.line, - column=item.span.column, + line=match_where.span.line, + column=match_where.span.column, ) - if reentry_match_clauses: - if not reentry_where_clauses: - raise _to_syntax_error( - "Cypher WHERE after post-WITH MATCH is not yet supported in the current GFQL Cypher compiler", - line=item.span.line, - column=item.span.column, - ) - if reentry_where_clauses[-1] is not None: - raise _to_syntax_error( - "Cypher only supports one WHERE clause per post-WITH MATCH stage in the current GFQL Cypher compiler", - line=item.span.line, - column=item.span.column, - ) - reentry_where_clauses[-1] = item - reentry_where_pending_with_idx = len(reentry_where_clauses) - 1 + if seen_stage: + reentry_match_clauses.append( + replace(item, where=None) if match_where is not None else item + ) + reentry_where_clauses.append(match_where) + if match_where is not None: + reentry_where_pending_with_idx = len(reentry_where_clauses) - 1 else: - # Associate the WHERE with its preceding MATCH clause - # so that MATCH ... WHERE ... OPTIONAL MATCH ... WHERE ... - # correctly scopes each predicate to its own clause. - if match_clauses and match_clauses[-1].where is None: - match_clauses[-1] = replace(match_clauses[-1], where=item) - where_clause = item + match_clauses.append(item) + if match_where is not None: + where_clause = match_where elif isinstance(item, CallClause): if call_clause is not None: raise _to_syntax_error( @@ -1927,16 +1908,9 @@ def graph_constructor_body(self, meta: Any, items: Sequence[Any]) -> Any: def graph_constructor(self, meta: Any, items: Sequence[Any]) -> GraphConstructor: span = _span_from_meta(meta) body_items = items[0] if items and isinstance(items[0], list) else list(items) - # Split grammar-bundled MATCH..WHERE back into the item sequence the - # constructor-body rules below were written against (see query_body). - flat: List[Any] = [] - for item in body_items: - if isinstance(item, MatchClause) and item.where is not None: - flat.append(replace(item, where=None)) - flat.append(item.where) - else: - flat.append(item) - body_items = flat + # WHERE is bundled onto its MATCH by the grammar; a constructor allows + # at most one, held here as the constructor's single top-level WHERE + # (matches themselves carry none). matches: List[MatchClause] = [] where: Optional[WhereClause] = None use: Optional[UseClause] = None @@ -1948,14 +1922,17 @@ def graph_constructor(self, meta: Any, items: Sequence[Any]) -> GraphConstructor "MATCH and CALL cannot be combined inside a graph constructor", line=item.span.line, column=item.span.column, ) - matches.append(item) - elif isinstance(item, WhereClause): - if where is not None: - raise _to_syntax_error( - "Only one WHERE clause is allowed inside a graph constructor", - line=item.span.line, column=item.span.column, - ) - where = item + match_where = item.where + if match_where is not None: + if where is not None: + raise _to_syntax_error( + "Only one WHERE clause is allowed inside a graph constructor", + line=match_where.span.line, column=match_where.span.column, + ) + where = match_where + matches.append(replace(item, where=None)) + else: + matches.append(item) elif isinstance(item, CallClause): if call is not None: raise _to_syntax_error(