From 8bf23c1d07379ed85c367296c19d7de7e8ade0c7 Mon Sep 17 00:00:00 2001 From: Darek Date: Thu, 22 Oct 2026 18:40:23 +0200 Subject: [PATCH 1/5] fix(optimizer): keep the join that drops rows an OPTIONAL MATCH left 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 --- .../remove_unnecessary_join_optimizer.h | 3 +- .../remove_unnecessary_join_optimizer.cpp | 82 ++++++++++++++++++- test/optimizer/optimizer_test.cpp | 36 ++++++++ .../optional_match/optional_match.test | 41 ++++++++++ 4 files changed, 159 insertions(+), 3 deletions(-) diff --git a/src/include/optimizer/remove_unnecessary_join_optimizer.h b/src/include/optimizer/remove_unnecessary_join_optimizer.h index 25509ef882..68755c8507 100644 --- a/src/include/optimizer/remove_unnecessary_join_optimizer.h +++ b/src/include/optimizer/remove_unnecessary_join_optimizer.h @@ -15,7 +15,8 @@ namespace optimizer { * E(e) S(b) * | * S(a) - * This optimizer prunes such redundant joins. + * This optimizer prunes such redundant joins. A join whose key an OPTIONAL MATCH can leave + * NULL is not redundant: it drops the rows with a NULL key, so it stays. */ class RemoveUnnecessaryJoinOptimizer : public LogicalOperatorVisitor { public: diff --git a/src/optimizer/remove_unnecessary_join_optimizer.cpp b/src/optimizer/remove_unnecessary_join_optimizer.cpp index 793f4f953e..e5c24c275e 100644 --- a/src/optimizer/remove_unnecessary_join_optimizer.cpp +++ b/src/optimizer/remove_unnecessary_join_optimizer.cpp @@ -1,8 +1,11 @@ #include "optimizer/remove_unnecessary_join_optimizer.h" +#include "planner/operator/logical_accumulate.h" +#include "planner/operator/logical_cross_product.h" #include "planner/operator/logical_hash_join.h" #include "planner/operator/scan/logical_scan_node_table.h" +using namespace lbug::binder; using namespace lbug::common; using namespace lbug::planner; @@ -24,6 +27,81 @@ std::shared_ptr RemoveUnnecessaryJoinOptimizer::visitOperator( return result; } +// No default labels in these two: -Wswitch flags a new type, which counts as NULL-producing +// until classified. +static bool nullsBuildOnlyColumns(JoinType joinType) { + switch (joinType) { + case JoinType::INNER: + case JoinType::MARK: + case JoinType::COUNT: + return false; + case JoinType::LEFT: + return true; + } + return true; +} + +static bool isOptional(AccumulateType accumulateType) { + switch (accumulateType) { + case AccumulateType::REGULAR: + return false; + case AccumulateType::OPTIONAL_: + return true; + } + return true; +} + +// An OPTIONAL MATCH that finds nothing binds its variables to NULL. They come from the build +// side of a LEFT join or an optional cross product, or from under an optional accumulate. +// MERGE's existence check also plans an OPTIONAL MATCH, marked; MERGE creates what it does +// not find, so those leave no NULLs. +static bool canBeNull(const LogicalOperator& op, const Expression& key) { + switch (op.getOperatorType()) { + case LogicalOperatorType::HASH_JOIN: { + const auto& join = op.constCast(); + if (nullsBuildOnlyColumns(join.getJoinType()) && !join.hasMark() && + !op.getChild(0)->getSchema()->isExpressionInScope(key) && + op.getChild(1)->getSchema()->isExpressionInScope(key)) { + return true; + } + } break; + case LogicalOperatorType::CROSS_PRODUCT: { + const auto& crossProduct = op.constCast(); + if (isOptional(crossProduct.getAccumulateType()) && !crossProduct.hasMark() && + op.getChild(1)->getSchema()->isExpressionInScope(key)) { + return true; + } + } break; + case LogicalOperatorType::ACCUMULATE: { + const auto& accumulate = op.constCast(); + if (isOptional(accumulate.getAccumulateType()) && !accumulate.hasMark() && + op.getChild(0)->getSchema()->isExpressionInScope(key)) { + return true; + } + } break; + default: + break; + } + for (auto i = 0u; i < op.getNumChildren(); ++i) { + if (canBeNull(*op.getChild(i), key)) { + return true; + } + } + return false; +} + +// 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()) { + const auto& key = keptChildIdx == 0 ? *probeKey : *buildKey; + if (canBeNull(*hashJoin.getChild(keptChildIdx), key)) { + return true; + } + } + return false; +} + std::shared_ptr RemoveUnnecessaryJoinOptimizer::visitHashJoinReplace( std::shared_ptr op) { auto hashJoin = (LogicalHashJoin*)op.get(); @@ -45,14 +123,14 @@ std::shared_ptr RemoveUnnecessaryJoinOptimizer::visitHashJoinRe // keep this condition tight. if (op->getChild(1)->getOperatorType() == LogicalOperatorType::SCAN_NODE_TABLE) { const auto scanNode = dynamic_cast_checked(op->getChild(1).get()); - if (scanNode->getProperties().empty()) { + if (scanNode->getProperties().empty() && !keptSideKeysCanBeNull(*hashJoin, 0)) { // Build side is trivial. Prune build side. return op->getChild(0); } } if (op->getChild(0)->getOperatorType() == LogicalOperatorType::SCAN_NODE_TABLE) { const auto scanNode = dynamic_cast_checked(op->getChild(0).get()); - if (scanNode->getProperties().empty()) { + if (scanNode->getProperties().empty() && !keptSideKeysCanBeNull(*hashJoin, 1)) { // Probe side is trivial. Prune probe side. return op->getChild(1); } diff --git a/test/optimizer/optimizer_test.cpp b/test/optimizer/optimizer_test.cpp index ec92fbba76..1f262376dc 100644 --- a/test/optimizer/optimizer_test.cpp +++ b/test/optimizer/optimizer_test.cpp @@ -351,6 +351,42 @@ TEST_F(OptimizerTest, RemoveUnnecessaryJoinTest) { ASSERT_STREQ(getEncodedPlan(q1).c_str(), "E(b)S(a)"); } +TEST_F(OptimizerTest, RemoveUnnecessaryJoinNullableKeyTest) { + // b can be NULL after each OPTIONAL MATCH form, so the join with S(b) stays. + auto q1 = "MATCH (a:person) OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) " + "WITH a, b MATCH (b) RETURN a.ID;"; + ASSERT_STREQ(getEncodedPlan(q1).c_str(), + "HJ(b._ID){HJ(a._ID){S(a)}{HJ(a._ID){}{E(b)S(a)}}}{S(b)}"); + auto q2 = "MATCH (a:person) WHERE a.ID < 3 OPTIONAL MATCH (b:organisation) " + "WHERE b.score > 1000 WITH a, b MATCH (b) RETURN a.ID;"; + ASSERT_STREQ(getEncodedPlan(q2).c_str(), "HJ(b._ID){CP(){Filter()S(a)}{Filter()S(b)}}{S(b)}"); + auto q3 = "OPTIONAL MATCH (b:organisation) WHERE b.score > 1000 WITH b MATCH (b) RETURN b.ID;"; + ASSERT_STREQ(getEncodedPlan(q3).c_str(), "HJ(b._ID){Filter()S(b)}{S(b)}"); + // a is the LEFT join's probe key and MERGE creates b, so neither is NULL: the join goes. + auto q4 = "MATCH (a:person) OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) " + "WITH a, b MATCH (a) RETURN b.ID;"; + ASSERT_STREQ(getEncodedPlan(q4).c_str(), + "HJ(a._ID){S(a)}{HJ(a._ID){}{HJ(b._ID){S(b)}{E(b)S(a)}}}"); + auto q5 = "MATCH (a:person) WHERE a.ID = 0 " + "MERGE (a)-[:studyAt]->(b:organisation {ID: 100}) WITH a, b MATCH (b) RETURN a.ID;"; + ASSERT_STREQ(getEncodedPlan(q5).c_str(), + "Filter()HJ(a._ID){IndexScan(a)}{HJ(b._ID){IndexScan(b)}{E(b)S(a)}}"); + auto q6 = "MATCH (a:person) WHERE a.ID = 0 MERGE (b:organisation {ID: 100}) " + "WITH a, b MATCH (b) RETURN a.ID;"; + ASSERT_STREQ(getEncodedPlan(q6).c_str(), "Filter()CP(){IndexScan(a)}{IndexScan(b)}"); + auto q7 = "MERGE (b:organisation {ID: 100}) WITH b MATCH (b) RETURN b.ID;"; + ASSERT_STREQ(getEncodedPlan(q7).c_str(), "Filter()IndexScan(b)"); + // After an update the scan is the probe side. + auto q8 = "MATCH (a:person) OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) " + "SET a.age = a.age WITH a, b MATCH (b) RETURN a.ID;"; + ASSERT_STREQ(getEncodedPlan(q8).c_str(), + "HJ(b._ID){S(b)}{HJ(a._ID){S(a)}{HJ(a._ID){}{E(b)S(a)}}}"); + auto q9 = "MATCH (a:person) OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) " + "SET a.age = a.age WITH a, b MATCH (a) RETURN b.ID;"; + ASSERT_STREQ(getEncodedPlan(q9).c_str(), + "HJ(a._ID){S(a)}{HJ(a._ID){}{HJ(b._ID){S(b)}{E(b)S(a)}}}"); +} + TEST_F(OptimizerTest, MergeConsecutiveMatch) { auto q1 = "MATCH (a:person) MATCH (b:person) WHERE b.ID=0 MATCH (a)-[]->(b) " "RETURN COUNT(*);"; diff --git a/test/test_files/optional_match/optional_match.test b/test/test_files/optional_match/optional_match.test index 0bffe66073..f7e6e8866f 100644 --- a/test/test_files/optional_match/optional_match.test +++ b/test/test_files/optional_match/optional_match.test @@ -118,3 +118,44 @@ Hubert Blaine Wolfeschlegelsteinhausenbergerdorff| -STATEMENT MATCH (a:person) OPTIONAL MATCH (a)-[:knows]->(b:person) OPTIONAL MATCH (b)-[:knows]->(c:person) RETURN COUNT(*) ---- 1 41 + +-LOG MatchDropsNullFromLeftJoin +-STATEMENT MATCH (a:person) OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) WITH a, b MATCH (b) RETURN a.ID, b.ID +---- 3 +0|1 +2|1 +8|1 + +-LOG MatchDropsNullFromLeftJoinAlias +-STATEMENT MATCH (a:person) OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) WITH a, b AS c MATCH (c) RETURN a.ID, c.name +---- 3 +0|ABFsUni +2|ABFsUni +8|ABFsUni + +-LOG MatchDropsNullFromLeftJoinAfterSet +-STATEMENT MATCH (a:person) OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) SET a.age = a.age WITH a, b MATCH (b) RETURN a.ID, b.ID +---- 3 +0|1 +2|1 +8|1 + +-LOG MatchDropsNullFromOptionalCrossProduct +-STATEMENT MATCH (a:person) WHERE a.ID < 3 OPTIONAL MATCH (b:organisation) WHERE b.score > 1000 WITH a, b MATCH (b) RETURN a.ID, b.ID +---- 0 + +-LOG MatchDropsNullFromOptionalAccumulate +-STATEMENT OPTIONAL MATCH (b:organisation) WHERE b.score > 1000 WITH b MATCH (b) RETURN b.ID +---- 0 + +-LOG MatchOnNonNullVariableKeepsRows +-STATEMENT MATCH (a:person) OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) WITH a, b MATCH (a) RETURN a.ID, b.ID +---- 8 +0|1 +2|1 +3| +5| +7| +8|1 +9| +10| From d726b9a98a78f28e24b557dabf6c703f1a302553 Mon Sep 17 00:00:00 2001 From: Darek Date: Fri, 23 Oct 2026 16:35:04 +0200 Subject: [PATCH 2/5] test(optimizer): pin re-matching a node after chained OPTIONAL MATCHes 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 --- test/test_files/optional_match/optional_match.test | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/test_files/optional_match/optional_match.test b/test/test_files/optional_match/optional_match.test index f7e6e8866f..aff54bd307 100644 --- a/test/test_files/optional_match/optional_match.test +++ b/test/test_files/optional_match/optional_match.test @@ -148,6 +148,19 @@ Hubert Blaine Wolfeschlegelsteinhausenbergerdorff| -STATEMENT OPTIONAL MATCH (b:organisation) WHERE b.score > 1000 WITH b MATCH (b) RETURN b.ID ---- 0 +-LOG MatchDropsNullFromCascadeLast +-STATEMENT MATCH (a:person) WHERE a.ID IN [0, 8, 10] OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) OPTIONAL MATCH (b)<-[:studyAt]-(c:person) WHERE c.ID > a.ID WITH a, b, c MATCH (c) RETURN a.ID, b.ID, c.ID +---- 2 +0|1|2 +0|1|8 + +-LOG MatchDropsNullFromCascadeMiddle +-STATEMENT MATCH (a:person) WHERE a.ID IN [0, 8, 10] OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) OPTIONAL MATCH (b)<-[:studyAt]-(c:person) WHERE c.ID > a.ID WITH a, b, c MATCH (b) RETURN a.ID, b.ID, c.ID +---- 3 +0|1|2 +0|1|8 +8|1| + -LOG MatchOnNonNullVariableKeepsRows -STATEMENT MATCH (a:person) OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) WITH a, b MATCH (a) RETURN a.ID, b.ID ---- 8 From b999aa6d950a20f3351c34f8c7001b9fc40863f8 Mon Sep 17 00:00:00 2001 From: Darek Date: Fri, 23 Oct 2026 21:28:50 +0200 Subject: [PATCH 3/5] test(optimizer): pin a labelled MATCH on a node OPTIONAL MATCH left NULL 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 --- test/test_files/optional_match/optional_match.test | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/test_files/optional_match/optional_match.test b/test/test_files/optional_match/optional_match.test index aff54bd307..fc5581573c 100644 --- a/test/test_files/optional_match/optional_match.test +++ b/test/test_files/optional_match/optional_match.test @@ -126,6 +126,13 @@ Hubert Blaine Wolfeschlegelsteinhausenbergerdorff| 2|1 8|1 +-LOG MatchDropsNullFromLeftJoinLabel +-STATEMENT MATCH (a:person) OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) WITH a, b MATCH (b:organisation) RETURN a.ID, b.ID +---- 3 +0|1 +2|1 +8|1 + -LOG MatchDropsNullFromLeftJoinAlias -STATEMENT MATCH (a:person) OPTIONAL MATCH (a)-[:studyAt]->(b:organisation) WITH a, b AS c MATCH (c) RETURN a.ID, c.name ---- 3 From 988db26ee782c7c87de86274a1730617bed7d9a1 Mon Sep 17 00:00:00 2001 From: Darek Date: Fri, 23 Oct 2026 21:28:50 +0200 Subject: [PATCH 4/5] perf(planner): return a hash join's conditions by const reference 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 --- src/include/planner/operator/logical_hash_join.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/planner/operator/logical_hash_join.h b/src/include/planner/operator/logical_hash_join.h index f6274073f4..0105ab72f0 100644 --- a/src/include/planner/operator/logical_hash_join.h +++ b/src/include/planner/operator/logical_hash_join.h @@ -37,7 +37,7 @@ class LBUG_API LogicalHashJoin final : public LogicalOperator { static binder::expression_vector getJoinNodeIDs( const std::vector& joinConditions); - std::vector getJoinConditions() const { return joinConditions; } + const std::vector& getJoinConditions() const { return joinConditions; } common::JoinType getJoinType() const { return joinType; } bool hasMark() const { return mark != nullptr; } std::shared_ptr getMark() const { return mark; } From 6312dcb7c6c568bd50920d385a719ffa815564b3 Mon Sep 17 00:00:00 2001 From: Darek Date: Fri, 23 Oct 2026 22:04:20 +0200 Subject: [PATCH 5/5] style(optimizer): say why the NULL-key walk passes other operators through 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 --- src/optimizer/remove_unnecessary_join_optimizer.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/optimizer/remove_unnecessary_join_optimizer.cpp b/src/optimizer/remove_unnecessary_join_optimizer.cpp index e5c24c275e..24e82296f5 100644 --- a/src/optimizer/remove_unnecessary_join_optimizer.cpp +++ b/src/optimizer/remove_unnecessary_join_optimizer.cpp @@ -32,8 +32,10 @@ std::shared_ptr RemoveUnnecessaryJoinOptimizer::visitOperator( static bool nullsBuildOnlyColumns(JoinType joinType) { switch (joinType) { case JoinType::INNER: + return false; case JoinType::MARK: case JoinType::COUNT: + // These output only their probe side, so no build column reaches the parent. return false; case JoinType::LEFT: return true; @@ -80,6 +82,7 @@ static bool canBeNull(const LogicalOperator& op, const Expression& key) { } } break; default: + // Other operators pass a key through or bind it non-NULL (scan, extend, MERGE). break; } for (auto i = 0u; i < op.getNumChildren(); ++i) {