[3.4] Fix end condition (hasMore) for EnumerateViewNode. (#7278)

* Fix end condition (hasMore) for EnumerateViewNode.

* Fix crashes.

* Some more fixes.

* eliminate code duplication

arangod/Aql/CalculationBlock.cpp

@@ -186,6 +186,7 @@ CalculationBlock::getSome(size_t atMost) {
   traceGetSomeBegin(atMost);
 
   if (_done) {
+    traceGetSomeEnd(nullptr, ExecutionState::DONE);
     return {ExecutionState::DONE, nullptr};
   }
 

arangod/IResearch/IResearchViewBlock.cpp

@@ -293,52 +293,30 @@ IResearchViewBlockBase::getSome(size_t atMost) {
     return {ExecutionState::DONE, nullptr};
   }
 
-  bool needMore;
-  AqlItemBlock* cur = nullptr;
-
   ReadContext ctx(getNrInputRegisters());
-
   RegisterId const nrOutRegs = getNrOutputRegisters();
 
   do {
-    do {
+    if (_buffer.empty()) {
-      needMore = false;
+      size_t const toFetch = (std::min)(DefaultBatchSize(), atMost);
 
-      if (_buffer.empty()) {
+      switch (getBlockIfNeeded(toFetch)) {
-        size_t const toFetch = (std::min)(DefaultBatchSize(), atMost);
+       case BufferState::NO_MORE_BLOCKS:
-        auto upstreamRes = ExecutionBlock::getBlock(toFetch);
+        TRI_ASSERT(_inflight == 0);
-        if (upstreamRes.first == ExecutionState::WAITING) {
+        _done = true;
-          traceGetSomeEnd(nullptr, ExecutionState::WAITING);
+        TRI_ASSERT(getHasMoreState() == ExecutionState::DONE);
-          return {upstreamRes.first, nullptr};
+        traceGetSomeEnd(nullptr, ExecutionState::DONE);
-        }
+        return {ExecutionState::DONE, nullptr};
-        _upstreamState = upstreamRes.first;
+       case BufferState::WAITING:
-        if (!upstreamRes.second) {
+        traceGetSomeEnd(nullptr, ExecutionState::WAITING);
-          _done = true;
+        return {ExecutionState::WAITING, nullptr};
-          traceGetSomeEnd(nullptr, ExecutionState::DONE);
+       default:
-          return {ExecutionState::DONE, nullptr};
-        }
-        _pos = 0;  // this is in the first block
         reset();
       }
+    }
 
-      // If we get here, we do have _buffer.front()
+    // If we get here, we do have _buffer.front()
-      cur = _buffer.front();
+    auto* cur = _buffer.front();
-
-      if (!_hasMore) {
-        needMore = true;
-        _hasMore = true;
-
-        if (++_pos >= cur->size()) {
-          _buffer.pop_front();  // does not throw
-          returnBlock(cur);
-          _pos = 0;
-        } else {
-          // we have exhausted this cursor
-          // re-initialize fetching of documents
-          reset();
-        }
-      }
-    } while (needMore);
 
     TRI_ASSERT(cur);
     TRI_ASSERT(ctx.curRegs == cur->getNrRegs());
@@ -352,19 +330,33 @@ IResearchViewBlockBase::getSome(size_t atMost) {
 
     throwIfKilled();  // check if we were aborted
 
-    TRI_IF_FAILURE("EnumerateViewBlock::moreDocuments") {
+    TRI_IF_FAILURE("IResearchViewBlockBase::getSome") {
       THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
     }
 
     _hasMore = next(ctx, atMost);
 
+    if (!_hasMore) {
+      _hasMore = true;
+
+      if (++_pos >= cur->size()) {
+        _buffer.pop_front();  // does not throw
+        returnBlock(cur);
+        _pos = 0;
+      } else {
+        // we have exhausted this cursor
+        // re-initialize fetching of documents
+        reset();
+      }
+    }
+
     // If the collection is actually empty we cannot forward an empty block
   } while (ctx.pos == 0);
 
   TRI_ASSERT(ctx.res);
 
   // aggregate stats
-   _engine->_stats.scannedIndex += static_cast<int64_t>(ctx.pos);
+  _engine->_stats.scannedIndex += static_cast<int64_t>(ctx.pos);
 
   if (ctx.pos < atMost) {
     // The collection did not have enough results

tests/js/server/aql/aql-profiler-noncluster.js

@@ -323,9 +323,15 @@ function ahuacatlProfilerTestSuite () {
       const query = `FOR d IN @@view SEARCH d.value != 0 OPTIONS { waitForSync: true } RETURN d.value`;
 
       const genNodeList = (rows, batches) => {
+        // EnumerateViewBlock returns HASMORE when asked for the exact number
+        // of items it has left. This could be improved.
+        const optimalBatches = Math.ceil(rows / defaultBatchSize);
+        const maxViewBatches = Math.floor(rows / defaultBatchSize) + 1;
+        const viewBatches = [optimalBatches, maxViewBatches];
+
         return [
-          {type: SingletonBlock, calls: 2, items: 1},
+          {type: SingletonBlock, calls: 1, items: 1},
-          {type: EnumerateViewNode, calls: batches + 1, items: rows},
+          {type: EnumerateViewNode, calls: viewBatches, items: rows},
           {type: CalculationBlock, calls: rows % defaultBatchSize === 0 ? batches + 1 : batches, items: rows},
           {type: ReturnBlock, calls: rows % defaultBatchSize === 0 ? batches + 1 : batches, items: rows}
         ];
@@ -350,9 +356,15 @@ function ahuacatlProfilerTestSuite () {
       const query = `FOR d IN @@view SEARCH d.value != 0 OPTIONS { waitForSync: true } SORT d.value DESC RETURN d.value`;
 
       const genNodeList = (rows, batches) => {
+        // EnumerateViewBlock returns HASMORE when asked for the exact number
+        // of items it has left. This could be improved.
+        const optimalBatches = Math.ceil(rows / defaultBatchSize);
+        const maxViewBatches = Math.floor(rows / defaultBatchSize) + 1;
+        const viewBatches = [optimalBatches, maxViewBatches];
+
         return [
-          {type: SingletonBlock, calls: 2, items: 1},
+          {type: SingletonBlock, calls: 1, items: 1},
-          {type: EnumerateViewNode, calls: batches + 1, items: rows},
+          {type: EnumerateViewNode, calls: viewBatches, items: rows},
           {type: CalculationBlock, calls: rows % defaultBatchSize === 0 ? batches + 1 : batches, items: rows},
           {type: SortBlock, calls: batches, items: rows},
           {type: ReturnBlock, calls: batches, items: rows}
@@ -378,9 +390,15 @@ function ahuacatlProfilerTestSuite () {
       const query = `FOR d IN @@view SEARCH d.value != 0 OPTIONS { waitForSync: true } SORT TFIDF(d) ASC, BM25(d) RETURN d.value`;
 
       const genNodeList = (rows, batches) => {
+        // EnumerateViewBlock returns HASMORE when asked for the exact number
+        // of items it has left. This could be improved.
+        const optimalBatches = Math.ceil(rows / defaultBatchSize);
+        const maxViewBatches = Math.floor(rows / defaultBatchSize) + 1;
+        const viewBatches = [optimalBatches, maxViewBatches];
+
         return [
-          {type: SingletonBlock, calls: 2, items: 1},
+          {type: SingletonBlock, calls: 1, items: 1},
-          {type: EnumerateViewNode, calls: batches + 1, items: rows},
+          {type: EnumerateViewNode, calls: viewBatches, items: rows},
           {type: SortBlock, calls: batches, items: rows},
           {type: CalculationBlock, calls: batches, items: rows},
           {type: ReturnBlock, calls: batches, items: rows}