diff --git a/src/engine/TransitivePathBase.cpp b/src/engine/TransitivePathBase.cpp index df3a0ac5d0..68f8bc1df7 100644 --- a/src/engine/TransitivePathBase.cpp +++ b/src/engine/TransitivePathBase.cpp @@ -354,9 +354,17 @@ std::shared_ptr TransitivePathBase::bindLeftOrRightSide( } else { rhs.treeAndCol_ = {leftOrRightOp, inputCol}; } - std::shared_ptr p = - TransitivePathBase::makeTransitivePath(getExecutionContext(), subtree_, - lhs, rhs, minDist_, maxDist_); + std::vector> 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::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 index 785ace56cf..4ff0f66b7e 100644 --- a/src/engine/TransitivePathBase.h +++ b/src/engine/TransitivePathBase.h @@ -34,8 +34,7 @@ struct TransitivePathSide { os << "Id: " << std::get(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 bindLeftOrRightSide( std::shared_ptr leftOrRightOp, size_t inputCol, bool isLeft) const; + + virtual std::span> + alternativeSubtrees() const { + return {}; + } }; diff --git a/src/engine/TransitivePathBinSearch.cpp b/src/engine/TransitivePathBinSearch.cpp index a9f178d699..faceb2259f 100644 --- 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 index 72b1c1dd3b..c06507d640 100644 --- a/src/engine/TransitivePathBinSearch.h +++ b/src/engine/TransitivePathBinSearch.h @@ -69,14 +69,8 @@ class TransitivePathBinSearch : public TransitivePathImpl { TransitivePathSide rightSide, size_t minDist, size_t maxDist); - std::vector resultSortedOn() const override; - // TODO Overwrite s.t. we can do this with the alternative subtree. - virtual std::shared_ptr bindLeftOrRightSide( - std::shared_ptr leftOrRightOp, size_t inputCol, - bool isLeft) const; - private: // initialize the map from the subresult BinSearchMap setupEdgesMap( @@ -84,4 +78,8 @@ class TransitivePathBinSearch : public TransitivePathImpl { const TransitivePathSide& targetSide) const override; std::shared_ptr alternativelySortedSubtree_; + std::span> alternativeSubtrees() + const override { + return {&alternativelySortedSubtree_, 1}; + } }; diff --git a/test/QueryPlannerTest.cpp b/test/QueryPlannerTest.cpp index fc70a7d404..4e2b0975e4 100644 --- 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

* ?y." @@ -766,9 +766,9 @@ TEST(QueryPlanner, TransitivePathBindRight) { left, right, 0, std::numeric_limits::max(), scan("?y", "

", ""), scan("?_qlever_internal_variable_query_planner_0", "

", - "?_qlever_internal_variable_query_planner_1")), - ad_utility::testing::getQec("

.

") - ); + "?_qlever_internal_variable_query_planner_1", + {Permutation::POS})), + ad_utility::testing::getQec("

.

")); } // __________________________________________________________________________