Final modification of ShadowRows in FilterExecutor. All but profiling tests pass. This needs some discussion about overfetching of data, and propagation of hardLimit

arangod/Aql/AqlItemBlockInputRange.cpp

@@ -39,22 +39,20 @@ AqlItemBlockInputRange::AqlItemBlockInputRange(ExecutorState state)
 
 AqlItemBlockInputRange::AqlItemBlockInputRange(ExecutorState state,
                                                SharedAqlItemBlockPtr const& block,
-                                               std::size_t index, std::size_t endIndex)
+                                               std::size_t index, std::size_t)
-    : _block{block}, _rowIndex{index}, _endIndex(endIndex), _finalState{state} {
+    : _block{block}, _rowIndex{index}, _endIndex(_block->size()), _finalState{state} {
-  TRI_ASSERT(index <= endIndex);
+  TRI_ASSERT(index <= _block->size());
-  TRI_ASSERT(endIndex <= block->size());
 }
 
 AqlItemBlockInputRange::AqlItemBlockInputRange(ExecutorState state,
                                                SharedAqlItemBlockPtr&& block,
-                                               std::size_t index, std::size_t endIndex) noexcept
+                                               std::size_t index, std::size_t) noexcept
-    : _block{std::move(block)}, _rowIndex{index}, _endIndex(endIndex), _finalState{state} {
+    : _block{std::move(block)}, _rowIndex{index}, _endIndex(_block->size()), _finalState{state} {
-  TRI_ASSERT(index <= endIndex);
+  TRI_ASSERT(index <= _block->size());
-  TRI_ASSERT(endIndex <= block->size());
 }
 
 std::pair<ExecutorState, InputAqlItemRow> AqlItemBlockInputRange::peek() {
-  if (indexIsValid()) {
+  if (hasMore()) {
     return std::make_pair(nextState<LookAhead::NEXT, RowType::DATA>(),
                           InputAqlItemRow{_block, _rowIndex});
   }
@@ -70,14 +68,8 @@ std::pair<ExecutorState, InputAqlItemRow> AqlItemBlockInputRange::next() {
   return res;
 }
 
-bool AqlItemBlockInputRange::indexIsValid() const noexcept {
+bool AqlItemBlockInputRange::hasMore() const noexcept {
-  return _block != nullptr && _rowIndex < _endIndex;
+  return isIndexValid(_rowIndex) && !isShadowRowAtIndex(_rowIndex);
-}
-
-bool AqlItemBlockInputRange::hasMore() const noexcept { return indexIsValid(); }
-
-bool AqlItemBlockInputRange::hasMoreAfterThis() const noexcept {
-  return indexIsValid() && _rowIndex + 1 < _endIndex;
 }
 
 ExecutorState AqlItemBlockInputRange::state() const noexcept {
@@ -108,20 +100,7 @@ std::pair<ExecutorState, ShadowAqlItemRow> AqlItemBlockInputRange::peekShadowRow
 
 std::pair<ExecutorState, ShadowAqlItemRow> AqlItemBlockInputRange::nextShadowRow() {
   auto res = peekShadowRow();
-  if (hasShadowRow()) {
+  if (res.second.isInitialized()) {
-    auto const& shadowRowIndexes = _block->getShadowRowIndexes();
-    auto it = std::find(shadowRowIndexes.begin(), shadowRowIndexes.end(), _rowIndex);
-    // We have a shadow row in this index, so we cannot be at the end now.
-    TRI_ASSERT(it != shadowRowIndexes.end());
-    // Go to next ShadowRow.
-    it++;
-    if (it == shadowRowIndexes.end()) {
-      // No more shadow row here.
-      _endIndex = _block->size();
-    } else {
-      // Set endIndex to the next ShadowRowIndex.
-      _endIndex = *it;
-    }
     // Advance the current row.
     _rowIndex++;
   }

arangod/Aql/AqlItemBlockInputRange.h

@@ -61,10 +61,6 @@ class AqlItemBlockInputRange {
  private:
   bool isIndexValid(std::size_t index) const noexcept;
 
-  bool indexIsValid() const noexcept;
-
-  bool hasMoreAfterThis() const noexcept;
-
   bool isShadowRowAtIndex(std::size_t index) const noexcept;
 
   enum LookAhead { NOW, NEXT };

arangod/Aql/ExecutionBlockImpl.cpp

@@ -1017,7 +1017,7 @@ ExecutionBlockImpl<FilterExecutor>::executeWithoutTrace(AqlCallStack stack) {
   }
   AqlCall executorRequest;
 
-  while (execState != ExecState::DONE) {
+  while (execState != ExecState::DONE && !_outputItemRow->isFull()) {
     switch (execState) {
       case ExecState::SKIP: {
         auto [state, skippedLocal, call] =
@@ -1028,6 +1028,7 @@ ExecutionBlockImpl<FilterExecutor>::executeWithoutTrace(AqlCallStack stack) {
         if (state == ExecutorState::DONE) {
           execState = ExecState::SHADOWROWS;
         } else if (myCall.getOffset() > 0) {
+          TRI_ASSERT(_upstreamState != ExecutionState::DONE);
           // We need to request more
           executorRequest = call;
           execState = ExecState::UPSTREAM;
@@ -1040,15 +1041,17 @@ ExecutionBlockImpl<FilterExecutor>::executeWithoutTrace(AqlCallStack stack) {
       case ExecState::PRODUCE: {
         auto linesBefore = _outputItemRow->numRowsWritten();
         TRI_ASSERT(myCall.getLimit() > 0);
+        auto limit = (std::min)(myCall.getLimit(), _outputItemRow->numRowsLeft());
         // Execute getSome
         auto const [state, stats, call] =
-            _executor.produceRows(myCall.getLimit(), _lastRange, *_outputItemRow);
+            _executor.produceRows(limit, _lastRange, *_outputItemRow);
         auto written = _outputItemRow->numRowsWritten() - linesBefore;
         _engine->_stats += stats;
         myCall.didProduce(written);
         if (state == ExecutorState::DONE) {
           execState = ExecState::SHADOWROWS;
         } else if (myCall.getLimit() > 0) {
+          TRI_ASSERT(_upstreamState != ExecutionState::DONE);
           // We need to request more
           executorRequest = call;
           execState = ExecState::UPSTREAM;

arangod/Aql/FilterExecutor.cpp

@@ -35,6 +35,8 @@
 #include "Aql/SingleRowFetcher.h"
 #include "Aql/Stats.h"
 
+#include "Logger/LogMacros.h"
+
 #include <utility>
 
 using namespace arangodb;
@@ -143,6 +145,10 @@ std::tuple<ExecutorState, FilterStats, AqlCall> FilterExecutor::produceRows(
   }
 
   AqlCall upstreamCall{};
-  upstreamCall.softLimit = limit;
+  /* We can use this value as a heuristic on overfetching.
+   * by default we do not skip, and do not set any soft or hardLimit
+   * on upstream
+   * upstreamCall.softLimit = limit;
+   */
   return {inputRange.state(), stats, upstreamCall};
 }

tests/Aql/AqlItemBlockInputRangeTest.cpp

@@ -128,6 +128,7 @@ class InputRangeTest : public ::testing::TestWithParam<ExecutorState> {
       ASSERT_NE(rowIndexBefore, testee.getRowIndex())
           << "Did not go to next row.";
     }
+    EXPECT_EQ(expectedState, testee.state());
   }
 
   void validateNextIsShadowRow(AqlItemBlockInputRange& testee, ExecutorState expectedState,