Don't use the exact size from the cache for query planning

src/engine/QueryExecutionTree.cpp

@@ -91,15 +91,11 @@ size_t QueryExecutionTree::getCostEstimate() {
 // _____________________________________________________________________________
 size_t QueryExecutionTree::getSizeEstimate() {
   if (!sizeEstimate_.has_value()) {
-    if (cachedResult_) {
-      AD_CORRECTNESS_CHECK(cachedResult_->isFullyMaterialized());
-      sizeEstimate_ = cachedResult_->idTable().size();
-    } else {
-      // if we are in a unit test setting and there is no QueryExecutionContest
-      // specified it is the rootOperation_'s obligation to handle this case
-      // correctly
-      sizeEstimate_ = rootOperation_->getSizeEstimate();
-    }
+    // Note: Previously we used the exact size instead of the estimate for
+    // results that were already in the cache. This however often lead to poor
+    // planning, because the query planner compared exact sizes with estimates,
+    // which lead to worse plans than just conistently choosing the estimate.
+    sizeEstimate_ = rootOperation_->getSizeEstimate();
   }
   return sizeEstimate_.value();
 }

test/OperationTest.cpp

@@ -242,12 +242,12 @@ 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 and the size estimate is
-  // exact (3 rows).
+  // the same operation again, the cost estimate is 0. The size estimate doesn't
+  // change (see the `getCostEstimate` function for details on why).
   {
     auto qet = makeQet();
     EXPECT_EQ(qet->getCacheKey(), qet->getRootOperation()->getCacheKey());
-    EXPECT_EQ(qet->getSizeEstimate(), 3u);
+    EXPECT_EQ(qet->getSizeEstimate(), 24u);
     EXPECT_EQ(qet->getCostEstimate(), 0u);
   }
 }