diff --git a/CHANGELOG.md b/CHANGELOG.md index 08a8d80ab0..024c070559 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL Cypher `count(*)` short-circuit — O(1) instead of O(N) materialize**: a lone `RETURN count(*)` over a single node or edge pattern (`MATCH (n) RETURN count(*)`, `MATCH ()-[r]->() RETURN count(*)`) previously materialized the entire matched frame and ran a constant-key `group_by` just to count its rows. The lowering now emits a new `count_table` row op that reads the scanned table's height directly (or, when the pattern applies a filter, sums the boolean alias-mask column) in a single reduction — no full-frame copy, no group_by. Applies only to the provably-equivalent shapes: exactly one non-DISTINCT `count(*)`, no group keys / post-aggregate exprs / row-level `WHERE` / `UNWIND` / paging / multi-relationship binding, and either a pure node scan (`relationship_count == 0`) or a single relationship counted on its edge alias — every other shape (node-alias-over-relationship, multi-hop paths, `count(DISTINCT …)`, grouped counts) falls through to the general aggregate path unchanged. Engine-polymorphic across pandas/cuDF/polars/polars-gpu; differential parity verified on all four engines (`count_all_nodes`/`count_all_edges` cypher conformance cases + a `count_table` row-op subject case, chain and DAG surfaces). No change to any result value — only the execution path. - **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: 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 - **CI: `compute_networkx` HITS test was scipy-version flaky**: `test_compute_networkx_hits_outputs_hubs_and_authorities` used a pure directed 3-cycle (`a→b→c→a`), whose adjacency is a permutation matrix (all singular values equal). networkx HITS computes scores via scipy `svds(k=1)`, which then returns an arbitrary vector from that fully-degenerate singular space; when its components sum to ~0, HITS's `h /= h.sum()` normalization blows up to `inf`/`nan`. Which vector `svds` returns is scipy/LAPACK-version dependent, so the test passed on the pinned scipy but failed on the `nx-upper-scipy` CI profile's newer scipy — a latent flake, not a code regression. Switched to a graph with a well-defined hub/authority structure (`a→b`, `a→c`, `b→c`), whose dominant singular vector is unique (Perron-Frobenius) and therefore version-stable. Test-only change. - **GFQL `materialize_nodes()` on an edges-only graph under `engine='polars'`**: crashed with `AttributeError: 'Series' object has no attribute 'drop_duplicates'` — the node-id derivation used pandas-only `.drop_duplicates()` / `.reset_index()` / `.rename(columns=)` on a polars Series/DataFrame. Now polars-aware (`.unique(maintain_order=True)`, `.select().rename({...})`), matching the pandas oracle. Surfaced by any `let()` DAG on an edges-only graph under Polars (the DAG pre-materializes nodes). pandas/cuDF paths unchanged. diff --git a/graphistry/compute/gfql/cypher/ast.py b/graphistry/compute/gfql/cypher/ast.py index 0b9524fe52..3a26dbd24a 100644 --- a/graphistry/compute/gfql/cypher/ast.py +++ b/graphistry/compute/gfql/cypher/ast.py @@ -192,11 +192,12 @@ class WhereClause: populated by the parser; consumers MUST handle all three: - **Structured path**: ``predicates`` populated, ``expr_tree is None``. - ``predicates`` carries either ``WherePredicate`` entries (pure AND of - comparable / has-labels predicates routed via the ``where_predicates`` - grammar rule, or AND-joined bare label predicates lifted by - ``generic_where_clause`` via label narrowing) or a single - ``WherePatternPredicate`` (pattern-only WHERE: ``WHERE (n)-[]->(m)``). + ``predicates`` carries either ``WherePredicate`` entries (a flat top-level + AND of simple predicates -- cmp / IS NULL / CONTAINS / STARTS/ENDS WITH / + has-labels -- lifted post-parse from the generic ``expr_tree`` by + ``generic_where_clause`` via ``_lift_label_only_and_spine`` / + ``_lift_and_spine_predicates``) or a single ``WherePatternPredicate`` + (pattern-only WHERE: ``WHERE (n)-[]->(m)``). - **Tree path**: ``predicates == ()``, ``expr_tree`` populated. Fires when ``generic_where_clause`` cannot lift to structured predicates (OR / XOR / NOT / parenthesized boolean / non-label atoms); consumers diff --git a/graphistry/compute/gfql/cypher/parser.py b/graphistry/compute/gfql/cypher/parser.py index 5f02788051..53138e3589 100644 --- a/graphistry/compute/gfql/cypher/parser.py +++ b/graphistry/compute/gfql/cypher/parser.py @@ -44,6 +44,31 @@ ) +# --------------------------------------------------------------------------- +# GRAMMAR — the declarative source of truth for Cypher syntax. +# +# EDITING THIS GRAMMAR (the safe-by-construction extension flow): +# 1. Add / change a rule here. +# 2. Add at least one query exercising the new syntax to +# DIFFERENTIAL_CORPUS in tests/.../test_grammar_invariants.py (or, if the +# construct is grammatical-but-intentionally-unsupported, to +# GRAMMAR_ONLY_COVERAGE). test_every_grammar_rule_is_exercised_by_the_corpus +# FAILS with the new rule's name until you do — you cannot land a rule +# with no coverage. +# 3. Run test_grammar_invariants.py. The machine checks that guard you: +# - the grammar has ZERO LALR conflicts and builds under strict=True — +# provably unambiguous; a new ambiguity introduces a conflict and +# fails the build. Fix it IN THE GRAMMAR (as WITH..WHERE bundling, +# the dotted-name split, and no-top-level-IN list elements did) — +# never by resolving a conflict in Python; +# - semantic ambiguity is ZERO (every Earley derivation -> same AST); +# - LALR == Earley over the corpus + full-repo scrape (AST-identical). +# 4. If the edit deliberately changes accept/reject, pin it in +# DELIBERATE_LANGUAGE_FIXES. Otherwise a language change fails the +# differential. +# The grammar carries the correctness argument; the tests make its properties +# machine-checked. Do not resolve ambiguity in Python — fix it in the grammar. +# --------------------------------------------------------------------------- _GRAMMAR = r""" ?start: graph_query @@ -57,7 +82,6 @@ graph_constructor_body: graph_constructor_item+ graph_constructor_item: match_clause - | where_clause | call_clause | use_clause @@ -69,7 +93,6 @@ | "UNION"i -> union_distinct query_item: match_clause - | where_clause | call_clause | unwind_clause | use_clause @@ -81,8 +104,12 @@ with_stage: with_clause with_where_clause? order_by_clause? skip_clause? limit_clause? return_stage: return_clause order_by_clause? skip_clause? limit_clause? -match_clause: "MATCH"i match_item ("," match_item)* -> match_clause - | "OPTIONAL"i "MATCH"i match_item ("," match_item)* -> optional_match_clause +// WHERE binds to its preceding clause IN THE GRAMMAR (openCypher scoping): +// after MATCH it is part of match_clause; after WITH it is with_where_clause. +// A standalone where_clause item would make `MATCH .. WHERE ..` derivable two +// ways (bundled vs standalone) -- a genuine ambiguity -- so it does not exist. +match_clause: "MATCH"i match_item ("," match_item)* where_clause? -> match_clause + | "OPTIONAL"i "MATCH"i match_item ("," match_item)* where_clause? -> optional_match_clause match_item: pattern | NAME "=" pattern -> bound_pattern | NAME "=" "shortestPath"i "(" pattern ")" -> shortest_path_pattern @@ -120,9 +147,11 @@ properties: "{" [property_entry ("," property_entry)*] "}" property_entry: NAME ":" expr -where_clause: "WHERE"i where_predicates - | "WHERE"i expr -> generic_where_clause -where_predicates: where_predicate ("AND"i where_predicate)* +// Unified: every WHERE parses as a generic boolean ``expr`` (so LALR(1) accepts +// OR/XOR/NOT/parenthesized clauses, no Earley). ``generic_where_clause`` lifts the +// structured (filter_dict) predicates back out of the parsed tree. ``where_predicate`` +// is retained as the building block for the ``where_predicate_chain`` lift parser. +where_clause: "WHERE"i expr -> generic_where_clause where_predicate: property_ref COMP_OP where_rhs -> cmp_where | property_ref "IS"i "NULL"i -> is_null_where | property_ref "IS"i "NOT"i "NULL"i -> is_not_null_where @@ -130,6 +159,10 @@ | property_ref "STARTS"i "WITH"i where_rhs -> starts_with_where | property_ref "ENDS"i "WITH"i where_rhs -> ends_with_where | variable labels -> has_labels_where +// Flat AND-chain of bare predicates; the start symbol for the lift parser. Parens +// / OR / XOR / NOT are not ``where_predicate``s, so such clauses fail this parse and +// stay on the row-filter path (which treats a missing property as null). +where_predicate_chain: where_predicate ("AND"i where_predicate)* where_rhs: property_ref | value @@ -142,14 +175,16 @@ yield_item: NAME alias? with_clause: "WITH"i distinct? return_item ("," return_item)* -with_where_clause.2: "WHERE"i expr +with_where_clause: "WHERE"i expr return_clause: "RETURN"i distinct? return_item ("," return_item)* distinct: "DISTINCT"i return_item: return_expr alias? +// A parenthesized label predicate ``(n:Admin)`` parses via ``expr`` -> +// ``grouped_expr`` over ``bare_label_predicate_expr`` (identical AST), so a +// dedicated ``label_predicate_expr`` rule here would be a pure derivation +// redundancy -- exactly the RPAR shift/reduce conflict. Omitted on purpose. return_expr: "*" -> projection_star - | label_predicate_expr | expr -label_predicate_expr: "(" NAME labels ")" -> grouped_label_predicate bare_label_predicate_expr: NAME labels -> bare_label_predicate alias: "AS"i NAME @@ -209,14 +244,20 @@ | MINUS unary -> uminus | postfix -?postfix: primary +// Dotted chains rooted at a bare NAME derive ONLY via qualified_name (a.b.c); +// property_access applies ONLY to composite roots (calls, groups, subscripts: +// f(x).y, (e).y, xs[0].y). Splitting the two keeps the grammar unambiguous -- +// a single derivation for every dotted expression -- and LALR(1) conflict-free +// at DOT (no reduce-qualified_name vs shift-extend race). +?postfix: qualified_name + | postfix_composite +?postfix_composite: primary_composite | postfix "[" subscript_key "]" -> subscript - | postfix "." NAME -> property_access + | postfix_composite "." NAME -> property_access -?primary: parameter +?primary_composite: parameter | literal | function_call - | qualified_name | case_expr | quantifier_expr | list_comprehension @@ -246,8 +287,35 @@ case_when: "WHEN"i expr "THEN"i expr case_else: "ELSE"i expr -list_literal: "[" [expr_list] "]" -expr_list: expr ("," expr)* +// A list literal's elements are expressions with NO top-level ``IN`` operator. +// Rationale: ``[x IN xs ...`` is list-comprehension syntax; if a list element +// could also be a bare ``in_op`` (``[x IN xs, y]``), the two overlap and the +// grammar is ambiguous at ``[ NAME . IN`` (this was the last shift/reduce +// conflict). Excluding top-level ``IN`` from list elements makes the grammar +// provably unambiguous (builds under ``strict=True``) AND rejects the invalid +// "list of IN-booleans" uniformly (``[1 IN a, 2]``, ``[n.x IN a, y]`` too, not +// just the bare-name case) — a construct GFQL cannot execute anyway. A genuine +// list of membership booleans is still expressible with parens: ``[(x IN xs)]``. +// ``list_element`` mirrors the ``expr`` hierarchy exactly, minus the ``in_op`` +// alternative at the comparable level; ``additive`` and below are shared. +list_literal: "[" [list_element_list] "]" +list_element_list: list_element ("," list_element)* +?list_element: or_expr_no_in +?or_expr_no_in: xor_expr_no_in | or_expr_no_in "OR"i xor_expr_no_in -> or_op +?xor_expr_no_in: and_expr_no_in | xor_expr_no_in "XOR"i and_expr_no_in -> xor_op +?and_expr_no_in: not_expr_no_in | and_expr_no_in "AND"i not_expr_no_in -> and_op +?not_expr_no_in: "NOT"i not_expr_no_in -> not_op + | predicate_no_in +?predicate_no_in: comparable_no_in + | comparable_no_in COMP_OP comparable_no_in COMP_OP comparable_no_in -> chained_cmp + | comparable_no_in COMP_OP comparable_no_in -> cmp_op + | bare_label_predicate_expr +?comparable_no_in: additive + | additive "IS"i "NULL"i -> expr_is_null + | additive "IS"i "NOT"i "NULL"i -> expr_is_not_null + | additive "CONTAINS"i additive -> contains_op + | additive "STARTS"i "WITH"i additive -> starts_with_op + | additive "ENDS"i "WITH"i additive -> ends_with_op map_literal: "{" [map_entries] "}" map_entries: map_entry ("," map_entry)* @@ -533,15 +601,15 @@ class _BoundPattern: @lru_cache(maxsize=1) -def _parser() -> _ParserLike: +def _parser_lalr() -> _ParserLike: Lark, _, _, _ = _lark_imports() - # Earley required: LALR(1) cannot resolve `comparable AND WHERE_PATTERN` - # where WHERE_PATTERN is a leaf in `?primary`. Ambiguity defaults to - # Lark's "resolve" mode (highest-priority derivation). + # Sole whole-query parser. The unified WHERE grammar is LALR(1)-parseable, so + # this accepts every supported query (~80x faster than the former Earley + # parser); generic_where_clause lifts the structured predicates back out. parser = Lark( _GRAMMAR, start="start", - parser="earley", + parser="lalr", maybe_placeholders=False, propagate_positions=True, ) @@ -551,16 +619,88 @@ def _parser() -> _ParserLike: @lru_cache(maxsize=1) def _pattern_parser() -> _ParserLike: Lark, _, _, _ = _lark_imports() + # Parses a single pattern fragment (e.g. ``(n)-[:R]->()``) for WHERE pattern + # predicates. LALR(1) accepts the pattern grammar, so no Earley is needed here. parser = Lark( _GRAMMAR, start="pattern", - parser="earley", + parser="lalr", maybe_placeholders=False, propagate_positions=True, ) return cast(_ParserLike, parser) +@lru_cache(maxsize=1) +def _where_predicate_chain_parser() -> _ParserLike: + Lark, _, _, _ = _lark_imports() + # Parses a flat ``where_predicate ("AND" where_predicate)*`` chain to lift a WHERE + # body into structured (filter_dict) form. Parens / OR / XOR / NOT / arithmetic make + # the body NOT a flat chain, so this parse fails and the clause stays on the + # ``where_rows`` row-filter path (see _lift_and_spine_predicates for why). + parser = Lark( + _GRAMMAR, + start="where_predicate_chain", + parser="lalr", + maybe_placeholders=False, + propagate_positions=True, + ) + return cast(_ParserLike, parser) + + +def _retarget_span(s: SourceSpan, base: SourceSpan) -> SourceSpan: + """Shift an atom-relative span (from re-parsing an isolated atom substring) into + absolute query coordinates, given the atom's absolute ``base`` span. Exact for + single-line atoms (the normal case); best-effort across embedded newlines.""" + return SourceSpan( + line=base.line + (s.line - 1), + column=(base.column + s.column - 1) if s.line == 1 else s.column, + end_line=base.line + (s.end_line - 1), + end_column=(base.column + s.end_column - 1) if s.end_line == 1 else s.end_column, + start_pos=base.start_pos + s.start_pos, + end_pos=base.start_pos + s.end_pos, + ) + + +def _retarget_predicate_spans(pred: "WherePredicate", base: SourceSpan) -> "WherePredicate": + """Rebuild a lifted predicate with every span shifted from atom-relative to + absolute (see ``_retarget_span``), so downstream errors (e.g. E108 in lowering) + point at the predicate's real position in the query instead of column 1.""" + left = replace(pred.left, span=_retarget_span(pred.left.span, base)) + right = pred.right + if isinstance(right, (PropertyRef, ParameterRef)): + right = replace(right, span=_retarget_span(right.span, base)) + return replace(pred, left=left, right=right, span=_retarget_span(pred.span, base)) + + +def _lift_and_spine_predicates( + expr_text: str, base_span: Optional[SourceSpan] = None +) -> Optional[List["WherePredicate"]]: + """Lift the WHERE body to structured ``filter_dict`` predicates iff it parses as a + flat ``where_predicate ("AND" where_predicate)*`` chain (cmp / IS NULL / CONTAINS / + STARTS|ENDS WITH / label); else ``None``, leaving it on ``expr_tree`` -> ``where_rows``. + + Only flat chains lift; parentheses, OR / XOR / NOT and arithmetic stay on + ``where_rows``. This is a correctness boundary, not just routing: a parenthesized + predicate over a *missing* property (e.g. ``a IS NULL AND (b = 1)`` where ``b`` is + absent) must stay on ``where_rows``, which treats an absent property as null + (Cypher semantics); ``filter_dict`` requires the column to exist and would raise. + + ``base_span`` is the WHERE body's absolute position; the body is parsed in isolation + (spans relative to it), so predicate spans are shifted back to absolute.""" + _, _, LarkError, _ = _lark_imports() + try: + tree = _where_predicate_chain_parser().parse(expr_text) + preds = _build_transformer(expr_text).transform(tree) + except (LarkError, GFQLSyntaxError, GFQLValidationError): + return None + if not (isinstance(preds, tuple) and preds and all(isinstance(p, WherePredicate) for p in preds)): + return None + if base_span is None: + return list(preds) + return [_retarget_predicate_spans(p, base_span) for p in preds] + + def _to_syntax_error(message: str, *, line: Optional[int] = None, column: Optional[int] = None, **extra: Any) -> GFQLSyntaxError: return GFQLSyntaxError( ErrorCode.E107, @@ -838,6 +978,13 @@ def optional_match_clause(self, meta: Any, items: Sequence[Any]) -> MatchClause: return self._match_clause(meta, items, optional=True) def _match_clause(self, meta: Any, items: Sequence[Any], *, optional: bool) -> MatchClause: + # The grammar bundles a trailing WHERE into the match_clause (WHERE + # binds to its preceding clause declaratively). Split it back off; + # query_body's assembly consumes it as if it were a standalone item. + where: Optional[WhereClause] = None + if items and isinstance(items[-1], WhereClause): + where = items[-1] + items = items[:-1] if len(items) < 1: clause_name = "OPTIONAL MATCH" if optional else "MATCH" raise _to_syntax_error(f"Cypher {clause_name} clause cannot be empty", line=meta.line, column=meta.column) @@ -853,12 +1000,26 @@ def _match_clause(self, meta: Any, items: Sequence[Any], *, optional: bool) -> M patterns.append(cast(Tuple[PatternElement, ...], item)) pattern_aliases.append(None) pattern_alias_kinds.append("pattern") + # Span covers the MATCH..patterns text only (not the bundled WHERE), + # preserving the clause span the AST has always carried: end at the + # last non-whitespace character before the WHERE keyword. + span = _span_from_meta(meta) + if where is not None: + end_pos = span.start_pos + len(source[span.start_pos:where.span.start_pos].rstrip()) + line_start = source.rfind("\n", 0, end_pos) + 1 + span = replace( + span, + end_pos=end_pos, + end_line=source.count("\n", 0, end_pos) + 1, + end_column=end_pos - line_start + 1, + ) return MatchClause( patterns=tuple(patterns), - span=_span_from_meta(meta), + span=span, optional=optional, pattern_aliases=tuple(pattern_aliases), pattern_alias_kinds=tuple(pattern_alias_kinds), + where=where, ) def distinct(self, _meta: Any, _items: Sequence[Any]) -> bool: @@ -876,6 +1037,10 @@ def where_rhs(self, _meta: Any, items: Sequence[Any]) -> object: raise _to_syntax_error("Invalid WHERE right-hand side") return _unwrap_primitive_literal(items[0]) + def where_predicate_chain(self, _meta: Any, items: Sequence[Any]) -> Tuple[WherePredicate, ...]: + # Flat AND-chain of bare predicates (the lift parser's start symbol). + return tuple(cast(WherePredicate, p) for p in items) + def cmp_where(self, meta: Any, items: Sequence[Any]) -> WherePredicate: if len(items) != 3: raise _to_syntax_error("Invalid WHERE comparison", line=meta.line, column=meta.column) @@ -938,9 +1103,6 @@ def has_labels_where(self, meta: Any, items: Sequence[Any]) -> WherePredicate: span=_span_from_meta(meta), ) - def where_predicates(self, _meta: Any, items: Sequence[Any]) -> Tuple[WherePredicate, ...]: - return tuple(items) - def _parse_single_where_pattern_predicate_text(self, pattern_item_text: str, span: SourceSpan) -> WherePatternPredicate: """Parse one bare-pattern-item text (no AND-chain) into a WherePatternPredicate. @@ -1101,36 +1263,37 @@ def not_op(self, meta: Any, items: Sequence[Any]) -> BooleanExpr: left=self._wrap_as_boolean_atom(items[0], meta), ) - def where_clause(self, meta: Any, items: Sequence[Any]) -> WhereClause: - if len(items) != 1: - raise _to_syntax_error("WHERE clause cannot be empty", line=meta.line, column=meta.column) - predicates = cast(Tuple[WherePredicate, ...], items[0]) - if len(predicates) == 0: - raise _to_syntax_error("WHERE clause cannot be empty", line=meta.line, column=meta.column) - return WhereClause(predicates=cast(Any, predicates), span=_span_from_meta(meta)) - def generic_where_clause(self, meta: Any, items: Sequence[Any]) -> WhereClause: span = _span_from_meta(meta) expr_text = self._slice(span)[len("WHERE"):].strip() + # Absolute span of the WHERE body (drops "WHERE" + whitespace): aligns the + # synth single-atom node and is the lift's span base (the lift parses + # ``expr_text`` in isolation, so its spans need shifting to absolute). + _body = self._slice(span)[len("WHERE"):] + _start_off = len("WHERE") + (len(_body) - len(_body.lstrip())) + body_span = replace( + span, + column=span.column + _start_off, + start_pos=span.start_pos + _start_off, + end_column=span.column + _start_off + len(expr_text), + end_pos=span.start_pos + _start_off + len(expr_text), + ) # Capture Lark's structural expression tree. Only ``and_op`` / # ``or_op`` / ``xor_op`` / ``not_op`` produce ``BooleanExpr``; # atomic WHERE expressions (single predicate) route here with a # non-BooleanExpr operand and are wrapped as a single-atom tree # so the invariant ``(expr is None) == (expr_tree is None)`` # holds (#1213 sub-PR A / #1214). - expr_tree: BooleanExpr = ( - cast(BooleanExpr, items[0]) - if items and isinstance(items[0], BooleanExpr) - else BooleanExpr(op="atom", span=span, atom_text=expr_text, atom_span=span) - ) - # Lark's ambiguity resolution prefers the generic `expr` path over - # `where_predicates` (#1194), so bare label predicates and AND - # chains of them land here as a ``BooleanExpr``. Walk the parsed - # tree (single source of truth) to lift them to structured - # predicates instead of re-splitting the WHERE body text on - # top-level ``AND``. Any non-AND boolean op, non-atom node, or - # non-bare-label atom causes a conservative fallback to raw expr - # (preserved on ``expr_tree`` for downstream consumers). + if items and isinstance(items[0], BooleanExpr): + expr_tree: BooleanExpr = cast(BooleanExpr, items[0]) + else: + expr_tree = BooleanExpr(op="atom", span=body_span, atom_text=expr_text, atom_span=body_span) + # The unified grammar parses every WHERE as a generic `expr`, so bare + # label predicates and AND chains of them arrive here as a ``BooleanExpr``. + # Walk the parsed tree (single source of truth) to lift them to structured + # predicates instead of re-splitting the WHERE body text on top-level + # ``AND``. Any non-AND boolean op, non-atom node, or non-bare-label atom + # causes a conservative fallback to raw expr (preserved on ``expr_tree``). lifted = _lift_label_only_and_spine(expr_tree) if lifted: predicates = tuple( @@ -1161,6 +1324,12 @@ def generic_where_clause(self, meta: Any, items: Sequence[Any]) -> WhereClause: expr_text=expr_text, span=span, ) + # Lift a flat AND chain of bare predicates to structured ``filter_dict`` + # predicates; parens / OR / XOR / NOT keep the clause on ``expr_tree`` -> + # ``where_rows`` (see _lift_and_spine_predicates for why parens matter). + lifted_preds = _lift_and_spine_predicates(expr_text, body_span) + if lifted_preds is not None: + return WhereClause(predicates=tuple(lifted_preds), span=span) return WhereClause( predicates=(), expr_tree=expr_tree, @@ -1245,7 +1414,7 @@ def projection_star(self, meta: Any, _items: Sequence[Any]) -> _ExpressionSlice: span = _span_from_meta(meta) return _ExpressionSlice(text="*", span=span) - def grouped_label_predicate(self, meta: Any, items: Sequence[Any]) -> _ExpressionSlice: + def bare_label_predicate(self, meta: Any, items: Sequence[Any]) -> _ExpressionSlice: if len(items) != 2: raise _to_syntax_error("Invalid label predicate expression", line=meta.line, column=meta.column) labels = cast(Sequence[str], items[1]) @@ -1254,8 +1423,6 @@ def grouped_label_predicate(self, meta: Any, items: Sequence[Any]) -> _Expressio span = _span_from_meta(meta) return _ExpressionSlice(text=self._slice(span), span=span) - bare_label_predicate = grouped_label_predicate - def _projection_clause( self, *, @@ -1377,6 +1544,18 @@ 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 trailing_semicolon = any(str(item) == ";" for item in items) match_clauses: List[MatchClause] = [] reentry_match_clauses: List[MatchClause] = [] @@ -1656,6 +1835,16 @@ 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 matches: List[MatchClause] = [] where: Optional[WhereClause] = None use: Optional[UseClause] = None @@ -1873,10 +2062,9 @@ def _parse_cypher_cached(query: str) -> Union[CypherQuery, CypherUnionQuery, Cyp # Pre-parse detection of known-but-unsupported Cypher forms _check_unsupported_syntax_patterns(query) - parser = _parser() transformer = _build_transformer(query) try: - tree = parser.parse(query) + tree = _parser_lalr().parse(query) node = transformer.transform(tree) except Exception as exc: _, _, LarkError, _ = _lark_imports() diff --git a/graphistry/tests/compute/gfql/cypher/test_differential_grammar_parity.py b/graphistry/tests/compute/gfql/cypher/test_differential_grammar_parity.py new file mode 100644 index 0000000000..4cc25188f3 --- /dev/null +++ b/graphistry/tests/compute/gfql/cypher/test_differential_grammar_parity.py @@ -0,0 +1,134 @@ +"""Full-repo LALR==Earley grammar-parity differential (CI gate). + +Scrapes every cypher-looking string literal from the graphistry package +(python ``ast`` over all ``.py`` files — tests, source, corpora) and runs each +through the PRODUCTION pipeline twice: once with the production LALR parser +and once with an Earley parser built from the SAME grammar. Asserts: + +- every query both parsers accept produces a byte-identical AST; +- both parsers reject the same queries (rejection parity), with the same + error class — EXCEPT the pinned deliberate language fixes (see + ``DELIBERATE_LANGUAGE_FIXES`` in test_grammar_invariants.py), where + Earley's dynamic lexer accepts a lexing accident that LALR correctly + rejects. + +This is the broad-corpus complement to the constructed corpus in +``test_grammar_invariants.py``: it sweeps everything the repo actually says, +so a grammar edit that shifts parser behavior on ANY in-repo query fails +here. The file name matches the ``test_differential*.py`` glob of the +``cypher-frontend-differential-parity`` CI job. +""" +from __future__ import annotations + +import ast as pyast +import os +from typing import Any, List, Set, Tuple + +import pytest + +from graphistry.compute.gfql.cypher import parse_cypher +from graphistry.compute.gfql.cypher import parser as parser_mod + +lark = pytest.importorskip("lark") + +# Queries where LALR (correctly) rejects what Earley's dynamic lexer +# accepted by accident — the pinned language fixes. Loaded from the sibling +# invariants module by path (the tests tree is not a package). +import importlib.util as _ilu # noqa: E402 + +_spec = _ilu.spec_from_file_location( + "_tgi", os.path.join(os.path.dirname(__file__), "test_grammar_invariants.py") +) +assert _spec is not None and _spec.loader is not None +_tgi = _ilu.module_from_spec(_spec) +_spec.loader.exec_module(_tgi) +DELIBERATE_LANGUAGE_FIXES = _tgi.DELIBERATE_LANGUAGE_FIXES + +_PKG_ROOT = os.path.abspath( + os.path.join(os.path.dirname(parser_mod.__file__), "..", "..", "..") +) + + +def _looks_cypher(s: str) -> bool: + if not (10 <= len(s) <= 4000) or "\x00" in s: + return False + return "RETURN" in s or "MATCH" in s + + +def _scrape_corpus() -> List[str]: + corpus: Set[str] = set() + for dirpath, dirnames, filenames in os.walk(_PKG_ROOT): + dirnames[:] = [d for d in dirnames if d != "__pycache__"] + for fn in filenames: + if not fn.endswith(".py"): + continue + try: + with open(os.path.join(dirpath, fn), encoding="utf-8") as f: + tree = pyast.parse(f.read()) + except (SyntaxError, OSError): + continue + for node in pyast.walk(tree): + if isinstance(node, pyast.Constant) and isinstance(node.value, str): + s = node.value.strip() + if _looks_cypher(s): + corpus.add(s) + return sorted(corpus) + + +def _run_with(parser: Any, query: str, monkeypatch: pytest.MonkeyPatch) -> Tuple[str, str]: + monkeypatch.setattr(parser_mod, "_parser_lalr", lambda: parser) + parser_mod._parse_cypher_cached.cache_clear() + try: + return ("OK", repr(parse_cypher(query))) + except Exception as exc: # both syntax + validation rejections count + return ("REJECT", type(exc).__name__) + finally: + monkeypatch.undo() + parser_mod._parse_cypher_cached.cache_clear() + + +def test_full_repo_lalr_earley_differential(monkeypatch: pytest.MonkeyPatch) -> None: + earley = lark.Lark( + parser_mod._GRAMMAR, + start="start", + parser="earley", + maybe_placeholders=False, + propagate_positions=True, + ) + lalr = parser_mod._parser_lalr() + corpus = _scrape_corpus() + assert len(corpus) > 500, "corpus scrape looks broken (too few queries)" + + known_fixes = set(DELIBERATE_LANGUAGE_FIXES) + ast_diverge: List[str] = [] + lang_diverge: List[str] = [] + err_diverge: List[str] = [] + n_ok = n_rej = 0 + for query in corpus: + lalr_status, lalr_val = _run_with(lalr, query, monkeypatch) + earley_status, earley_val = _run_with(earley, query, monkeypatch) + if lalr_status == earley_status == "OK": + n_ok += 1 + if lalr_val != earley_val: + ast_diverge.append(query) + elif lalr_status == earley_status == "REJECT": + n_rej += 1 + if lalr_val != earley_val: + err_diverge.append(f"{query!r}: LALR={lalr_val} Earley={earley_val}") + elif query in known_fixes: + assert lalr_status == "REJECT", ( + f"pinned language fix should be LALR-rejected: {query!r}" + ) + else: + lang_diverge.append(f"{query!r}: LALR={lalr_status} Earley={earley_status}") + + assert not ast_diverge, ( + f"AST divergence on {len(ast_diverge)} queries, e.g.:\n" + + "\n".join(repr(q) for q in ast_diverge[:10]) + ) + assert not lang_diverge, ( + "unpinned accept/reject divergence (add to DELIBERATE_LANGUAGE_FIXES " + "only if deliberate):\n" + "\n".join(lang_diverge[:10]) + ) + assert not err_diverge, "error-class divergence:\n" + "\n".join(err_diverge[:10]) + assert n_ok > 500 and n_rej > 50, f"corpus balance unexpected: ok={n_ok} rej={n_rej}" diff --git a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py new file mode 100644 index 0000000000..05c1049c57 --- /dev/null +++ b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py @@ -0,0 +1,397 @@ +"""Machine-checkable grammar invariants for the GFQL Cypher LALR parser. + +The parser's correctness argument is grammar-level, not implementation-level: + +1. **The grammar is PROVABLY UNAMBIGUOUS LALR(1)** — zero conflicts. + ``test_grammar_has_zero_lalr_conflicts`` asserts the LALR build emits no + shift/reduce and no reduce/reduce conflicts (dependency-free), and + ``test_grammar_builds_under_strict_mode`` confirms it builds under Lark's + ``strict=True`` (also checks lexer-terminal collisions; needs the optional + ``interegular`` dep, skipped if absent). This is the strongest form of the + "ambiguity is machine-checkable" invariant: a single derivation for every + input, verified at build time. A grammar edit that introduces ANY conflict + fails here. + +2. **Semantic ambiguity is ZERO** (``test_semantic_ambiguity_zero``): + expanding EVERY Earley derivation of every corpus query + (``ambiguity='explicit'`` + ``CollapseAmbiguities``), each query has + exactly one derivation and they all transform to the SAME AST — a + redundant but independent confirmation of (1) at the AST level. The + WITH..WHERE attachment ambiguity, the dotted-name redundancy, the + ``(n:Admin)`` label predicate, and the ``[x IN xs]`` comprehension overlap + were all eliminated at grammar level (see the grammar's inline comments). + +3. **Parser-choice neutrality** (``test_lalr_ast_equals_earley_ast_differential``): + running the production pipeline with an Earley parser over the same grammar + yields byte-identical ASTs to the LALR parser across the corpus, and both + reject the same invalid inputs. LALR is a pure speed change (~70x). + +Together: the grammar (a declarative artifact) carries the correctness +argument, and these tests make its safety properties machine-checked so +syntactic extensions cannot silently regress them. +""" +from __future__ import annotations + +import logging +import re +from typing import Any, List + +import pytest + +from graphistry.compute.exceptions import GFQLSyntaxError +from graphistry.compute.gfql.cypher import parse_cypher +from graphistry.compute.gfql.cypher import parser as parser_mod + +lark = pytest.importorskip("lark") + + +# --- Corpus -------------------------------------------------------------------- +# One query per grammar construct family. Every query must parse in production. + +DIFFERENTIAL_CORPUS = [ + # patterns + "MATCH (n) RETURN n", + "MATCH (n:Person) RETURN n", + "MATCH (n:Person {id: 1, name: 'x'}) RETURN n", + "MATCH (a)-[r]->(b) RETURN a, r, b", + "MATCH (a)<-[r:KNOWS]-(b) RETURN a", + "MATCH (a)-[r:A|B|C]-(b) RETURN r", + "MATCH (a)-[r:KNOWS*1..3]->(b) RETURN b", + "MATCH (a)-[:R*]->(b) RETURN b", + "MATCH p = shortestPath((a:X)-[:R*1..4]->(b:Y)) RETURN p", + "MATCH (a)-[:R*2..]->(b) RETURN b", + "MATCH (a)-[:R]->(b), (b)-[:S]->(c) RETURN a, c", + "OPTIONAL MATCH (n)-[r]->(m) RETURN n, m", + "MATCH (n) MATCH (m) RETURN n, m", + # WHERE: structured predicate forms (flat AND chain lifts) + "MATCH (n) WHERE n.x = 50 RETURN n", + "MATCH (n) WHERE n.x <> 1 AND n.y >= 2 AND n.z < 3 RETURN n", + "MATCH (n) WHERE n.name CONTAINS 'ab' RETURN n", + "MATCH (n) WHERE n.name STARTS WITH 'a' AND n.name ENDS WITH 'z' RETURN n", + "MATCH (n) WHERE n.x IS NULL AND n.y IS NOT NULL RETURN n", + "MATCH (n) WHERE n:Admin AND n.active = true RETURN n", + "MATCH (a), (b) WHERE a.id = b.id RETURN a", + "MATCH (n) WHERE n.x = $param RETURN n", + # WHERE: generic expression forms (stay on expr_tree) + "MATCH (n) WHERE n.a > 3 OR n.b = 1 RETURN n", + "MATCH (n) WHERE n.x = 1 XOR n.y = 2 RETURN n", + "MATCH (n) WHERE NOT n.deleted = true RETURN n", + "MATCH (n) WHERE (n.x = 1 AND n.y = 2) OR n.z = 3 RETURN n", + "MATCH (n) WHERE n.x = 1 AND (n.y = 2 AND n.z = 3) RETURN n", + "MATCH (n) WHERE n.x IN [1, 2, 3] RETURN n", + "MATCH (n) WHERE n.x + 1 > n.y * 2 RETURN n", + "MATCH (n) WHERE NOT (n)-[:BLOCKED]->() RETURN n", + # RETURN / projection + "MATCH (n) RETURN DISTINCT n.city AS city", + "MATCH (n) RETURN count(n) AS c, sum(n.x) AS s", + "MATCH (n) RETURN n.a, n.b ORDER BY n.a ASC, n.b DESC SKIP 2 LIMIT 5", + "MATCH (n) RETURN CASE WHEN n.x > 1 THEN 'hi' ELSE 'lo' END AS bucket", + "MATCH (n) RETURN CASE n.x WHEN 1 THEN 'a' ELSE 'b' END AS k", + "RETURN 1 AS one", + # WITH pipelines + "MATCH (n) WITH n.city AS city, count(n) AS c WHERE c > 1 RETURN city, c", + "MATCH (n) WITH n ORDER BY n.x LIMIT 3 RETURN n.id", + "MATCH (n:Person) WITH n AS m WHERE m.age > 1 RETURN m", + "MATCH (a)-[r]->(x) WITH a, x WHERE a.animal = x.animal RETURN a, x", + "MATCH (a:A)-[:R]->(b) WITH collect(b) AS bs UNWIND bs AS b2 MATCH (b2)-[:S]->(c) RETURN c", + "MATCH (n) WITH n WHERE n.x = 1 MATCH (m) WHERE m.y = n.x RETURN m", + # UNWIND / CALL / UNION / params + "UNWIND [1, 2, 3] AS x RETURN x", + "MATCH (n) UNWIND n.tags AS t RETURN t", + "CALL gds.pageRank({maxIter: 10}) YIELD nodeId, score RETURN nodeId, score", + "MATCH (a:X) RETURN a.id AS id UNION MATCH (b:Y) RETURN b.id AS id", + "MATCH (a:X) RETURN a.id AS id UNION ALL MATCH (b:Y) RETURN b.id AS id", + # graph constructor / USE + "GRAPH g1 = GRAPH { MATCH (a)-[r]->(b) WHERE a.id = 'x' } USE g1 MATCH (n) RETURN n", + # comprehensions / quantifiers / subscripts / composite property access + "MATCH (n) RETURN [x IN n.tags WHERE x > 1 | x + 1] AS t", + "MATCH (n) RETURN [x IN n.tags WHERE x > 1] AS t", + "MATCH (n) RETURN [x IN n.tags | x + 1] AS t", + "MATCH (n) WHERE any(x IN n.tags WHERE x = 'a') RETURN n", + "MATCH (n) WHERE all(x IN n.tags WHERE x > 0) RETURN n", + "MATCH (n) WHERE none(x IN n.tags WHERE x < 0) RETURN n", + "MATCH (n) WHERE single(x IN n.tags WHERE x = 1) RETURN n", + "MATCH (n) RETURN n.tags[0] AS first, head(n.tags) AS h", + "MATCH (n) RETURN (n:Admin) AS is_admin", + "MATCH (n) RETURN [x IN n.tags] AS t", + # remaining construct families (rule-coverage: every grammar rule must be + # exercised -- see test_every_grammar_rule_is_exercised_by_the_corpus) + "MATCH p = (a)-[:R]->(b) RETURN p", + "MATCH (n) RETURN count(DISTINCT n.city) AS c", + "MATCH (n) WHERE n.flag = false AND n.gone = null RETURN n", + "GRAPH { MATCH (a)-[r]->(b) }", + "MATCH (n) RETURN {name: n.x, 'k': 1} AS m", + "MATCH (n) RETURN *", + "MATCH ()-[r]->() RETURN count(*)", + "MATCH (a)-->(b) RETURN a, b", + "MATCH (a)<--(b) RETURN a, b", + "MATCH (a)--(b) RETURN a, b", + "MATCH (a)<-->(b) RETURN a, b", + "MATCH (a)-[:R*2]->(b) RETURN b", + # arithmetic / comparison operators + unary + "MATCH (n) WHERE 1 < n.x < 10 RETURN n", + "MATCH (n) WHERE n.x / 2 > n.y % 3 RETURN n", + "MATCH (n) WHERE n.x - 1 > 0 RETURN n", + # unary +/- on a NON-literal operand: a signed literal (-1) is an + # ambiguous literal-vs-uminus form, but -n.x is an unambiguous unary node + "MATCH (n) RETURN -n.x AS v, +n.y AS w", + # composite-root property access + list subscript slices + "MATCH (n) RETURN head(n.tags).name AS v", + "MATCH (n) RETURN n.tags[1..3] AS a, n.tags[1..] AS b, n.tags[..3] AS c, n.tags[..] AS d", +] + +# Queries that are GRAMMATICAL but rejected by the transformer by design +# (known-unsupported features that must still parse to give a good error). +# They contribute to grammar-rule coverage via raw parse only. +GRAMMAR_ONLY_COVERAGE = [ + "MATCH p = allShortestPaths((a)-[:R]->(b)) RETURN p", # raises unsupported at transform +] + +# Invalid inputs: both parsers must reject (rejection parity, not message parity). +REJECTED_CORPUS = [ + "MATCH (n) WHERE RETURN n", + "MATCH (n RETURN n", + "MATCH (n) RETURN", + "MATCH (n)-[r->-(m) RETURN n", + "WHERE n.x = 1 RETURN n", # WHERE without MATCH (rejected post-parse) + "MATCH (a)-[:R*..4]->(b) RETURN b", # open LOWER hop bound is not in the grammar +] + + +def _fresh_lalr_with_log() -> List[logging.LogRecord]: + """Build a fresh (uncached) LALR parser with debug on, capturing lark's + grammar-analysis log records (conflicts are reported at WARNING).""" + records: List[logging.LogRecord] = [] + + class _Capture(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + records.append(record) + + lark_logger = logging.getLogger("lark") + handler = _Capture() + old_level = lark_logger.level + lark_logger.addHandler(handler) + lark_logger.setLevel(logging.DEBUG) # lark.utils defaults it to CRITICAL + try: + lark.Lark( + parser_mod._GRAMMAR, + start="start", + parser="lalr", + debug=True, + maybe_placeholders=False, + propagate_positions=True, + ) + finally: + lark_logger.removeHandler(handler) + lark_logger.setLevel(old_level) + return records + + +def _earley_parser(ambiguity: str = "resolve") -> Any: + return lark.Lark( + parser_mod._GRAMMAR, + start="start", + parser="earley", + ambiguity=ambiguity, + maybe_placeholders=False, + propagate_positions=True, + ) + + +# --- 1. Provable unambiguity: zero conflicts ----------------------------------- + +def test_grammar_has_zero_lalr_conflicts() -> None: + """The grammar is unambiguous LALR(1): the build emits ZERO conflicts — + no shift/reduce, no reduce/reduce. Every input has a single derivation. + + Dependency-free (parses lark's debug log). This is the machine-checkable + "ambiguity is decidable" invariant the whole design is built around: any + grammar edit that introduces an ambiguity produces a conflict and fails + here, naming the terminal. Historically the grammar had 8 shift/reduce + conflicts; each was eliminated at grammar level (WITH..WHERE bundling, + dotted-name split via ``qualified_name``, dropping the redundant + ``label_predicate_expr``, and excluding top-level ``IN`` from list + elements) — see the grammar's inline comments.""" + conflict_re = re.compile( + r"(Shift/Reduce|Reduce/Reduce) conflict for terminal (\w+)" + ) + conflicts = [ + m.group(0) + for record in _fresh_lalr_with_log() + if (m := conflict_re.match(record.getMessage())) + ] + assert conflicts == [], f"grammar is no longer conflict-free: {conflicts}" + + +def test_grammar_builds_under_strict_mode() -> None: + """The grammar builds under Lark ``strict=True`` — a build-time PROOF of + unambiguity that also checks lexer-terminal collisions. Stronger than the + conflict-log check, but needs the optional ``interegular`` dependency, so + it skips where that isn't installed (the log check above is the always-on + guard).""" + pytest.importorskip("interegular") + lark.Lark( + parser_mod._GRAMMAR, + start="start", + parser="lalr", + strict=True, + maybe_placeholders=False, + propagate_positions=True, + ) + + +# --- 2. Semantic ambiguity ----------------------------------------------------- + +def test_semantic_ambiguity_zero() -> None: + """Every corpus query has EXACTLY ONE Earley derivation, and it transforms + to a single AST — semantic ambiguity is zero, no exceptions and no residual + witnesses (an independent AST-level confirmation of the zero-conflict + invariant). Every ambiguity the grammar once had — WITH..WHERE attachment, + dotted-name redundancy, the ``(n:Admin)`` label predicate, and the + ``[x IN xs]`` comprehension overlap — was eliminated at grammar level. + + Note: ``CollapseAmbiguities`` rebuilds trees without ``meta`` positions, so + derivation ASTs have degraded spans/text and CANNOT be compared against the + production AST — only against each other (all equally degraded, so any + structural divergence between derivations still shows).""" + from lark.visitors import CollapseAmbiguities + + explicit = _earley_parser(ambiguity="explicit") + unexpected: List[str] = [] + for query in DIFFERENTIAL_CORPUS: + trees = CollapseAmbiguities().transform(explicit.parse(query)) + asts = { + repr(parser_mod._build_transformer(query).transform(t)) for t in trees + } + if len(trees) != 1 or len(asts) != 1: + unexpected.append(f"{query!r}: {len(trees)} derivations, {len(asts)} ASTs") + assert not unexpected, ( + "Derivation ambiguity introduced (expected exactly 1 derivation each):\n" + + "\n".join(unexpected) + ) + + +# --- Rule coverage: the corpus must exercise EVERY grammar rule ------------------ + +def test_every_grammar_rule_is_exercised_by_the_corpus() -> None: + """Every tree-producing grammar rule must appear in some corpus parse. + + This is what makes syntactic EXTENSIONS safe-by-construction: adding a + grammar rule without a corpus query fails HERE with the rule's name, and + adding the query automatically subjects the new construct to every other + invariant in this file (ambiguity probe, LALR==Earley differential) plus + the full-repo differential. There is no way to extend the grammar and + silently skip the safety net. + + Mechanics: 'tree-producing' = rule aliases plus non-inlined rule names + (lark's ``?rule:`` single-child inlining and ``_``-prefixed helpers never + appear as tree nodes, so they are not coverable and not counted). + Coverage counts raw parses of DIFFERENTIAL_CORPUS + GRAMMAR_ONLY_COVERAGE + (grammatical-but-unsupported constructs) on all three grammar entry + points (whole query, WHERE-lift chain, pattern fragment).""" + lalr = parser_mod._parser_lalr() + + expected = set() + for rule in lalr.rules: # type: ignore[attr-defined] # concrete lark.Lark + if rule.alias: + expected.add(rule.alias) + elif not rule.options.expand1 and not rule.origin.name.startswith("_"): + expected.add(rule.origin.name) + + observed = set() + + def walk(tree: Any) -> None: + if isinstance(tree, lark.Tree): + observed.add(tree.data) + for child in tree.children: + walk(child) + + for query in DIFFERENTIAL_CORPUS + GRAMMAR_ONLY_COVERAGE: + walk(lalr.parse(query)) + # the two sub-grammar entry points (their rules are unused from start=) + walk(parser_mod._where_predicate_chain_parser().parse( + "n.x = 1 AND n.y IS NULL AND n.z IS NOT NULL AND n.s CONTAINS 'a' " + "AND n.t STARTS WITH 'b' AND n.u ENDS WITH 'c' AND n:Admin" + )) + walk(parser_mod._pattern_parser().parse("(a:X {k: 1})-[r:R*1..2]->(b)")) + + uncovered = sorted(expected - observed) + assert not uncovered, ( + "Grammar rules with NO corpus coverage (add a DIFFERENTIAL_CORPUS " + "query exercising each, or GRAMMAR_ONLY_COVERAGE if the construct is " + f"grammatical-but-unsupported): {uncovered}" + ) + + +# --- 3. LALR == Earley differential --------------------------------------------- + +def test_lalr_ast_equals_earley_ast_differential( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Running the production pipeline with Earley instead of LALR yields the + identical AST for every corpus query: the parser swap is AST-neutral.""" + earley = _earley_parser() + + def _with_parser(parser: Any, query: str) -> str: + monkeypatch.setattr(parser_mod, "_parser_lalr", lambda: parser) + parser_mod._parse_cypher_cached.cache_clear() + try: + return repr(parse_cypher(query)) + finally: + monkeypatch.undo() + parser_mod._parse_cypher_cached.cache_clear() + + for query in DIFFERENTIAL_CORPUS: + lalr_ast = _with_parser(parser_mod._parser_lalr(), query) + earley_ast = _with_parser(earley, query) + assert lalr_ast == earley_ast, f"AST divergence on: {query}" + + +def test_lalr_and_earley_reject_the_same_inputs( + monkeypatch: pytest.MonkeyPatch, +) -> None: + earley = _earley_parser() + for query in REJECTED_CORPUS: + for parser in (parser_mod._parser_lalr(), earley): + monkeypatch.setattr(parser_mod, "_parser_lalr", lambda p=parser: p) + parser_mod._parse_cypher_cached.cache_clear() + with pytest.raises(GFQLSyntaxError): + parse_cypher(query) + monkeypatch.undo() + parser_mod._parse_cypher_cached.cache_clear() + + +# The complete set of DELIBERATE accept/reject differences the LALR parser +# introduces, found by full repo-corpus differentials (1800+ queries; +# everything else is accept/reject- and AST-identical). Two kinds: shapes the +# OLD Earley parser accepted by accident, and one shape same-grammar Earley +# accepts but LALR(1) cannot. All are now honest syntax errors. +DELIBERATE_LANGUAGE_FIXES = [ + # Earley's dynamic lexer re-lexed DISTINCT (absent from the NAME reserved + # lookahead) as a NAME and accepted this as returning a *variable named* + # DISTINCT with distinct=False. The LALR contextual lexer keeps it a keyword. + "MATCH (n) RETURN DISTINCT", + # The old flat clause-item grammar accepted WHERE anywhere and attached it + # positionally in Python; these shapes had ill-defined attachment (a double + # WHERE kept BOTH predicates in different AST fields). WHERE now binds to + # its preceding clause in the grammar, so they are syntax errors: + "MATCH (a) WHERE a.x = 1 WHERE a.y = 2 RETURN a", # double WHERE + "MATCH (n) WITH n WHERE n.x = 1 WHERE n.y = 2 RETURN n", # double post-WITH WHERE + "MATCH (n) UNWIND n.tags AS t WHERE t = 'a' RETURN t", # WHERE after UNWIND (not openCypher) + "GRAPH g1 = GRAPH { WHERE a.x = 1 MATCH (a) } USE g1 MATCH (n) RETURN n", # WHERE before MATCH + # A list literal element cannot be a top-level `IN` expression -- that + # syntax is reserved for list comprehensions (`[x IN xs | ...]`), and + # allowing both made the grammar ambiguous. So a "list of IN-booleans" is + # rejected UNIFORMLY at grammar level -- the old parser accepted some of + # these (then failed downstream: `[1 IN a, 2 IN b]` raises a GFQLTypeError). + # Parenthesize for a genuine list of membership booleans: `[(x IN xs), y]`. + "RETURN [x IN xs, y IN ys] AS t", # bare-name first + "RETURN [1 IN a, 2 IN b] AS t", # int first (old parser accepted this) + "RETURN [n.x IN a, y] AS t", # dotted first (old parser accepted this) +] + + +@pytest.mark.parametrize("query", DELIBERATE_LANGUAGE_FIXES) +def test_deliberate_language_fixes_are_rejected(query: str) -> None: + with pytest.raises(GFQLSyntaxError): + parse_cypher(query) diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 07eb7f841d..0ad2b73cd5 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -16952,3 +16952,17 @@ def test_count_table_frame_op_error_and_empty_paths() -> None: assert ctx3._nodes is None and ctx3._edges is None out3 = frame_ops.count_table(ctx3, table="nodes", alias="c") assert out3._nodes.to_dict(orient="records") == [{"c": 0}] + +def test_string_cypher_parenthesized_and_preserves_missing_property_as_null() -> None: + # A parenthesized AND must stay on the row-filter path, not the filter_dict path: + # the row engine treats an absent property as null (Cypher semantics), whereas + # filter_dict requires the column to exist and would raise. + g = _mk_graph( + pd.DataFrame({"id": ["a", "b", "c", "d"], "x": [1, 2, 1, 1]}), # no 'missing' col + pd.DataFrame({"s": ["a", "b", "c"], "d": ["b", "c", "d"]}), + ) + got = g.gfql("MATCH (n) WHERE n.missing IS NULL AND (n.x = 1) RETURN n.id AS id") + assert sorted(got._nodes["id"].tolist()) == ["a", "c", "d"] + # IS NOT NULL on the same absent property yields no rows (absent -> null) + none = g.gfql("MATCH (n) WHERE n.missing IS NOT NULL AND (n.x = 1) RETURN n.id AS id") + assert none._nodes["id"].tolist() == [] if "id" in none._nodes.columns else len(none._nodes) == 0 diff --git a/graphistry/tests/compute/gfql/cypher/test_parser.py b/graphistry/tests/compute/gfql/cypher/test_parser.py index e8f45bf24b..3f78c096a0 100644 --- a/graphistry/tests/compute/gfql/cypher/test_parser.py +++ b/graphistry/tests/compute/gfql/cypher/test_parser.py @@ -1206,3 +1206,142 @@ def test_parse_cypher_invalid_input_not_cached() -> None: parse_cypher("") with pytest.raises(GFQLSyntaxError): parse_cypher(" ") + + +# --- Single LALR(1) path (Earley removed) ------------------------------------- +# After WHERE-grammar unification, parse_cypher uses one LALR(1) parser for every +# supported query -- including OR/XOR/NOT-in-WHERE and post-WITH WHERE, which the +# former dual grammar could only parse under Earley. These smoke-test that the +# whole corpus parses to a valid query on the sole LALR path. + +_PARITY_QUERIES = [ + "MATCH (n) WHERE n.x = 50 RETURN n.__rid__", + "MATCH (n) WHERE n.active = true AND n.x = 5 RETURN n.id", + "MATCH (a)-[r:R]->(b) RETURN a.id, r.weight, b.id", + "MATCH (n {a: 1, b: 2}) RETURN n", + "MATCH (n) WHERE n.x IN [1, 2, 3] RETURN count(n)", + "OPTIONAL MATCH (n)-[r]->(m) RETURN n, m", + "MATCH (n) WHERE NOT (n)-[:R]->() RETURN n", + "MATCH (n) RETURN n.first_name AS fn ORDER BY fn", + # Formerly Earley-only constructs, now on the LALR path: + "MATCH (n) WHERE n:Admin AND n.active = true RETURN n", # label + property + "MATCH (n) WHERE n.a > 3 OR n.b = 1 RETURN n", # OR in WHERE + "MATCH (n) WHERE n.x = 1 XOR n.y = 2 RETURN n", # XOR in WHERE + "MATCH (n)-[rel]->(x) WITH n, x WHERE n.animal = x.animal RETURN n, x", # WITH-WHERE +] + + +@pytest.mark.parametrize("query", _PARITY_QUERIES) +def test_unified_lalr_parses_corpus(query: str) -> None: + assert isinstance(parse_cypher(query), CypherQuery) + + +def test_lalr_is_the_only_parser() -> None: + # The Earley parser and its fallback hook are gone. + from graphistry.compute.gfql.cypher import parser as _p + + assert not hasattr(_p, "_parser") + assert not hasattr(_p, "_lalr_tree_needs_earley") + assert _p._parser_lalr().parse("MATCH (n) WHERE n.x = 1 OR n.y = 2 RETURN n") is not None + + +def test_unified_where_lifts_label_and_property_to_structured() -> None: + # Post grammar-unification: every WHERE parses as the generic expr under the + # sole LALR parser, and generic_where_clause lifts label + property back to + # structured predicates. + label = "MATCH (n) WHERE n:Admin AND n.active = true RETURN n" + parsed = cast(CypherQuery, parse_cypher(label)) + assert parsed.where is not None + assert parsed.where.expr_tree is None # structured via the lift + assert len(parsed.where.predicates) == 2 + + +# --- Flat-AND-chain lift in generic_where_clause ------------------------------- +# Only a FLAT ``where_predicate ("AND" where_predicate)*`` chain lifts to the +# structured (filter_dict) form. Parens / OR / XOR / NOT keep the WHOLE clause on +# expr_tree -> where_rows. + +def test_flat_and_chain_lifts_to_structured() -> None: + w = cast(CypherQuery, parse_cypher( + "MATCH (n) WHERE n.x = 1 AND n.y = 2 AND n.z = 3 RETURN n" + )).where + assert w is not None + assert w.expr_tree is None # structured, not a tree + assert len(w.predicates) == 3 # x, y, z all lifted + + +def test_parenthesized_and_stays_on_expr_tree() -> None: + # Parens make the body NOT a flat predicate chain, so it stays on where_rows, + # which (unlike filter_dict) treats an absent property as null. + for q in ( + "MATCH (n) WHERE n.x = 1 AND (n.y = 2 AND n.z = 3) RETURN n", + "MATCH (n) WHERE (n.x = 1 AND n.y = 2) AND n.z = 3 RETURN n", + "MATCH (n) WHERE n.missing IS NULL AND (n.x = 1) RETURN n", + ): + w = cast(CypherQuery, parse_cypher(q)).where + assert w is not None + assert w.predicates == () + assert w.expr_tree is not None + + +def test_and_with_or_stays_on_expr_tree() -> None: + w = cast(CypherQuery, parse_cypher( + "MATCH (n) WHERE n.x = 1 AND (n.y = 2 OR n.z = 3) RETURN n" + )).where + assert w is not None + assert w.predicates == () # no partial split + assert w.expr_tree is not None # whole clause stays a tree + + +def test_not_in_and_spine_stays_on_expr_tree() -> None: + w = cast(CypherQuery, parse_cypher( + "MATCH (n) WHERE NOT n.x = 1 AND n.y = 2 RETURN n" + )).where + assert w is not None + assert w.predicates == () + assert w.expr_tree is not None + + +# --- OR stays on the row engine (no OR->is_in optimization in this PR) ---------- +# OR-of-equalities collapse to is_in is a deferred routing optimization; the +# parser-switch lift reproduces the old grammar's routing, under which any OR +# clause stays on expr_tree -> where_rows. +def test_or_clause_stays_on_expr_tree() -> None: + for q in ( + "MATCH (n) WHERE n.x = 1 OR n.x = 2 OR n.x = 3 RETURN n", # same column + "MATCH (n) WHERE n.x = 1 OR n.y = 2 RETURN n", # cross column + ): + w = cast(CypherQuery, parse_cypher(q)).where + assert w is not None + assert w.predicates == () + assert w.expr_tree is not None + + +# --- Lifted predicates keep absolute source spans ------------------------------ +# Re-parsing each conjunct in isolation would collapse its span to column 1; the +# lift shifts spans back to absolute query coordinates (via the conjunct's +# atom_span) so downstream errors (e.g. E108) point at the real predicate, not +# the start of the query. Regression guard for the column-1 span bug. +def test_lifted_predicate_spans_are_absolute() -> None: + q = "MATCH (n) WHERE n.x = 1 AND m.y = 2 RETURN n" + w = cast(CypherQuery, parse_cypher(q)).where + assert w is not None and w.expr_tree is None # structured via the lift + by_alias = {cast(PropertyRef, p.left).alias: p for p in w.predicates} + # `n.x` / `m.y` sit at their true offsets in the query, not column 1. + assert q[by_alias["n"].left.span.start_pos:].startswith("n.x") + assert by_alias["n"].left.span.column == q.index("n.x") + 1 + assert q[by_alias["m"].left.span.start_pos:].startswith("m.y") + assert by_alias["m"].left.span.column == q.index("m.y") + 1 + + +def test_lifted_single_predicate_span_skips_where_keyword() -> None: + # Single-predicate WHERE: the synthesized atom span must align with the + # predicate text, not the WHERE keyword (the +len("WHERE ") off-by-six case). + q = "MATCH (a)-[]->(b) WHERE a.x = b.y RETURN a" + w = cast(CypherQuery, parse_cypher(q)).where + assert w is not None and len(w.predicates) == 1 + left = cast(PropertyRef, w.predicates[0].left) + right = w.predicates[0].right + assert left.span.column == q.index("a.x") + 1 + assert isinstance(right, PropertyRef) + assert right.span.column == q.index("b.y") + 1