test(#1201): WHERE boolean-shape conformance matrix#1204
Merged
Conversation
…itter comment Rename test_or_on_non_optional_alias_not_rejecting → test_or_on_optional_alias_is_rejecting to match its assertion (OR form IS null-rejecting, conservatively). Add comment to Shape B pushdown test clarifying that split_top_level_and operates independently of the parser. Drop typing.List import in favour of builtin list[] (from __future__ already present). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lmeyerov
force-pushed
the
feat/issue-1201-where-bool-conformance
branch
from
864c4b8 to
3a325b6
Compare
lmeyerov
added a commit
that referenced
this pull request
Apr 25, 2026
…slice 2 The conformance suite added in #1204 (commit 91c9777) was explicitly designed to track the #1194 grammar-disambiguation and #1200 IR boolean-tree migrations. Slice 2 (#1207) flips three shape contracts from one combined BoundPredicate to per-conjunct BoundPredicates as the binder now flattens top-level AND. Updates: - Shape C ``(A OR B) AND C`` → 2 BoundPredicates (OR-compound + bare C) - Shape D ``NOT A AND B`` → 2 BoundPredicates (NOT-compound + bare B) - Shape F ``n:Admin AND n.active = true`` → 2 BoundPredicates (label + property, top-level AND splits regardless of operand flavor) Assertions now check both count AND content (each conjunct identifiable by substring) so future shape-contract drift is caught precisely. Other shapes in the suite (A, B, E, G) are unaffected: shape A already emitted N BoundPredicates via where_predicates structured route; shape B raises GFQLSyntaxError; shape E (XOR) stays one conjunct because top-level isn't AND; shape G (quoted AND) already produced 2. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lmeyerov
added a commit
that referenced
this pull request
Apr 25, 2026
…vel AND (#1200 slice 2/N) (#1207) * feat(gfql/binder): walk expr_tree, emit one BoundPredicate per top-level AND (#1200 slice 2/N) Slice 2 of issue #1200. Slice 1 (PR #1202) populated ``WhereClause.expr_tree``; this slice teaches the binder to walk it. When ``where.expr_tree`` is present, ``_where_predicates`` flattens top-level AND conjuncts and emits one ``BoundPredicate`` per conjunct instead of one giant ``BoundPredicate`` with the whole expression text. Downstream passes (predicate pushdown, filters) now see pre-split single-conjunct strings, eliminating one source of text re-parsing. When ``where.expr_tree`` is None (mixed-clause routes; pure structured ``where_predicates``; queries with no boolean operators), behavior is unchanged — fallback to the existing ``where.expr.text`` path or the structured ``where.predicates`` path. What this PR adds: - ``_flatten_top_level_ands(expr) -> List[BooleanExpr]``: descends through ``and`` nodes only; or/xor/not/atom stop the recursion and count as one conjunct. Handles left-, right-, and mixed-associative AND chains. - ``_boolean_expr_to_text(expr) -> str``: recursive surface-text reconstruction for a boolean subtree. Atoms emit atom_text; binary ops glue with ``L OP R``; NOT prefixes; branch operands are wrapped in parens for unambiguous round-tripping. Inherits the slice-1 literal-atom fidelity caveat (``str(True) == "True"``). - Updated ``_where_predicates`` branches on ``expr_tree`` availability. 20 tests in ``test_binder_expr_tree.py`` covering helper invariants, ``_where_predicates`` with hand-built ``WhereClause`` fixtures (backward-compat, OR root, AND root, no-expr), and end-to-end through ``parse_cypher`` + binder for OR / NOT / nested AND-with-OR queries. What this PR does NOT change: - ``predicate_pushdown._split_conjuncts`` (slice 3 — relax to fallback only since binder now pre-splits). - ``graphistry/compute/gfql/expr_split.py`` (retired in slice 4 once pushdown migrates). - ``WhereClause.expr`` field (kept for backward compat with mixed-clause routes that still use the text path). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test+docs: address wave-1 review findings on #1207 - Tighten test_e2e_pure_and_of_comparables_unchanged to assert expr_tree is None directly, not just BoundPredicate count. A future parser change that accidentally populates expr_tree on the pure-AND path would now fail loudly rather than silently match the count. - Document the parser-layer mutual-exclusion invariant inline in _where_predicates so a future refactor introducing overlap does not silently double-emit BoundPredicates. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(changelog): add #1200/#1207 binder-walks-expr-tree slice-2 entry Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(gfql/conformance): update WHERE boolean-shape contract for #1200 slice 2 The conformance suite added in #1204 (commit 91c9777) was explicitly designed to track the #1194 grammar-disambiguation and #1200 IR boolean-tree migrations. Slice 2 (#1207) flips three shape contracts from one combined BoundPredicate to per-conjunct BoundPredicates as the binder now flattens top-level AND. Updates: - Shape C ``(A OR B) AND C`` → 2 BoundPredicates (OR-compound + bare C) - Shape D ``NOT A AND B`` → 2 BoundPredicates (NOT-compound + bare B) - Shape F ``n:Admin AND n.active = true`` → 2 BoundPredicates (label + property, top-level AND splits regardless of operand flavor) Assertions now check both count AND content (each conjunct identifiable by substring) so future shape-contract drift is caught precisely. Other shapes in the suite (A, B, E, G) are unaffected: shape A already emitted N BoundPredicates via where_predicates structured route; shape B raises GFQLSyntaxError; shape E (XOR) stays one conjunct because top-level isn't AND; shape G (quoted AND) already produced 2. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
test_where_bool_conformance.pycovering all 7 boolean expression shapes (A–G) × 3 pipeline layers (parser, binder, pushdown).predicatescount +.exprNone/not-None;GFQLSyntaxErrorforAND (... OR ...)(E107)BoundQueryPart.predicatescount per shapesplit_top_level_andsplits +is_null_rejectingper expression formCloses #1201
Test plan
🤖 Generated with Claude Code