From 612a8df15e5cf241f3a534ac0a299b25467c797e Mon Sep 17 00:00:00 2001 From: Hannah Bast Date: Sun, 9 Feb 2025 02:49:12 +0100 Subject: [PATCH 1/2] Don't use zero cost for cached subtrees 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`). --- src/engine/QueryExecutionTree.cpp | 9 +++++++-- src/global/RuntimeParameters.h | 3 +++ test/OperationTest.cpp | 13 +++++++++++-- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/engine/QueryExecutionTree.cpp b/src/engine/QueryExecutionTree.cpp index 49ceb340aa..cb4a905021 100644 --- a/src/engine/QueryExecutionTree.cpp +++ b/src/engine/QueryExecutionTree.cpp @@ -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 { diff --git a/src/global/RuntimeParameters.h b/src/global/RuntimeParameters.h index f61705e7fc..33154d9849 100644 --- a/src/global/RuntimeParameters.h +++ b/src/global/RuntimeParameters.h @@ -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; diff --git a/test/OperationTest.cpp b/test/OperationTest.cpp index 96c6525221..75d086313b 100644 --- a/test/OperationTest.cpp +++ b/test/OperationTest.cpp @@ -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); + } } // ________________________________________________ From bed12e4776c72245cbb3314453f498d1d57c93c7 Mon Sep 17 00:00:00 2001 From: Hannah Bast Date: Sun, 9 Feb 2025 14:26:37 +0100 Subject: [PATCH 2/2] Address Robin's comment and improve parameter name --- src/engine/QueryExecutionTree.cpp | 6 +++--- src/global/RuntimeParameters.h | 2 +- test/OperationTest.cpp | 12 ++++++++---- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/engine/QueryExecutionTree.cpp b/src/engine/QueryExecutionTree.cpp index cb4a905021..cef01b1129 100644 --- a/src/engine/QueryExecutionTree.cpp +++ b/src/engine/QueryExecutionTree.cpp @@ -77,10 +77,10 @@ QueryExecutionTree::selectedVariablesToColumnIndices( // _____________________________________________________________________________ size_t QueryExecutionTree::getCostEstimate() { - // If the result is cached and `zero-cost-for-cached-subtrees` is set to - // `true`, we set the cost estimate to zero. + // If the result is cached and `zero-cost-estimate-for-cached-subtrees` is set + // to `true`, we set the cost estimate to zero. if (cachedResult_ && - RuntimeParameters().get<"zero-cost-for-cached-subtree">()) { + RuntimeParameters().get<"zero-cost-estimate-for-cached-subtree">()) { return 0; } diff --git a/src/global/RuntimeParameters.h b/src/global/RuntimeParameters.h index 33154d9849..f05c23b081 100644 --- a/src/global/RuntimeParameters.h +++ b/src/global/RuntimeParameters.h @@ -62,7 +62,7 @@ inline auto& RuntimeParameters() { 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}, + Bool<"zero-cost-estimate-for-cached-subtree">{false}, }; }(); return params; diff --git a/test/OperationTest.cpp b/test/OperationTest.cpp index 75d086313b..c564707514 100644 --- a/test/OperationTest.cpp +++ b/test/OperationTest.cpp @@ -243,18 +243,22 @@ 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 and `zero-cost-for-cached-subtrees` is set to - // `true`, the cost estimate should be zero. The size estimate does not + // the same operation again and `zero-cost-estimate-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 restoreWhenScopeEnds = + setRuntimeParameterForTest<"zero-cost-estimate-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 restoreWhenScopeEnds = + setRuntimeParameterForTest<"zero-cost-estimate-for-cached-subtree">( + false); auto qet = makeQet(); EXPECT_EQ(qet->getCacheKey(), qet->getRootOperation()->getCacheKey()); EXPECT_EQ(qet->getSizeEstimate(), 24u);