Skip to content

feat(rel): node equality + unlabelled-endpoint property join (#598) — passing 18→25#901

Merged
DecisionNerd merged 1 commit into
mainfrom
feature/598-node-equality
Jun 24, 2026
Merged

feat(rel): node equality + unlabelled-endpoint property join (#598) — passing 18→25#901
DecisionNerd merged 1 commit into
mainfrom
feature/598-node-equality

Conversation

@DecisionNerd

@DecisionNerd DecisionNerd commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Advances #598. Two read-path WHERE-join fixes that fully un-skip the equi-join features.

Fixes

  1. Node equalityWHERE a = b / a <> b over node variables compares their node_uuid. Each operand otherwise lowered to a bare var_<v> qualifier (a multi-column node, no scalar form) → error. Handled in lower_binary via node_uuid_of (a node var is one with a NodeScan shape).
  2. Unlabelled bound-endpoint property join — an already-bound but unlabelled node (the x in (n)-[r]->(x)) now joins its properties (_untyped in exploratory, feat(rel): complete node-value materialization — unlabelled RETURN n, render, rel/path values #889), so a later WHERE x.p = … / RETURN x.p resolves. The bound-var NodeScan path previously returned early for ty == None.

Un-skips (mostly node-returning → exercise render)

  • MatchWhere3 (all 3) and WithWhere3 (all 3) — feature-level.
  • MatchWhere4 [1] — per-scenario ([2] = WHERE pattern-predicate, still tagged).
  • Corpus scenarios_passing 18 → 25.

Validation

  • New e2e node_equality_compares_identity, unlabelled_bound_dst_property_resolves; BDD 25/25; tck_coverage green; gf-rel goldens unchanged; cargo clippy --workspace -- -D warnings + fmt clean.

🤖 Generated with Claude Code

Note

Add node equality comparison by identity and unlabelled-endpoint property join

  • Node equality/inequality (=, <>) on node variables now compares node_uuid columns instead of failing on bare multi-column qualifiers, enabling WHERE a = b and WHERE a <> b to work correctly in expr.rs.
  • Bound unlabelled node endpoints (e.g., x in (n)-[:KNOWS]->(x)) now have their properties joined into the stream via join_node_properties, allowing x.prop in WHERE/RETURN to resolve in lowerer.rs.
  • TCK scenario pass count increases from 18 to 25; MatchWhere3, WithWhere3, and Boolean1 move from skipped to passing.

Macroscope summarized eb67b41.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed comparison operations for node variables to correctly compare node identity
    • Fixed property filtering on unlabeled relationship endpoint variables in traversal patterns
  • Tests

    • Expanded test coverage with additional baseline tests for node comparisons and relationship variables
    • Updated test matrix reflecting improved scenario pass rates

Two read-path WHERE-join fixes that fully un-skip the equi-join features:

- Node equality: `WHERE a = b` / `a <> b` over node variables compares
  their `node_uuid`. Each operand otherwise lowered to a bare `var_<v>`
  qualifier (a multi-column node, no scalar form) and errored. Handled in
  `lower_binary` via `node_uuid_of` (a node var has a NodeScan shape).
- Unlabelled bound endpoint property join: an already-bound but
  unlabelled node — the dst of `(n)-[r]->(x)` — now joins its properties
  (`join_node_properties` → `_untyped` in exploratory, #889), so a later
  `WHERE x.p = …` / `RETURN x.p` resolves. The bound-var NodeScan path
  previously returned early for `ty == None`.

Un-skips: MatchWhere3 (all 3) and WithWhere3 (all 3) feature-level;
MatchWhere4 [1] (per-scenario, [2] = pattern predicate still tagged).
Corpus scenarios_passing 18 → 25 (mostly node-returning, exercising the
render arms).

Validated: new e2e `node_equality_compares_identity`,
`unlabelled_bound_dst_property_resolves`; BDD 25/25; tck_coverage green;
gf-rel goldens unchanged; clippy --workspace -D warnings + fmt clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Two correctness fixes in the relational lowering layer: Eq/Neq comparisons between node variables now use node_uuid columns for identity semantics, and unlabelled already-bound NodeScan nodes (with ty: None) now call join_node_properties so downstream WHERE predicates on traversal-bound endpoints resolve. Seven TCK scenarios that were previously skipped for Rust now pass, and the coverage matrix is updated accordingly.

Changes

Node identity comparison and unlabelled node property resolution

Layer / File(s) Summary
Node UUID identity comparison in Eq/Neq
crates/gf-rel/src/expr.rs, crates/gf-api/tests/e2e_baseline.rs
Adds node_uuid_of helper and updates lower_binary to intercept Eq/Neq when both operands are node variables, emitting {alias}.node_uuid column comparisons instead of the generic path. E2E test verifies self-pairs match and non-self pairs do not across a Cartesian Person product.
Unlabelled bound NodeScan property join
crates/gf-rel/src/lowerer.rs, crates/gf-api/tests/e2e_baseline.rs
Replaces the no-op return for already-bound NodeScan { ty: None } with join_node_properties(..., None, input) so traversal-produced unlabelled node bindings expose properties for WHERE filtering. E2E test asserts x.name = 'Bob' resolves and n.name returns 'Alice'.
TCK skip annotations and coverage matrix
tests/tck/features/clauses/match-where/MatchWhere3.feature, tests/tck/features/clauses/match-where/MatchWhere4.feature, tests/tck/features/clauses/with-where/WithWhere3.feature, tests/tck/coverage_matrix.json
Removes @skip-rust from MatchWhere3, MatchWhere4 (feature level), and WithWhere3; adds @skip-rust to one MatchWhere4 scenario. Updates coverage_matrix.json: total scenarios_passing 18→25, MatchWhere3 skip→passing(3), MatchWhere4 skip→partial(1), WithWhere3 skip→passing(3).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • DecisionNerd/graphforge#890: Adds NodeShape/node_shapes tracking to ExprLowerer; this PR reuses that same node_shapes map to special-case Eq/Neq on node variables via {alias}.node_uuid.
  • DecisionNerd/graphforge#893: Also modifies lowerer.rs to ensure unlabelled/bare node variables have their properties joined for downstream WHERE use, directly overlapping with the join_node_properties change in this PR.
  • DecisionNerd/graphforge#891: Updates tests/tck/coverage_matrix.json and @skip-rust/@skip-node tags in TCK feature files, the same artifact types updated in this PR.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: node equality comparison and unlabelled-endpoint property join, with metrics showing the impact (18→25 scenarios passing).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description is mostly complete and follows the template structure with key sections properly filled.

✏️ 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 feature/598-node-equality

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

@DecisionNerd DecisionNerd merged commit 58813d9 into main Jun 24, 2026
41 checks passed
@DecisionNerd DecisionNerd deleted the feature/598-node-equality branch June 24, 2026 00:21
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.

1 participant