Fix bugs and segfaults

src/engine/CartesianProductJoin.h

@@ -67,7 +67,10 @@ class CartesianProductJoin : public Operation {
   bool knownEmptyResult() override;
 
   // The Cartesian product join can efficiently evaluate a limited result.
-  [[nodiscard]] bool supportsLimit() const override { return true; }
+  [[nodiscard]] bool supportsLimit(
+      [[maybe_unused]] bool lazyResult) const override {
+    return true;
+  }
 
  protected:
   // Don't promise any sorting of the result.

src/engine/IndexScan.h

@@ -92,8 +92,8 @@ class IndexScan : public Operation {
   }
 
   // Currently only the full scans support a limit clause.
-  [[nodiscard]] bool supportsLimit() const override {
-    return getResultWidth() == 3;
+  [[nodiscard]] bool supportsLimit(bool lazyResult) const override {
+    return !lazyResult && getResultWidth() == 3;
   }
 
   Permutation::Enum permutation() const { return permutation_; }

src/engine/Operation.cpp

@@ -159,7 +159,7 @@ std::shared_ptr<const Result> Operation::getResult(
       // Apply LIMIT and OFFSET, but only if the call to `computeResult` did not
       // already perform it. An example for an operation that directly computes
       // the Limit is a full index scan with three variables.
-      if (!supportsLimit()) {
+      if (!supportsLimit(!result.isDataEvaluated())) {
         runtimeInfo().addLimitOffsetRow(_limit, std::chrono::milliseconds{0},
                                         true);
         result.applyLimitOffset(_limit,
@@ -214,9 +214,10 @@ std::shared_ptr<const Result> Operation::getResult(
       return nullptr;
     }
 
-    if (result._resultPointer->resultTable()->isDataEvaluated()) {
-      updateRuntimeInformationOnSuccess(result, timer.msecs());
-    }
+    updateRuntimeInformationOnSuccess(
+        result, result._resultPointer->resultTable()->isDataEvaluated()
+                    ? timer.msecs()
+                    : result._resultPointer->runtimeInfo().totalTime_);
 
     if (result._resultPointer->resultTable()->isDataEvaluated()) {
       auto resultNumRows =
@@ -330,7 +331,7 @@ void Operation::updateRuntimeInformationOnSuccess(
 
 // ____________________________________________________________________________________________________________________
 void Operation::updateRuntimeInformationOnSuccess(
-    const ConcurrentLruCache ::ResultAndCacheStatus& resultAndCacheStatus,
+    const ConcurrentLruCache::ResultAndCacheStatus& resultAndCacheStatus,
     Milliseconds duration) {
   updateRuntimeInformationOnSuccess(
       *resultAndCacheStatus._resultPointer->resultTable(),

src/engine/Operation.h

@@ -103,7 +103,10 @@ class Operation {
 
   // True iff this operation directly implement a `OFFSET` and `LIMIT` clause on
   // its result.
-  [[nodiscard]] virtual bool supportsLimit() const { return false; }
+  [[nodiscard]] virtual bool supportsLimit(
+      [[maybe_unused]] bool lazyResult) const {
+    return false;
+  }
 
  public:
   virtual float getMultiplicity(size_t col) = 0;

src/engine/Result.cpp

@@ -37,20 +37,21 @@ auto Result::getMergedLocalVocab(const Result& resultTable1,
 LocalVocab Result::getCopyOfLocalVocab() const { return localVocab().clone(); }
 
 // _____________________________________________________________________________
-void Result::validateIdTable(const IdTable& idTable) const {
-  AD_CONTRACT_CHECK(std::ranges::all_of(sortedBy_, [&idTable](size_t numCols) {
+void Result::validateIdTable(const IdTable& idTable,
+                             const std::vector<ColumnIndex>& sortedBy) {
+  AD_CONTRACT_CHECK(std::ranges::all_of(sortedBy, [&idTable](size_t numCols) {
     return numCols < idTable.numColumns();
   }));
 
-  [[maybe_unused]] auto compareRowsByJoinColumns = [this](const auto& row1,
-                                                          const auto& row2) {
-    for (size_t col : sortedBy_) {
-      if (row1[col] != row2[col]) {
-        return row1[col] < row2[col];
-      }
-    }
-    return false;
-  };
+  [[maybe_unused]] auto compareRowsByJoinColumns =
+      [&sortedBy](const auto& row1, const auto& row2) {
+        for (size_t col : sortedBy) {
+          if (row1[col] != row2[col]) {
+            return row1[col] < row2[col];
+          }
+        }
+        return false;
+      };
   AD_EXPENSIVE_CHECK(std::ranges::is_sorted(idTable, compareRowsByJoinColumns));
 }
 
@@ -61,7 +62,7 @@ Result::Result(IdTable idTable, std::vector<ColumnIndex> sortedBy,
       sortedBy_{std::move(sortedBy)},
       localVocab_{std::move(localVocab.localVocab_)} {
   AD_CONTRACT_CHECK(localVocab_ != nullptr);
-  validateIdTable(std::get<IdTable>(data_));
+  validateIdTable(std::get<IdTable>(data_), sortedBy_);
 }
 
 // _____________________________________________________________________________
@@ -75,13 +76,13 @@ Result::Result(cppcoro::generator<IdTable> idTables,
                std::vector<ColumnIndex> sortedBy,
                SharedLocalVocabWrapper localVocab)
     : data_{ad_utility::CacheableGenerator{
-          [this, idTables = std::move(
-                     idTables)]() mutable -> cppcoro::generator<IdTable> {
+          [](auto idTables,
+             auto sortedBy) mutable -> cppcoro::generator<IdTable> {
             for (IdTable& idTable : idTables) {
-              validateIdTable(idTable);
+              validateIdTable(idTable, sortedBy);
               co_yield std::move(idTable);
             }
-          }()}},
+          }(std::move(idTables), sortedBy)}},
       sortedBy_{std::move(sortedBy)},
       localVocab_{std::move(localVocab.localVocab_)} {
   AD_CONTRACT_CHECK(localVocab_ != nullptr);
@@ -123,6 +124,7 @@ void Result::applyLimitOffset(
   // Apply the OFFSET clause. If the offset is `0` or the offset is larger
   // than the size of the `IdTable`, then this has no effect and runtime
   // `O(1)` (see the docs for `std::shift_left`).
+  AD_CONTRACT_CHECK(limitTimeCallback);
   AD_CONTRACT_CHECK(
       !std::holds_alternative<cppcoro::generator<const IdTable>>(data_));
   using Gen = ad_utility::CacheableGenerator<IdTable>;
@@ -140,12 +142,13 @@ void Result::applyLimitOffset(
       }
       for (auto&& idTable : original) {
         ad_utility::timer::Timer limitTimer{ad_utility::timer::Timer::Started};
+        size_t originalSize = idTable.numRows();
         modifyIdTable(idTable, limitOffset);
-        uint64_t offsetDelta = limitOffset.actualOffset(idTable.numRows());
+        uint64_t offsetDelta = limitOffset.actualOffset(originalSize);
         limitOffset._offset -= offsetDelta;
         if (limitOffset._limit.has_value()) {
           limitOffset._limit.value() -=
-              limitOffset.actualSize(idTable.numRows() - offsetDelta);
+              limitOffset.actualSize(originalSize - offsetDelta);
         }
         limitTimeCallback(limitTimer.msecs());
         if (limitOffset._offset == 0) {
@@ -175,11 +178,13 @@ void Result::enforceLimitOffset(const LimitOffsetClause& limitOffset) {
     auto generator =
         [](cppcoro::generator<IdTable> original,
            LimitOffsetClause limitOffset) -> cppcoro::generator<IdTable> {
+      size_t elementCount = 0;
       for (auto&& idTable : original) {
-        AD_CONTRACT_CHECK(idTable.numRows() ==
-                          limitOffset.actualSize(idTable.numRows()));
+        elementCount += idTable.numRows();
+        AD_CONTRACT_CHECK(elementCount <= limitOffset.actualSize(elementCount));
         co_yield std::move(idTable);
       }
+      AD_CONTRACT_CHECK(elementCount == limitOffset.actualSize(elementCount));
     }(std::move(std::get<Gen>(data_)).extractGenerator(), limitOffset);
     data_.emplace<Gen>(std::move(generator));
   } else {
@@ -378,9 +383,9 @@ Result Result::createResultWithFallback(
           std::chrono::duration_cast<std::chrono::milliseconds>(stop - start));
     }
   };
-  return Result{generator(std::move(original), std::move(fallback),
-                          std::move(onIteration)),
-                original->sortedBy_, original->localVocab_};
+  return Result{
+      generator(original, std::move(fallback), std::move(onIteration)),
+      original->sortedBy_, original->localVocab_};
 }
 
 // _____________________________________________________________________________
@@ -399,6 +404,6 @@ Result Result::createResultAsMasterConsumer(
       co_yield idTable;
     }
   };
-  return Result{generator(std::move(original), std::move(onIteration)),
+  return Result{generator(original, std::move(onIteration)),
                 original->sortedBy_, original->localVocab_};
 }

src/engine/Result.h

@@ -76,7 +76,8 @@ class Result {
   Result(cppcoro::generator<const IdTable> idTables,
          std::vector<ColumnIndex> sortedBy, LocalVocabPtr localVocab);
 
-  void validateIdTable(const IdTable& idTable) const;
+  static void validateIdTable(const IdTable& idTable,
+                              const std::vector<ColumnIndex>& sortedBy);
 
  public:
   // Construct from the given arguments (see above) and check the following

src/util/CacheableGenerator.h

@@ -2,8 +2,8 @@
 //   Chair of Algorithms and Data Structures.
 //   Author: Robin Textor-Falconi <textorr@informatik.uni-freiburg.de>
 
-#ifndef REUSABLEGENERATOR_H
-#define REUSABLEGENERATOR_H
+#ifndef CACHEABLEGENERATOR_H
+#define CACHEABLEGENERATOR_H
 
 #include <chrono>
 #include <optional>
@@ -67,6 +67,10 @@ class CacheableGenerator {
         }
         return;
       }
+      if (generatorIterator_.has_value() &&
+          generatorIterator_.value() == generator_.end()) {
+        return;
+      }
       if (masterState_ == MasterIteratorState::MASTER_STARTED) {
         if (!isMaster) {
           conditionVariable_.wait(lock, [this, index]() {
@@ -118,7 +122,7 @@ class CacheableGenerator {
    public:
     bool isDone(size_t index) noexcept {
       std::shared_lock lock{mutex_};
-      return index == cachedValues_.size() && generatorIterator_.has_value() &&
+      return index >= cachedValues_.size() && generatorIterator_.has_value() &&
              generatorIterator_.value() == generator_.end();
     }
 
@@ -201,7 +205,7 @@ class CacheableGenerator {
 
    public:
     explicit Iterator(std::weak_ptr<ComputationStorage> storage, bool isMaster)
-        : storage_{storage,
+        : storage_{std::move(storage),
                    [isMaster](auto&& storage) {
                      if (isMaster) {
                        auto pointer = storage.lock();
@@ -214,7 +218,7 @@ class CacheableGenerator {
     }
 
     friend bool operator==(const Iterator& it, IteratorSentinel) noexcept {
-      return !it.storage()->isDone(it.currentIndex_);
+      return it.storage()->isDone(it.currentIndex_);
     }
 
     friend bool operator!=(const Iterator& it, IteratorSentinel s) noexcept {
@@ -238,14 +242,14 @@ class CacheableGenerator {
     // Need to provide post-increment operator to implement the 'Range' concept.
     void operator++(int) { (void)operator++(); }
 
-    Reference operator*() const noexcept {
+    Reference operator*() const {
       return storage()->getCachedValue(currentIndex_);
     }
 
-    Pointer operator->() const noexcept { return std::addressof(operator*()); }
+    Pointer operator->() const { return std::addressof(operator*()); }
   };
 
-  Iterator begin(bool isMaster = false) const noexcept {
+  Iterator begin(bool isMaster = false) const {
     return Iterator{computationStorage_, isMaster};
   }
 
@@ -277,4 +281,4 @@ class CacheableGenerator {
 };
 };  // namespace ad_utility
 
-#endif  // REUSABLEGENERATOR_H
+#endif  // CACHEABLEGENERATOR_H

test/engine/ValuesForTesting.h

@@ -59,7 +59,9 @@ class ValuesForTesting : public Operation {
     }
     return {std::move(table), resultSortedOn(), localVocab_.clone()};
   }
-  bool supportsLimit() const override { return supportsLimit_; }
+  bool supportsLimit([[maybe_unused]] bool lazyResult) const override {
+    return supportsLimit_;
+  }
 
  private:
   // ___________________________________________________________________________