Skip to content

fix: EXISTS {} inner anonymous node with properties no longer raises ValidationError#496

Merged
DecisionNerd merged 1 commit into
mainfrom
fix/474-exists-subquery-outer-variable
May 5, 2026
Merged

fix: EXISTS {} inner anonymous node with properties no longer raises ValidationError#496
DecisionNerd merged 1 commit into
mainfrom
fix/474-exists-subquery-outer-variable

Conversation

@DecisionNerd
Copy link
Copy Markdown
Owner

@DecisionNerd DecisionNerd commented May 5, 2026

Closes #474

Summary

  • EXISTS { MATCH (:Label {prop: outer.var}) } raised ValidationError: PropertyAccess requires either 'variable' or 'base' when the inner node was anonymous (no explicit variable name)
  • Root cause: _plan_match passed node_pattern.variable (which is None for anonymous nodes) to _properties_to_predicate, but var_name on the line above already resolves the correct name (real or generated anon)
  • One-line fix: use var_name instead of node_pattern.variable at the call site

Test plan

  • test_anonymous_inner_node_with_outer_property — exact failure case from issue
  • test_outer_variable_in_inner_where — outer var referenced in inner WHERE
  • test_outer_variable_in_in_expression — outer var in IN expression
  • test_ontology_conformance_query — full repro from issue, domain-violation rows correct
  • All existing EXISTS TCK scenarios still pass
  • make pre-push green (87.08% total coverage)

🤖 Generated with Claude Code


View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

Summary by CodeRabbit

…ValidationError (#474)

_plan_match passed node_pattern.variable (which can be None for anonymous
nodes) to _properties_to_predicate, causing a Pydantic ValidationError.
Fix: use var_name instead — already set to the generated anon name at
line 791 for nameless nodes.

Adds four integration tests covering all acceptance criteria: anonymous
inner node with outer-var property, outer var in inner WHERE, outer var
in IN expression, and the full ontology conformance query from the issue.

Closes #474
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 644da33b-11c3-4d93-a01a-33b8cfd0f5a4

📥 Commits

Reviewing files that changed from the base of the PR and between 79726d9 and d148eb4.

📒 Files selected for processing (2)
  • src/graphforge/planner/planner.py
  • tests/integration/test_exists_is_empty.py

Walkthrough

The planner's single-node MATCH pattern builder now uses the resolved internal variable name when constructing inline property predicates, fixing a bug where anonymous pattern nodes (lacking explicit variables) would fail with validation errors during EXISTS subquery planning.

Changes

Planner Fix for EXISTS Subqueries with Outer Variables

Layer / File(s) Summary
Core Logic Fix
src/graphforge/planner/planner.py
_plan_match uses the resolved var_name (including auto-generated names for anonymous nodes) instead of node_pattern.variable (which can be None) when building inline property predicates for single-node patterns.
Integration Tests
tests/integration/test_exists_is_empty.py
New TestExistsSubqueryOuterVariable suite with four tests validates EXISTS subqueries correctly resolve outer-scope variables in inner node properties, WHERE clauses, and IN expressions; includes an ontology conformance query test.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • DecisionNerd/graphforge#376: Related work on pattern predicates and variable binding in planner logic for OPTIONAL MATCH and pattern-variable handling.
  • DecisionNerd/graphforge#396: Related planner updates for correlated EXISTS subqueries and outer-scope variable handling in planner.py.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately describes the main fix: resolving a ValidationError when EXISTS subqueries use anonymous inner nodes with properties.
Description check ✅ Passed The PR description includes a clear summary, root cause analysis, test plan with checkmarks, and references the issue (#474), meeting all key template requirements despite not using the full checklist format.
Linked Issues check ✅ Passed The PR fully addresses all acceptance criteria from #474: fixes the ValidationError for anonymous inner nodes with outer variables, adds comprehensive tests (anonymous node with outer property, outer var in WHERE/IN), confirms existing EXISTS TCK scenarios pass, and verifies the ontology conformance query works correctly.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the EXISTS subquery bug: the one-line fix in _plan_match and the four integration tests validating the fix against acceptance criteria from #474.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/474-exists-subquery-outer-variable

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.02%. Comparing base (79726d9) to head (d148eb4).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #496      +/-   ##
==========================================
- Coverage   88.02%   88.02%   -0.01%     
==========================================
  Files          40       40              
  Lines       14471    14470       -1     
  Branches     3434     3434              
==========================================
- Hits        12738    12737       -1     
  Misses       1142     1142              
  Partials      591      591              
Flag Coverage Δ
full-coverage 88.02% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
parser 95.67% <ø> (ø)
planner 82.88% <100.00%> (-0.01%) ⬇️
executor 83.63% <ø> (ø)
storage 91.25% <ø> (ø)
ast 98.20% <ø> (ø)
types 94.75% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79726d9...d148eb4. Read the comment docs.

@DecisionNerd DecisionNerd merged commit 6be48ac into main May 5, 2026
34 checks passed
@DecisionNerd DecisionNerd deleted the fix/474-exists-subquery-outer-variable branch May 5, 2026 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: EXISTS {} subquery fails in planner when outer variable used in inner MATCH property

1 participant