Skip to content

feat(cypher): WHERE on OPTIONAL MATCH + multiple OPTIONAL MATCH (#1024, #1025) - #1030

Merged
lmeyerov merged 13 commits into
masterfrom
fix/issue-1025-multiple-optional-match
Apr 4, 2026
Merged

feat(cypher): WHERE on OPTIONAL MATCH + multiple OPTIONAL MATCH (#1024, #1025)#1030
lmeyerov merged 13 commits into
masterfrom
fix/issue-1025-multiple-optional-match

Conversation

@lmeyerov

@lmeyerov lmeyerov commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Architecture

  • ast.py: New MatchClause.where: Optional[WhereClause] field
  • parser.py: Each WHERE is scoped to its preceding MATCH via replace(match_clauses[-1], where=item)
  • lowering.py: _apply_where_to_ops() applies label/property WHERE per-clause; N-arm ConnectedOptionalMatchPlan
  • gfql_unified.py: Chained left-outer-join loop over N arms

Test plan

  • 8 new WHERE tests: label on base, label on optional, both, property comparison, null-safe, eliminates-all, filters-all, combined with CASE+ORDER BY
  • 4 new multi-optional tests: two optionals, one misses, single-node base, chained with CASE
  • Full test_lowering.py: 558 passed, 49 skipped
  • typecheck: success (204 files)
  • lint: all checks passed
  • DGX GPU validation (pending)
  • Test amplification in progress

🤖 Generated with Claude Code

@lmeyerov
lmeyerov force-pushed the fix/issue-1025-multiple-optional-match branch from 2b3a793 to 40a9a18 Compare April 3, 2026 08:30
@lmeyerov

lmeyerov commented Apr 3, 2026

Copy link
Copy Markdown
Contributor Author

Branch-validation benchmark update from pyg-bench on pushed branch origin/fix/issue-1025-multiple-optional-match e5b04cba38d36b36cf42d2307f4667b877f74991.

Pinned artifact:

  • results/runs/dgx-spark-snb-interactive-is7-cypher-origin-fix-issue-1025-e5b04cba3-no-warmup-r1/

Command:

  • uv run python scripts/run_dgx_spark_suite.py --suite snb-interactive --config configs/suites/snb-interactive-is7-cypher-conformance-sf1-no-warmup.yaml --output-dir results/runs/dgx-spark-snb-interactive-is7-cypher-origin-fix-issue-1025-e5b04cba3-no-warmup-r1 --graphistry-repo-path /tmp/pygraphistry-origin-fix-issue-1025-e5b04cba3 --host-uv

Current result:

  • still returns 137
  • direct-Cypher IS7 probe does start; it no longer looks like the old compiler rejection lane
  • dgx.execute_remote_command duration_ms = 293200.718
  • snb.load_dataset_fixture duration_ms = 7794.353

So the benchmark read is:

  • this branch does not regress the post-#996 progress
  • but it also does not make official interactive-short-7 / message-replies complete on sf1
  • the remaining problem still looks like runtime/perf/memory kill territory rather than the old OPTIONAL MATCH lowering rejection

I would not call IS7 benchmark-closed from this branch alone.

@lmeyerov

lmeyerov commented Apr 3, 2026

Copy link
Copy Markdown
Contributor Author

Additional DGX benchmark detail from the same staged #1030 branch run.

I reran the already-staged remote directory directly under /usr/bin/time -v using the existing .venv/bin/python to separate launcher overhead from runtime behavior.

Result:

  • terminated by signal 9
  • elapsed wall time 5:03.77
  • max RSS 120547356 kbytes
  • shell-reported EXIT_CODE=137

So the current benchmark read is stronger than just “still fails”:

  • this branch gets past the old lowering/compiler rejection
  • but the official IS7 lane is now clearly blowing up in execution/resource territory on sf1
  • the failure envelope is worse than the current-master baseline, not better

That makes it hard to treat this branch as materially improving benchmark readiness for IS7 yet.

lmeyerov and others added 11 commits April 3, 2026 16:34
@lmeyerov @claude
feat(cypher): WHERE on OPTIONAL MATCH + multiple OPTIONAL MATCH (#1024,
398f6c4 
#1025)

- Add MatchClause.where field to associate WHERE with its preceding MATCH
- Fix parser to scope each WHERE to its MATCH clause
- Add _apply_where_to_ops() to apply per-match WHERE predicates
- Generalize _is_connected_optional_match_query() to N optionals
- Generalize ConnectedOptionalMatchPlan with N arms for chained left-outer-joins
- 12 new tests covering WHERE + multi-optional boundaries

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lmeyerov @claude
test(cypher): amplify OPTIONAL MATCH boundary coverage (#1024, #1025)
cd06db1 
Audit-driven tests: chained optionals sharing non-base aliases,
transitive null-fill, WHERE on 3+ optionals, single-node base with
3 optionals, partial null-fill pattern, property WHERE filtering,
multi-optional ORDER BY + LIMIT.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lmeyerov @claude
ci: retrigger with tck-gfql branch alignment
ce2fb4e 
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lmeyerov @claude
refactor+test(cypher): DRY _apply_where_to_ops, amplify boundary tests (
090803f 
#1024, #1025)

- Remove unused `ops` parameter from `_apply_where_to_ops()`
- Add single-node base + WHERE + 3 optionals test
- Add cross-alias property comparison WHERE test

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lmeyerov @claude
ci: retrigger with tck-gfql match-where6-4 promotion
a322d51 
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lmeyerov @claude
fix(cypher): reject cross-clause WHERE alias references with proper v…
283ea44 
…alidation error (#1024, #1025)

When a WHERE predicate references an alias from a different MATCH clause
(e.g., WHERE x.val < z.val where x is in the base MATCH and z is in a
different OPTIONAL MATCH), raise GFQLValidationError instead of letting
a raw ValueError escape from the chain executor.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lmeyerov @claude
fix(cypher): reject unsupported WHERE expr in connected optional match (
3ad2365 
#1024, #1025)

WHERE expressions that cannot be lowered to node/edge filters (e.g.,
r <> r2 alias comparisons) now raise GFQLValidationError instead of
being silently dropped, which caused wrong-answer results for TCK
match7-11.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lmeyerov @claude
fix(cypher): reject variable-length relationships in optional match a…
dc6ae6c 
…rms (#1024, #1025)

Variable-length relationships in OPTIONAL MATCH clauses (e.g., [:BAR*])
are not yet supported in the connected optional match path. The detection
function now rejects these to prevent wrong-answer results (TCK match7-15).

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lmeyerov @claude
fix(cypher): reject comma-separated base MATCH in connected optional …
18c7b11 
…path (#1024, #1025)

Comma-separated MATCH patterns like (a:A), (b:B) require cartesian
product semantics that the binding_ops mechanism cannot handle. Reject
them in _is_connected_optional_match_query() so they fall through to
the existing rejection path (TCK match7-26).

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lmeyerov @claude
ci: retrigger after rebase onto master cd7c7bc
94122ed 
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lmeyerov @claude
ci: retrigger with tck-gfql with4-2 promotion
0bcb631 
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lmeyerov
lmeyerov force-pushed the fix/issue-1025-multiple-optional-match branch from e5b04cb to 0bcb631 Compare April 3, 2026 23:37
@lmeyerov
lmeyerov merged commit 11038fd into master Apr 4, 2026
68 checks passed
@lmeyerov
lmeyerov deleted the fix/issue-1025-multiple-optional-match branch April 4, 2026 00:38
This was referenced Apr 4, 2026
Closed
Closed
Closed
Closed
Open
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@lmeyerov