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/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; } diff --git a/src/optimizer/remove_unnecessary_join_optimizer.cpp b/src/optimizer/remove_unnecessary_join_optimizer.cpp index 793f4f953e..24e82296f5 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,84 @@ 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: + 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; + } + 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: + // Other operators pass a key through or bind it non-NULL (scan, extend, MERGE). + 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 +126,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..fc5581573c 100644 --- a/test/test_files/optional_match/optional_match.test +++ b/test/test_files/optional_match/optional_match.test @@ -118,3 +118,64 @@ 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 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 +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 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 +0|1 +2|1 +3| +5| +7| +8|1 +9| +10|