This now seems to work,

feed it to the tools and comment it. Signed-off-by: Johannes Kalmbach <[email protected]>
ad-freiburg · Apr 19, 2024 · 6a52ae5 · 6a52ae5
1 parent a6c6dab
commit 6a52ae5
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 18 deletions.
diff --git a/src/engine/TransitivePathBase.cpp b/src/engine/TransitivePathBase.cpp
@@ -354,9 +354,17 @@ std::shared_ptr<TransitivePathBase> TransitivePathBase::bindLeftOrRightSide(
   } else {
     rhs.treeAndCol_ = {leftOrRightOp, inputCol};
   }
-  std::shared_ptr<TransitivePathBase> p =
-      TransitivePathBase::makeTransitivePath(getExecutionContext(), subtree_,
-                                             lhs, rhs, minDist_, maxDist_);
+  std::vector<std::shared_ptr<TransitivePathBase>> candidates;
+  candidates.push_back(TransitivePathBase::makeTransitivePath(
+      getExecutionContext(), subtree_, lhs, rhs, minDist_, maxDist_));
+  for (const auto& alternativeSubtree : alternativeSubtrees()) {
+    candidates.push_back(TransitivePathBase::makeTransitivePath(
+        getExecutionContext(), alternativeSubtree, lhs, rhs, minDist_,
+        maxDist_));
+  }
+
+  auto& p = *std::ranges::min_element(
+      candidates, {}, [](const auto& tree) { return tree->getCostEstimate(); });
 
   // Note: The `variable` in the following structured binding is `const`, even
   // if we bind by value. We deliberately make one unnecessary copy of the
@@ -374,7 +382,7 @@ std::shared_ptr<TransitivePathBase> TransitivePathBase::bindLeftOrRightSide(
     p->variableColumns_[variable] = columnIndexWithType;
     p->resultWidth_++;
   }
-  return p;
+  return std::move(p);
 }
 
 // _____________________________________________________________________________

diff --git a/src/engine/TransitivePathBase.h b/src/engine/TransitivePathBase.h
@@ -34,8 +34,7 @@ struct TransitivePathSide {
       os << "Id: " << std::get<Id>(value_);
     }
 
-    os << ", subColumn: " <<
-        subCol_ << "to " << outputCol_;
+    os << ", subColumn: " << subCol_ << "to " << outputCol_;
 
     if (treeAndCol_.has_value()) {
       const auto& [tree, col] = treeAndCol_.value();
@@ -270,4 +269,9 @@ class TransitivePathBase : public Operation {
   virtual std::shared_ptr<TransitivePathBase> bindLeftOrRightSide(
       std::shared_ptr<QueryExecutionTree> leftOrRightOp, size_t inputCol,
       bool isLeft) const;
+
+  virtual std::span<const std::shared_ptr<QueryExecutionTree>>
+  alternativeSubtrees() const {
+    return {};
+  }
 };
diff --git a/src/engine/TransitivePathBinSearch.cpp b/src/engine/TransitivePathBinSearch.cpp
@@ -19,7 +19,7 @@ TransitivePathBinSearch::TransitivePathBinSearch(
                                        std::move(rightSide), minDist, maxDist) {
   auto [startSide, targetSide] = decideDirection();
   alternativelySortedSubtree_ = QueryExecutionTree::createSortedTree(
-      subtree_, {targetSide.subCol_, targetSide.subCol_});
+      subtree_, {targetSide.subCol_, startSide.subCol_});
   subtree_ = QueryExecutionTree::createSortedTree(
       subtree_, {startSide.subCol_, targetSide.subCol_});
 }

diff --git a/src/engine/TransitivePathBinSearch.h b/src/engine/TransitivePathBinSearch.h
@@ -69,19 +69,17 @@ class TransitivePathBinSearch : public TransitivePathImpl<BinSearchMap> {
                           TransitivePathSide rightSide, size_t minDist,
                           size_t maxDist);
 
-
   std::vector<ColumnIndex> resultSortedOn() const override;
 
-  // TODO<joka921> Overwrite s.t. we can do this with the alternative subtree.
-  virtual std::shared_ptr<TransitivePathBase> bindLeftOrRightSide(
-      std::shared_ptr<QueryExecutionTree> leftOrRightOp, size_t inputCol,
-      bool isLeft) const;
-
  private:
   // initialize the map from the subresult
   BinSearchMap setupEdgesMap(
       const IdTable& dynSub, const TransitivePathSide& startSide,
       const TransitivePathSide& targetSide) const override;
 
   std::shared_ptr<QueryExecutionTree> alternativelySortedSubtree_;
+  std::span<const std::shared_ptr<QueryExecutionTree>> alternativeSubtrees()
+      const override {
+    return {&alternativelySortedSubtree_, 1};
+  }
 };
diff --git a/test/QueryPlannerTest.cpp b/test/QueryPlannerTest.cpp
@@ -756,8 +756,8 @@ TEST(QueryPlanner, TransitivePathBindLeft) {
 
 TEST(QueryPlanner, TransitivePathBindRight) {
   auto scan = h::IndexScanFromStrings;
-  TransitivePathSide left{std::nullopt, 0, Variable("?x"), 0};
-  TransitivePathSide right{std::nullopt, 1, Variable("?y"), 1};
+  TransitivePathSide left{std::nullopt, 1, Variable("?x"), 0};
+  TransitivePathSide right{std::nullopt, 0, Variable("?y"), 1};
   h::expect(
       "SELECT ?x ?y WHERE {"
       "?x <p>* ?y."
@@ -766,9 +766,9 @@ TEST(QueryPlanner, TransitivePathBindRight) {
           left, right, 0, std::numeric_limits<size_t>::max(),
           scan("?y", "<p>", "<o>"),
           scan("?_qlever_internal_variable_query_planner_0", "<p>",
-                       "?_qlever_internal_variable_query_planner_1")),
-      ad_utility::testing::getQec("<x> <p> <o>. <x2> <p> <o2>")
-      );
+               "?_qlever_internal_variable_query_planner_1",
+               {Permutation::POS})),
+      ad_utility::testing::getQec("<x> <p> <o>. <x2> <p> <o2>"));
 }
 
 // __________________________________________________________________________