feat: EXISTS subquery support — simple, full, nested forms (#385)#396
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- functions.md: mark extract(), filter(), reduce() as COMPLETE (all three are implemented as dedicated grammar rules, not missing) - functions.md: mark all(), any(), none(), single(), exists(), isEmpty() as COMPLETE (were incorrectly listed as NOT_IMPLEMENTED) - functions.md: add missing numeric functions: e(), pi(), exp(), log(), log10(), trig (sin/cos/tan/cot/asin/acos/atan/atan2), degrees(), radians() — total numeric count corrected from 10 to 19 - functions.md: add startNode(), endNode() to Scalar section - functions.md: remove stale line-number references (changed in Phases 5-9); update summary stats and version history to v0.3.8 - functions.md: update Limitations and Priority sections to remove now-fixed items; add Known Gaps table with open issues - opencypher-compatibility-matrix.md: correct all the same function statuses; flip XOR, ^, list slicing, negative indexing to ✅; update CALL procedures to ✅ (v0.3.7 #351); update TCK metrics to 3,801/3,885 (97.8%); replace stale v0.3.1-v0.3.7 roadmap with current v0.3.9/v0.4.0 roadmap; update version history Closes #249 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…385) - Grammar: add `exists_simple` alternative to `exists_expr` for bare pattern form `EXISTS { pattern [WHERE expr] }` alongside existing full form `EXISTS { query }` - AST: extend `SubqueryExpression` with optional `pattern_parts` and `where_predicate` fields for the simple form (query=None when unused) - Parser: add `exists_full` and `exists_simple` transformer methods; update `count_expr` and legacy `exists_expr` to pass all optional fields explicitly - Planner: add `outer_scope_vars` parameter to `plan()` so correlated subquery variables from the outer MATCH are pre-declared, preventing false UndefinedVariable errors; add `_validate_exists_subqueries` / `_validate_no_write_in_exists` to raise SyntaxError(InvalidClauseComposition) when write clauses (SET, REMOVE, CREATE, DELETE, MERGE) appear in EXISTS bodies - Evaluator: route simple form to `_evaluate_simple_exists` which walks pattern parts against the graph, binds new variables per match, and evaluates the WHERE predicate; full form passes outer scope bindings when planning the subquery Fixes all 10 TCK existential-subquery scenarios (ExistentialSubquery1-3), including nested EXISTS and the must-fail update-in-EXISTS validation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (3)
WalkthroughThis PR introduces EXISTS subquery support by adding dual syntactic forms: a full subquery form ( Changes
Sequence Diagram(s)sequenceDiagram
participant Parser as Parser
participant Planner as Planner
participant Executor as Executor
participant Evaluator as Evaluator
rect rgba(100, 150, 200, 0.5)
note over Parser,Evaluator: Simple EXISTS { pattern_parts WHERE... }
end
Parser->>Parser: Parse EXISTS expression
Parser->>Parser: Transform to SubqueryExpression<br/>(pattern_parts, where_predicate)
Parser->>Planner: plan(ast, outer_scope_vars)
Planner->>Planner: Validate no write clauses in EXISTS
Planner-->>Parser: Execution plan
Parser->>Executor: Execute with outer bindings
Executor->>Evaluator: _evaluate_simple_exists()
Evaluator->>Evaluator: Traverse pattern_parts<br/>from outer variable
Evaluator->>Evaluator: Evaluate WHERE_predicate per match
Evaluator-->>Executor: CypherBool(True/False)
Executor-->>Parser: Result
rect rgba(200, 150, 100, 0.5)
note over Parser,Evaluator: Full EXISTS { query }
end
Parser->>Parser: Parse EXISTS expression
Parser->>Parser: Transform to SubqueryExpression<br/>(query, where_predicate=None)
Parser->>Planner: plan(ast, outer_scope_vars)
Planner->>Planner: Validate no write clauses in nested query
Planner-->>Parser: Execution plan
Parser->>Executor: Execute with outer bindings
Executor->>Evaluator: Evaluate nested query<br/>(planner-based)
Evaluator-->>Executor: CypherBool(True/False)
Executor-->>Parser: Result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 20 minutes and 16 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/graphforge/executor/evaluator.py`:
- Around line 1605-1727: The current EXISTS traversal (seed_bindings,
_extend_hop, hop_count, parts) only continues from named variables and ignores
anonymous nodes and var-length (min_hops/max_hops) patterns; fix by carrying the
current node explicitly in the hop state (e.g., include a special key like
"_cur_node" in each binding or change _extend_hop signature to accept a current
NodeRef), so _extend_hop uses that node when src variable is absent, and when
src variable exists prefer bindings[src_var] but still update "_cur_node" as you
advance; also implement var-length expansion by iterating/recursing between
rel_pattern.min_hops and rel_pattern.max_hops (mirroring the logic in
_evaluate_pattern_predicate) to repeatedly follow edges and collect intermediate
bindings, applying the same label/type/bound-variable equality checks
(dst_pattern.labels, rel_pattern.types, ctx.bindings enforcement) at each step,
and ensure seed_bindings includes initial "_cur_node" for anonymous source
scans.
- Around line 1609-1614: When seeding traversal from an already-bound variable
(the src_var branch using ctx.get / ctx.bindings and appending to
seed_bindings), add a label validation against src_pattern.labels: after
retrieving src_node (and after the CypherNull check) verify that if
src_pattern.labels is non-empty the src_node contains all those labels (use
whatever NodeRef API provides for labels); only append {src_var: src_node} to
seed_bindings when the label check passes, otherwise skip seeding. This prevents
bound variables that don't match src_pattern.labels from incorrectly satisfying
EXISTS patterns.
In `@src/graphforge/planner/planner.py`:
- Around line 2773-2834: The EXISTS-walker misses some paths: in
_validate_exists_subqueries pass clause.where.predicate (not clause.where), and
also iterate WithClause.items like ReturnClause.items and call
_validate_no_write_in_exists(item.expression); in _validate_no_write_in_exists,
when handling SubqueryExpression also recurse into its where_predicate (or
.where.predicate if the AST uses a Where container) so nested WHERE/EXISTS
chains (e.g. WithClause items and SubqueryExpression.where_predicate) are
visited and write clauses inside EXISTS are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4cd06592-0097-4568-8d4e-3c5bc31ddd5d
⛔ Files ignored due to path filters (4)
.github/workflows/test.ymlis excluded by!**/.github/**README.mdis excluded by!**/*.mddocs/reference/implementation-status/functions.mdis excluded by!**/*.md,!**/docs/**docs/reference/opencypher-compatibility-matrix.mdis excluded by!**/*.md,!**/docs/**
📒 Files selected for processing (5)
src/graphforge/ast/expression.pysrc/graphforge/executor/evaluator.pysrc/graphforge/parser/cypher.larksrc/graphforge/parser/parser.pysrc/graphforge/planner/planner.py
❌ 54 Tests Failed:
View the top 3 failed test(s) by shortest run time
View the full list of 49 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
evaluator.py — _evaluate_simple_exists: - Carry _cur sentinel key in each binding dict so anonymous intermediate node patterns (no variable name) advance traversal correctly; previously an anonymous src caused a silent skip via the `elif src_v: continue` branch - Add src label validation for the bound-variable seed path (mirrors the label check already present for unbound scans) - Implement var-length hop expansion inside _extend_hop: when rel_pattern has min_hops/max_hops set, delegate to _var_length_reachable and apply dst label/equality checks to each reachable node planner.py — _validate_exists_subqueries / _validate_no_write_in_exists: - Pass clause.where.predicate (not clause.where) to avoid a WhereClause object reaching _validate_no_write_in_exists with no matching isinstance - Iterate WithClause.items and recurse into each item.expression so EXISTS subqueries embedded in WITH projections are validated - Recurse into SubqueryExpression.where_predicate in _validate_no_write_in_exists so write clauses nested inside a simple-form WHERE are caught Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…in CI - Update comparison tables in README and docs from "openCypher subset" to "Full OpenCypher*" with footnote: 3,865/3,885 TCK passing, 20 permanent xfails due to CPython integer/float precision limitations - Add @pytest.mark.snap to TestSmallDatasetLoading and TestComplexQueries so their live network downloads are skipped in CI - Exclude snap-marked tests in CI integration and coverage shard runs via -m "not snap" (snap marker was already documented as CI-skipped in pyproject.toml but not enforced in the workflow) 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
Implements full
EXISTSsubquery support across all four layers.WHERE EXISTS { (n)-->() }andWHERE EXISTS { (n)-->(m) WHERE n.prop = m.prop }— bare pattern with optional WHERE, evaluated by_evaluate_simple_existsWHERE EXISTS { MATCH (n)-->(m) RETURN true }and with aggregation (WITH count(*) ...) — correlated subquery planning with outer-scope variable injectionWHERE EXISTS { MATCH ... SET ... }raisesSyntaxError: InvalidClauseCompositionat compile timeFixes 10 TCK scenarios across ExistentialSubquery1, ExistentialSubquery2, ExistentialSubquery3.
Changes
cypher.lark): Addexists_simplealternative for bare pattern formast/expression.py): ExtendSubqueryExpressionwith optionalpattern_partsandwhere_predicatefieldsparser.py): Addexists_full/exists_simpletransformer methods; explicitNonefor unused optional fieldsplanner.py):outer_scope_varsparameter toplan()for correlated subquery scope;_validate_exists_subqueriespre-pass to reject write clausesevaluator.py):_evaluate_simple_existsfor pattern matching; outer scope forwarded for full-form planningTest plan
ruff checkandruff format --checkpassmypy --strict-optionalpassesCloses #385
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements