Repository navigation
fix(planner): bind each relationship once per match - #82
Merged
Merged
Conversation
Two single-hop relationships of the same type in one MATCH could bind the same edge. Add a filter that their endpoints differ, naming anonymous endpoints so they can be compared. Fixes #59
Deploying lance-csr with
|
| Latest commit: |
67aca1f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f596f25d.lance-csr.pages.dev |
| Branch Preview URL: | https://fix-relationship-uniqueness.lance-csr.pages.dev |
prrao87
marked this pull request as ready for review
October 28, 2026 21:22
prrao87
added a commit
that referenced
this pull request
Oct 28, 2026
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
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.
Why
openCypher binds each relationship at most once within one
MATCH. Here two single-hop relationships of the same type could bind the same edge, soMATCH (a)-[r1:KNOWS]->(b)<-[r2:KNOWS]-(c)also returned rows withc = awherer1andr2are one edge. Named paths already enforced this through the path operator; ordinary patterns did not.Fixes #59.
What changed
MATCH, the planner adds one filter for each pair of single-hop relationships whose types overlap, requiring the two edges to differ. An edge is identified by its endpoint node ids, so this works on the join and CSR paths alike and adds no new columns.MATCHclauses may still bind the same edge, as openCypher allows.MATCH (a)-[:KNOWS]->(b)<-[:KNOWS]-(c) RETURN a, cKnown limits, marked as a
shortcut:in the code: variable-length and untyped relationships are not checked outside named paths, and two parallel edges between the same pair of nodes count as one edge. The second matches the existing undirected backtracking rule; lifting both needs edge row ids carried through the hops.Behaviour change: queries with two same-type hops can return fewer rows. One existing integration test expected the old answer:
COUNT(DISTINCT shared.id)for people known by two different people is now 1 (only Charlie), which matches that test's own comment.Validation
cargo test --workspace: all pass, including a new planner test for the filter and the pairs it skips.cargo clippy --workspace --all-targets -- -D warningsandcargo fmt --all --check: clean.make testinpython/: 1602 passed, including a new test of six query shapes withuse_csroff and on. It ran before two test-only edits (a reformatted test line and the updated Rust expectation); the engine did not change after it.make lint,zensical build --strictandtypos: clean.Benchmarks
Both suites, built with the harness in CONTRIBUTING.md: release wheels of
main(3324055) and of #83's head (9dae1c8, which contains this change), loaded withuv run --with, alternatingmainand branch, two runs each. No benchmark query reuses a relationship variable, so #83's code isn't reached and these numbers measure this change. Every run passed every expected-answer check (30/30 and 14/14).main(geometric mean)mainruns (geometric mean, largest)memorylazymemorylazyOverall there's no slowdown. Two queries are about 10% slower in both branch runs, minimum latencies included, so it's a real cost rather than noise, though it sits inside the 15–20% noise band: IC14 in both modes, and IC8 in
memory(itslazyminimums are unchanged).memory): itspostHasCreator|commentHasCreatoralternation expands into a branch with twocommentHasCreatorhops, which now gets the uniqueness filter.commentHasCreatorhops. Here the check is always true, becauseaandbare different people, but the planner can't know that.EXPLAINshows the same plan on both builds except for one extraFilterExecper affected branch (one in IC8, two in IC14), so the cost is evaluating the filter itself.The custom suite's
memoryset was run twice. In the first, one branch run was noisy (higher means, unchanged minimums; the other branch run matchedmain), which put the geometric mean at 1.113; the table shows the rerun. Inlazymode,mainalone moves by up to 36% between runs on some queries.Custom (30 queries),
memory: every querymain(ms)Custom (30 queries),
lazy: every querymain(ms)Interactive complex (IC1–IC14),
memory: every querymain(ms)Interactive complex (IC1–IC14),
lazy: every querymain(ms)Machine: Apple M5, 24 GB, macOS 26.6.2, on AC power. Benchmark repositories: graph-benchmark-ldbc
b649bc1, graph-benchmark-ldbc-complexf3e63fd.🤖 Generated with Claude Code