Repository navigation
fix(optimizer): drop the rows where MATCH re-binds a node that OPTIONAL MATCH left NULL - #1362
Merged
adsharma merged 5 commits intoOct 23, 2026
Merged
Conversation
…NULL After OPTIONAL MATCH (a)-[:K]->(b), a later MATCH (b) is planned as an inner hash join of the previous plan with a bare scan of b on b._ID. RemoveUnnecessaryJoinOptimizer removed that join because the scan has no properties, but the join is what drops the rows where b is NULL, so persons without a neighbour came back with a NULL b. The join now stays when a key on the kept side can be NULL: it comes from the build side of a LEFT join or an optional cross product, or from under an optional accumulate. MERGE's marked existence check is skipped, since MERGE creates what it does not find. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After two chained OPTIONAL MATCHes, MATCH (c) must drop the rows where either step found nothing, and MATCH (b) the rows where the first did. Re-matching b needs the NULL check to look below the second OPTIONAL MATCH, whose join has b on both sides; a check that stops at that join keeps the rows where b is NULL. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
adsharma
reviewed
Oct 23, 2026
| // Joining with a full node scan drops the rows whose key is NULL, so the join is only | ||
| // redundant when the kept side's keys are never NULL. | ||
| static bool keptSideKeysCanBeNull(const LogicalHashJoin& hashJoin, uint32_t keptChildIdx) { | ||
| for (auto& [probeKey, buildKey] : hashJoin.getJoinConditions()) { |
Contributor
There was a problem hiding this comment.
Nit (non-blocking): hashJoin.getJoinConditions() returns by value, so this copies the condition vector on every candidate join. Consider hoisting to const auto joinConditions = hashJoin.getJoinConditions(); outside the loop.
Contributor
|
Two non-blocking follow-ups from review (inline nit posted separately):
|
The issue names MATCH (b:P) as affected; the new cases only used a bare MATCH (b). MATCH (b:organisation) takes the same bare-scan join and returns 8 rows on main instead of 3. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
getJoinConditions() returned the vector by value, so each caller copied it, including RemoveUnnecessaryJoinOptimizer once per candidate join. Every caller only reads the conditions or copies them explicitly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Added On the nit: the range-for already calls |
…rough A comment on the default case of canBeNull's operator switch, and one on why MARK and COUNT joins never expose a build-side key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zahariash
marked this pull request as ready for review
October 23, 2026 20:33
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.
This work was produced with the help of language models.
Fixes #1298.
With the issue's data,
MATCH (a:P) WHERE a.id < 3 OPTIONAL MATCH (a)-[:K]->(b:P) WITH a, b MATCH (b) RETURN a.id, b.idreturned[0, None], [1, 5], [2, None]instead of[1, 5]. The secondMATCHkept the rows wherebwas NULL. So didMATCH (b:P), an alias, aSETbetween the clauses, chainedOPTIONAL MATCHes, and anOPTIONAL MATCHthat is uncorrelated or comes first.Fix. The planner joins the earlier rows with a scan of
b, and that join is what drops a NULLb.RemoveUnnecessaryJoinOptimizerremoved it because the scan reads no properties. The join now stays when anOPTIONAL MATCHbelow can leave its key NULL. MERGE's existence check doesn't count, since MERGE creates what it doesn't find. Re-matching a variable that can't be NULL still drops the join. The kept join readsb's whole node table, as withenable_plan_optimizer=false.Tests.
optional_match.testcovers each form above, and all of them fail onmain(8 rows instead of 3 for the issue's shape).RemoveUnnecessaryJoinNullableKeyTestchecks that the join stays where the key can be NULL and goes where it can't, including after MERGE. Removing any one of thehasMarkchecks or the probe-side checks fails it.Full ctest: 3164/3164 passed. LDBC SNB SF1: all 44 plans are unchanged, and planning costs 0.085% more instructions, within run-to-run noise.