Allow reordering of joins for regular match subquery

Temp disable reorder for hash join

Re-enable hash-join reorder

Undo test change

Correctly populate schema cardinality multiplier in cardinality multiplier

Temp raise buffer pool size for lsqb test

Author: royi-luo

src/include/optimizer/cardinality_updater.h

@@ -27,6 +27,7 @@ class CardinalityUpdater : public LogicalOperatorVisitor {
     void visitOperatorDefault(planner::LogicalOperator* op);
     void visitScanNodeTable(planner::LogicalOperator* op) override;
     void visitExtend(planner::LogicalOperator* op) override;
+    void visitRecursiveExtend(planner::LogicalOperator* op) override;
     void visitHashJoin(planner::LogicalOperator* op) override;
     void visitCrossProduct(planner::LogicalOperator* op) override;
     void visitIntersect(planner::LogicalOperator* op) override;

src/optimizer/cardinality_updater.cpp

@@ -2,6 +2,7 @@
 
 #include "planner/join_order/cardinality_estimator.h"
 #include "planner/operator/extend/logical_extend.h"
+#include "planner/operator/extend/logical_recursive_extend.h"
 #include "planner/operator/logical_aggregate.h"
 #include "planner/operator/logical_filter.h"
 #include "planner/operator/logical_flatten.h"
@@ -19,6 +20,8 @@ void CardinalityUpdater::visitOperator(planner::LogicalOperator* op) {
     for (auto i = 0u; i < op->getNumChildren(); ++i) {
         visitOperator(op->getChild(i).get());
     }
+    // we need to recompute the cardinality multipliers for each factorized group
+    op->computeFactorizedSchema();
     visitOperatorSwitchWithDefault(op);
 }
 
@@ -83,6 +86,18 @@ void CardinalityUpdater::visitExtend(planner::LogicalOperator* op) {
     const auto extensionRate = cardinalityEstimator.getExtensionRate(*extend.getRel(),
         *extend.getBoundNode(), transaction);
     extend.setCardinality(cardinalityEstimator.estimateExtend(extensionRate, *op->getChild(0)));
+    auto group = extend.getSchema()->getGroup(extend.getNbrNode()->getInternalID());
+    group->setMultiplier(extensionRate);
+}
+
+void CardinalityUpdater::visitRecursiveExtend(planner::LogicalOperator* op) {
+    KU_ASSERT(transaction);
+    auto& extend = op->cast<planner::LogicalRecursiveExtend&>();
+    const auto extensionRate = cardinalityEstimator.getExtensionRate(*extend.getRel(),
+        *extend.getBoundNode(), transaction);
+    extend.setCardinality(cardinalityEstimator.estimateExtend(extensionRate, *op->getChild(0)));
+    auto group = extend.getSchema()->getGroup(extend.getNbrNode()->getInternalID());
+    group->setMultiplier(extensionRate);
 }
 
 void CardinalityUpdater::visitHashJoin(planner::LogicalOperator* op) {

src/optimizer/factorization_rewriter.cpp

@@ -1,6 +1,7 @@
 #include "optimizer/factorization_rewriter.h"
 
 #include "binder/expression_visitor.h"
+#include "planner/operator/extend/logical_extend.h"
 #include "planner/operator/extend/logical_recursive_extend.h"
 #include "planner/operator/factorization/flatten_resolver.h"
 #include "planner/operator/logical_accumulate.h"

src/planner/plan/plan_subquery.cpp

@@ -2,6 +2,7 @@
 #include "binder/expression/property_expression.h"
 #include "binder/expression/subquery_expression.h"
 #include "binder/expression_visitor.h"
+#include "planner/join_order/cost_model.h"
 #include "planner/operator/factorization/flatten_resolver.h"
 #include "planner/planner.h"
 
@@ -100,6 +101,17 @@ void Planner::planOptionalMatch(const QueryGraphCollection& queryGraphCollection
     }
 }
 
+template<std::invocable<LogicalPlan&, LogicalPlan&, LogicalPlan&> AppendJoinFunc,
+    std::invocable<LogicalPlan&, LogicalPlan&> EstimateJoinCostFunc>
+static void planRegularMatchJoinOrder(LogicalPlan& leftPlan, LogicalPlan& rightPlan,
+    const AppendJoinFunc& appendJoinFunc, const EstimateJoinCostFunc& estimateJoinCostFunc) {
+    if (estimateJoinCostFunc(leftPlan, rightPlan) <= estimateJoinCostFunc(rightPlan, leftPlan)) {
+        appendJoinFunc(leftPlan, rightPlan, leftPlan);
+    } else {
+        appendJoinFunc(rightPlan, leftPlan, leftPlan);
+    }
+}
+
 void Planner::planRegularMatch(const QueryGraphCollection& queryGraphCollection,
     const expression_vector& predicates, LogicalPlan& leftPlan) {
     expression_vector predicatesToPushDown, predicatesToPullUp;
@@ -124,7 +136,15 @@ void Planner::planRegularMatch(const QueryGraphCollection& queryGraphCollection,
         if (leftPlan.hasUpdate()) {
             appendCrossProduct(*rightPlan, leftPlan, leftPlan);
         } else {
-            appendCrossProduct(leftPlan, *rightPlan, leftPlan);
+            planRegularMatchJoinOrder(
+                leftPlan, *rightPlan,
+                [this](LogicalPlan& leftPlan, LogicalPlan& rightPlan, LogicalPlan& resultPlan) {
+                    appendCrossProduct(leftPlan, rightPlan, resultPlan);
+                },
+                [](LogicalPlan&, LogicalPlan& rightPlan) {
+                    // we want to minimize the cardinality of the build plan
+                    return rightPlan.getCardinality();
+                });
         }
     } else {
         // TODO(Xiyang): there is a question regarding if we want to plan as a correlated subquery
@@ -137,7 +157,16 @@ void Planner::planRegularMatch(const QueryGraphCollection& queryGraphCollection,
         if (leftPlan.hasUpdate()) {
             appendHashJoin(joinNodeIDs, JoinType::INNER, *rightPlan, leftPlan, leftPlan);
         } else {
-            appendHashJoin(joinNodeIDs, JoinType::INNER, leftPlan, *rightPlan, leftPlan);
+            planRegularMatchJoinOrder(
+                leftPlan, *rightPlan,
+                [this, &joinNodeIDs](LogicalPlan& leftPlan, LogicalPlan& rightPlan,
+                    LogicalPlan& resultPlan) {
+                    appendHashJoin(joinNodeIDs, JoinType::INNER, leftPlan, rightPlan, resultPlan);
+                },
+                [&joinNodeIDs](LogicalPlan& leftPlan, LogicalPlan& rightPlan) {
+                    return CostModel::computeHashJoinCost(joinNodeIDs, leftPlan, rightPlan);
+                });
+            // appendHashJoin(joinNodeIDs, JoinType::INNER, leftPlan, *rightPlan, leftPlan);
         }
     }
     for (auto& predicate : predicatesToPullUp) {

test/planner/CMakeLists.txt

@@ -1 +1,3 @@
-add_kuzu_test(planner_tests cardinality_test.cpp)
+add_kuzu_test(planner_tests
+    cardinality_test.cpp
+    planner_test.cpp)

test/planner/planner_test.cpp

@@ -0,0 +1,65 @@
+#include "graph_test/graph_test.h"
+#include "planner/operator/logical_plan_util.h"
+#include "test_runner/test_runner.h"
+
+namespace kuzu {
+namespace testing {
+
+class PlannerTest : public DBTest {
+public:
+    std::string getInputDir() override {
+        return TestHelper::appendKuzuRootPath("dataset/tinysnb/");
+    }
+
+    std::string getEncodedPlan(const std::string& query) {
+        return planner::LogicalPlanUtil::encodeJoin(*getRoot(query));
+    }
+    std::unique_ptr<planner::LogicalPlan> getRoot(const std::string& query) {
+        return TestRunner::getLogicalPlan(query, *conn);
+    }
+    std::pair<planner::LogicalOperator*, planner::LogicalOperator*> getSource(
+        planner::LogicalOperator* op, planner::LogicalOperator* parent = nullptr) {
+        if (op->getNumChildren() == 0) {
+            return {parent, op};
+        }
+        return getSource(op->getChild(0).get(), op);
+    }
+    planner::LogicalOperator* getOpWithType(planner::LogicalOperator* op,
+        planner::LogicalOperatorType type) {
+        if (op->getOperatorType() == type) {
+            return op;
+        }
+        if (op->getNumChildren() == 0) {
+            return nullptr;
+        }
+        return getOpWithType(op->getChild(0).get(), type);
+    }
+};
+
+TEST_F(PlannerTest, TestSubqueryJoinOrder) {
+    // Cross Product
+    {
+        // We should pick the smaller table to be on the build side
+        auto query = "MATCH (a:person) WITH a MATCH (b:organisation) RETURN *";
+        EXPECT_STREQ("CP(){S(a)}{S(b)}", getEncodedPlan(query).c_str());
+        auto queryFlipped = "MATCH (b:organisation) WITH b MATCH (a:person) RETURN *";
+        EXPECT_STREQ("CP(){S(a)}{S(b)}", getEncodedPlan(queryFlipped).c_str());
+    }
+
+    // Hash Join
+    {
+        // cardinality(person) > cardinality(studyAt)
+        // scan(person) should go on probe side
+        auto query = "MATCH (a:person) WITH a MATCH (a)-[s:studyAt]->(b:organisation) RETURN *";
+        EXPECT_STREQ("HJ(a._ID){S(a)}{HJ(b._ID){S(b)}{E(b)S(a)}}", getEncodedPlan(query).c_str());
+
+        // cardinality(organisation) < cardinality(studyAt) + cardinality(workAt)
+        // scan(organisation) should go on build side
+        auto queryFlipped =
+            "MATCH (b:organisation) WITH b MATCH (a:person)-[s:studyAt|:workAt]->(b) RETURN *";
+        EXPECT_STREQ("HJ(b._ID){E(b)S(a)}{S(b)}", getEncodedPlan(queryFlipped).c_str());
+    }
+}
+
+} // namespace testing
+} // namespace kuzu

test/test_files/lsqb/lsqb_queries.test

@@ -1,5 +1,5 @@
 -DATASET CSV lsqb-sf01
--BUFFER_POOL_SIZE 1073741824
+-BUFFER_POOL_SIZE 4294967296
 
 --