From b740866bb7c9d7efd344c9a29a7e14294d57586f Mon Sep 17 00:00:00 2001
From: Johannes Kalmbach <johannes.kalmbach@gmail.com>
Date: Thu, 13 Jun 2024 22:25:39 +0200
Subject: [PATCH] A first round of reviews.

---
 src/engine/IndexScan.cpp         |  2 +-
 src/index/CompressedRelation.cpp | 18 +++++++++++-------
 test/CompressedRelationsTest.cpp |  5 +----
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/engine/IndexScan.cpp b/src/engine/IndexScan.cpp
index e361174cf3..2bcfd19a70 100644
--- a/src/engine/IndexScan.cpp
+++ b/src/engine/IndexScan.cpp
@@ -289,7 +289,7 @@ Permutation::IdTableGenerator IndexScan::getLazyScan(
   // If there is a LIMIT or OFFSET clause that constrains the scan
   // (which can happen with an explicit subquery), we cannot use the prefiltered
   // blocks, as we currently have no mechanism to include limits and offsets
-  // into the prefiltering.
+  // into the prefiltering (`std::nullopt` means `scan all blocks`).
   auto actualBlocks = s.getLimit().isUnconstrained()
                           ? std::optional{std::move(blocks)}
                           : std::nullopt;
diff --git a/src/index/CompressedRelation.cpp b/src/index/CompressedRelation.cpp
index c8cc0a2641..1d4ec23b77 100644
--- a/src/index/CompressedRelation.cpp
+++ b/src/index/CompressedRelation.cpp
@@ -26,21 +26,25 @@ using namespace std::chrono_literals;
 static auto getBeginAndEnd(auto& range) {
   return std::pair{std::ranges::begin(range), std::ranges::end(range)};
 }
+
+// modify the `block` according to the `limitOffset`. Also modify the
+// `limitOffset` to reflect the parts of the LIMIT and OFFSET that have been
+// performed by pruning this `block`.
 static void pruneBlock(auto& block, LimitOffsetClause& limitOffset) {
   auto& offset = limitOffset._offset;
-  auto relevantOffset = std::min(static_cast<size_t>(offset), block.size());
-  if (relevantOffset == block.size()) {
+  auto offsetInBlock = std::min(static_cast<size_t>(offset), block.size());
+  if (offsetInBlock == block.size()) {
     block.clear();
   } else {
-    block.erase(block.begin(), block.begin() + relevantOffset);
+    block.erase(block.begin(), block.begin() + offsetInBlock);
   }
-  offset -= relevantOffset;
+  offset -= offsetInBlock;
   auto& limit = limitOffset._limit;
-  auto relevantLimit =
+  auto limitInBlock =
       std::min(block.size(), static_cast<size_t>(limit.value_or(block.size())));
-  block.resize(relevantLimit);
+  block.resize(limitInBlock);
   if (limit.has_value()) {
-    limit.value() -= relevantLimit;
+    limit.value() -= limitInBlock;
   }
 }
 
diff --git a/test/CompressedRelationsTest.cpp b/test/CompressedRelationsTest.cpp
index 99efcd6a7e..c2aad9d7c6 100644
--- a/test/CompressedRelationsTest.cpp
+++ b/test/CompressedRelationsTest.cpp
@@ -181,7 +181,7 @@ void testCompressedRelations(const auto& inputs, std::string testCaseName,
     const auto& col1And2 = inputs[i].col1And2_;
     checkThatTablesAreEqual(col1And2, table);
     table.clear();
-    // Check that the scans also work with various values for LIMIT and OFFSET
+    // Check that the scans also work with various values for LIMIT and OFFSET.
     std::vector<LimitOffsetClause> limitOffsetClauses{
         {std::nullopt, 5}, {5, 0}, {std::nullopt, 12}, {12, 0}, {7, 5}};
     for (const auto& limitOffset : limitOffsetClauses) {
@@ -192,9 +192,6 @@ void testCompressedRelations(const auto& inputs, std::string testCaseName,
       col1And2.erase(
           col1And2.begin(),
           col1And2.begin() + limitOffset.actualOffset(col1And2.size()));
-      if (col1And2.size() != table.numRows()) {
-        std::cerr << "mismatch " << std::endl;
-      }
       checkThatTablesAreEqual(col1And2, table);
     }
     for (const auto& block : reader.lazyScan(