Skip to content

Commit

Permalink
Don't use zero cost for cached subtrees
Browse files Browse the repository at this point in the history
This continues #1736, which changed the size estimate for cached subtrees
from the exact size to the estimate size. Analogous to that, now also
change the cost estimate for cached subtrees from zero to the normal
cost estimate.

Introduce a runtime parameter `zero-cost-for-cached-subtree` to revert
to the old behavior if needed (the default is `false`).
  • Loading branch information
Hannah Bast committed Feb 9, 2025
1 parent d7a70c7 commit 612a8df
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 4 deletions.
9 changes: 7 additions & 2 deletions src/engine/QueryExecutionTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,15 @@ QueryExecutionTree::selectedVariablesToColumnIndices(

// _____________________________________________________________________________
size_t QueryExecutionTree::getCostEstimate() {
if (cachedResult_) {
// result is pinned in cache. Nothing to compute
// If the result is cached and `zero-cost-for-cached-subtrees` is set to
// `true`, we set the cost estimate to zero.
if (cachedResult_ &&
RuntimeParameters().get<"zero-cost-for-cached-subtree">()) {
return 0;
}

// Otherwise, we return the cost estimate of the root operation. For index
// scans, we assume one unit of work per result row.
if (getRootOperation()->isIndexScanWithNumVariables(1)) {
return getSizeEstimate();
} else {
Expand Down
3 changes: 3 additions & 0 deletions src/global/RuntimeParameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ inline auto& RuntimeParameters() {
// its size estimate will be the size of the block divided by this
// value.
SizeT<"small-index-scan-size-estimate-divisor">{5},
// Determines whether the cost estimate for a cached subtree should be
// set to zero in query planning.
Bool<"zero-cost-for-cached-subtree">{false},
};
}();
return params;
Expand Down
13 changes: 11 additions & 2 deletions test/OperationTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -242,15 +242,24 @@ TEST(OperationTest, estimatesForCachedResults) {

[[maybe_unused]] auto res = qet->getResult();
}
// The result is now cached inside the static execution context, if we create
// the same operation again, the cost estimate is 0. The size estimate doesn't
// The result is now cached inside the static execution context. If we create
// the same operation again and `zero-cost-for-cached-subtrees` is set to
// `true`, the cost estimate should be zero. The size estimate does not
// change (see the `getCostEstimate` function for details on why).
{
RuntimeParameters().set<"zero-cost-for-cached-subtree">(true);
auto qet = makeQet();
EXPECT_EQ(qet->getCacheKey(), qet->getRootOperation()->getCacheKey());
EXPECT_EQ(qet->getSizeEstimate(), 24u);
EXPECT_EQ(qet->getCostEstimate(), 0u);
}
{
RuntimeParameters().set<"zero-cost-for-cached-subtree">(false);
auto qet = makeQet();
EXPECT_EQ(qet->getCacheKey(), qet->getRootOperation()->getCacheKey());
EXPECT_EQ(qet->getSizeEstimate(), 24u);
EXPECT_EQ(qet->getCostEstimate(), 210u);
}
}

// ________________________________________________
Expand Down

0 comments on commit 612a8df

Please sign in to comment.