Splicing gather cleanup (#10479)

Some cleanup and deprecation information

CMakeLists.txt

@@ -896,7 +896,7 @@ if (CMAKE_COMPILER_IS_GNUCC)
     message(STATUS "Compiler type GNU: ${CMAKE_CXX_COMPILER}")
   endif ()
 
-  set(BASE_FLAGS "-Wall -Wextra -Wno-unused-parameter ${BASE_FLAGS}")
+  set(BASE_FLAGS "-Wall -Wextra -Wno-unused-parameter -Wno-deprecated-declarations ${BASE_FLAGS}")
 
   set(CMAKE_C_FLAGS                "-g"                             CACHE INTERNAL "default C compiler flags")
   set(CMAKE_C_FLAGS_DEBUG          "-O0 -g -D_DEBUG=1"              CACHE INTERNAL "C debug flags")
@@ -915,7 +915,7 @@ elseif (CMAKE_COMPILER_IS_CLANG)
     message(STATUS "Compiler type CLANG: ${CMAKE_CXX_COMPILER}")
   endif ()
 
-  set(BASE_FLAGS "-Wall -Wextra -Wno-unused-parameter ${BASE_FLAGS}")
+  set(BASE_FLAGS "-Wall -Wextra -Wno-unused-parameter -Wno-deprecated-declarations ${BASE_FLAGS}")
 
   set(CMAKE_C_FLAGS                "-g"                             CACHE INTERNAL "default C compiler flags")
   set(CMAKE_C_FLAGS_DEBUG          "-O0 -g -D_DEBUG=1"              CACHE INTERNAL "C debug flags")
@@ -934,6 +934,8 @@ elseif (MSVC)
     message(STATUS "Compiler type MSVC: ${CMAKE_CXX_COMPILER}")
   endif ()
 
+  set(BASE_FLAGS "/wd4996 ${BASE_FLAGS}")
+
   set(CMAKE_C_FLAGS                "/MTd"                              CACHE INTERNAL "default C++ compiler flags")
   set(CMAKE_C_FLAGS_DEBUG          "/D _DEBUG /MTd /Zi /Ob0 /Od /RTC1 /bigobj" CACHE INTERNAL "C++ debug flags")
   set(CMAKE_C_FLAGS_MINSIZEREL     "/MT /O1 /Ob1"                      CACHE INTERNAL "C++ minimal size flags")

arangod/Aql/AllRowsFetcher.h

@@ -57,8 +57,6 @@ class ShadowAqlItemRow;
  *     => If all rows have been Fetched, it will return DONE and an AqlItemMatrix, the Matrix will return results
  *     => Any later call will return DONE and a nullptr. So make sure you keep the Matrix.
  *     => This state can be left only if the shadowRow is fetched.
- *   - fetchBlockForPassthrough()
- *     => Cannot be used! Only required to make the code compile
  *   - preFetchNumberOfRows()
  *     => Will do the same as fetchAllRows, but NOT give out the data, it will only hold it internally.
  *     => On response it will inform the caller on exactly how many Rows will be returned until the next ShadowRow appears.

arangod/Aql/AqlValue.h

@@ -331,11 +331,13 @@ struct AqlValue final {
 
   /// @brief materializes a value into the builder
   void toVelocyPack(velocypack::Options const*, arangodb::velocypack::Builder&, bool resolveExternals) const;
+  [[deprecated("Pass VPackOptions instead of the transaction")]]
   void toVelocyPack(transaction::Methods*, arangodb::velocypack::Builder&, bool resolveExternals) const;
 
   /// @brief materialize a value into a new one. this expands docvecs and
   /// ranges
   AqlValue materialize(velocypack::Options const*, bool& hasCopied, bool resolveExternals) const;
+  [[deprecated("Pass VPackOptions instead of the transaction")]]
   AqlValue materialize(transaction::Methods*, bool& hasCopied, bool resolveExternals) const;
 
   /// @brief return the slice for the value
@@ -368,6 +370,7 @@ struct AqlValue final {
   /// @brief compare function for two values
   static int Compare(velocypack::Options const*, AqlValue const& left,
                      AqlValue const& right, bool useUtf8);
+  [[deprecated("Pass VPackOptions instead of the transaction")]]
   static int Compare(transaction::Methods*, AqlValue const& left,
                      AqlValue const& right, bool useUtf8);
 

arangod/Aql/AqlValueMaterializer.h

@@ -43,6 +43,7 @@ namespace aql {
 /// @brief Helper class to materialize AqlValues (see AqlValue::materialize).
 struct AqlValueMaterializer {
   explicit AqlValueMaterializer(velocypack::Options const* options);
+  [[deprecated("Pass VPackOptions instead of the transaction")]]
   explicit AqlValueMaterializer(arangodb::transaction::Methods* trx);
 
   AqlValueMaterializer(AqlValueMaterializer const& other);

arangod/Aql/CalculationExecutor.cpp

@@ -36,13 +36,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-template <CalculationType calculationType>
-constexpr bool CalculationExecutor<calculationType>::Properties::preservesOrder;
-template <CalculationType calculationType>
-constexpr BlockPassthrough CalculationExecutor<calculationType>::Properties::allowsBlockPassthrough;
-template <CalculationType calculationType>
-constexpr bool CalculationExecutor<calculationType>::Properties::inputSizeRestrictsOutputSize;
-
 CalculationExecutorInfos::CalculationExecutorInfos(
     RegisterId outputRegister, RegisterId nrInputRegisters,
     RegisterId nrOutputRegisters, std::unordered_set<RegisterId> registersToClear,

arangod/Aql/ClusterNodes.cpp

@@ -42,6 +42,7 @@
 #include "Aql/IndexNode.h"
 #include "Aql/ModificationNodes.h"
 #include "Aql/MultiDependencySingleRowFetcher.h"
+#include "Aql/ParallelUnsortedGatherExecutor.h"
 #include "Aql/Query.h"
 #include "Aql/RemoteExecutor.h"
 #include "Aql/ScatterExecutor.h"
@@ -49,7 +50,6 @@
 #include "Aql/SortRegister.h"
 #include "Aql/SortingGatherExecutor.h"
 #include "Aql/UnsortedGatherExecutor.h"
-#include "Aql/UnsortingGatherExecutor.h"
 #include "Aql/types.h"
 #include "Basics/VelocyPackHelper.h"
 #include "Cluster/ServerState.h"
@@ -494,14 +494,15 @@ std::unique_ptr<ExecutionBlock> GatherNode::createBlock(
     TRI_ASSERT(getRegisterPlan()->nrRegs[previousNode->getDepth()] ==
                getRegisterPlan()->nrRegs[getDepth()]);
     if (ServerState::instance()->isCoordinator() && _parallelism == Parallelism::Parallel) {
-      UnsortedGatherExecutorInfos infos(getRegisterPlan()->nrRegs[getDepth()],
+      ParallelUnsortedGatherExecutorInfos infos(getRegisterPlan()->nrRegs[getDepth()],
-                                        calcRegsToKeep(), getRegsToClear());
+                                                calcRegsToKeep(), getRegsToClear());
-      return std::make_unique<ExecutionBlockImpl<UnsortedGatherExecutor>>(&engine, this, std::move(infos));
+      return std::make_unique<ExecutionBlockImpl<ParallelUnsortedGatherExecutor>>(
+          &engine, this, std::move(infos));
     } else {
       IdExecutorInfos infos(getRegisterPlan()->nrRegs[getDepth()],
                             calcRegsToKeep(), getRegsToClear());
 
-      return std::make_unique<ExecutionBlockImpl<UnsortingGatherExecutor>>(&engine, this,
+      return std::make_unique<ExecutionBlockImpl<UnsortedGatherExecutor>>(&engine, this,
                                                                            std::move(infos));
     }
   }

arangod/Aql/ConstrainedSortExecutor.cpp

@@ -36,10 +36,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool ConstrainedSortExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough ConstrainedSortExecutor::Properties::allowsBlockPassthrough;
-constexpr bool ConstrainedSortExecutor::Properties::inputSizeRestrictsOutputSize;
-
 namespace {
 
 void eraseRow(SharedAqlItemBlockPtr& block, size_t row) {
@@ -55,7 +51,7 @@ void eraseRow(SharedAqlItemBlockPtr& block, size_t row) {
 class arangodb::aql::ConstrainedLessThan {
  public:
   ConstrainedLessThan(velocypack::Options const* options,
-                      std::vector<arangodb::aql::SortRegister>& sortRegisters) noexcept
+                      std::vector<arangodb::aql::SortRegister> const& sortRegisters) noexcept
       : _vpackOptions(options), _heapBuffer(nullptr), _sortRegisters(sortRegisters) {}
 
   void setBuffer(arangodb::aql::AqlItemBlock* heap) { _heapBuffer = heap; }
@@ -82,7 +78,7 @@ class arangodb::aql::ConstrainedLessThan {
  private:
   velocypack::Options const* const _vpackOptions;
   arangodb::aql::AqlItemBlock* _heapBuffer;
-  std::vector<arangodb::aql::SortRegister>& _sortRegisters;
+  std::vector<arangodb::aql::SortRegister> const& _sortRegisters;
 };  // ConstrainedLessThan
 
 arangodb::Result ConstrainedSortExecutor::pushRow(InputAqlItemRow& input) {
@@ -92,16 +88,16 @@ arangodb::Result ConstrainedSortExecutor::pushRow(InputAqlItemRow& input) {
 
   size_t dRow = _rowsPushed;
 
-  if (dRow >= _infos._limit) {
+  if (dRow >= _infos.limit()) {
     // pop an entry first
-    std::pop_heap(_rows.begin(), _rows.end(), *_cmpHeap.get());
+    std::pop_heap(_rows.begin(), _rows.end(), *_cmpHeap);
     dRow = _rows.back();
     eraseRow(_heapBuffer, dRow);
   } else {
     _rows.emplace_back(dRow);  // add to heap vector
   }
 
-  TRI_ASSERT(dRow < _infos._limit);
+  TRI_ASSERT(dRow < _infos.limit());
   TRI_IF_FAILURE("SortBlock::doSortingInner") {
     THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
   }
@@ -113,7 +109,7 @@ arangodb::Result ConstrainedSortExecutor::pushRow(InputAqlItemRow& input) {
   ++_rowsPushed;
 
   // now restore heap condition
-  std::push_heap(_rows.begin(), _rows.end(), *_cmpHeap.get());
+  std::push_heap(_rows.begin(), _rows.end(), *_cmpHeap);
 
   return TRI_ERROR_NO_ERROR;
 }
@@ -142,15 +138,15 @@ ConstrainedSortExecutor::ConstrainedSortExecutor(Fetcher& fetcher, SortExecutorI
       _rowsPushed(0),
       _rowsRead(0),
       _skippedAfter(0),
-      _heapBuffer(_infos._manager.requestBlock(_infos._limit,
+      _heapBuffer(_infos.itemBlockManager().requestBlock(_infos.limit(),
-                                               _infos.numberOfOutputRegisters())),
+                                                         _infos.numberOfOutputRegisters())),
       _cmpHeap(std::make_unique<ConstrainedLessThan>(_infos.vpackOptions(),
                                                      _infos.sortRegisters())),
       _heapOutputRow{_heapBuffer, make_shared_unordered_set(),
                      make_shared_unordered_set(_infos.numberOfOutputRegisters()),
                      _infos.registersToClear()} {
-  TRI_ASSERT(_infos._limit > 0);
+  TRI_ASSERT(_infos.limit() > 0);
-  _rows.reserve(infos._limit);
+  _rows.reserve(infos.limit());
   _cmpHeap->setBuffer(_heapBuffer.get());
 }
 
@@ -184,7 +180,7 @@ ExecutionState ConstrainedSortExecutor::consumeInput() {
       TRI_ASSERT(_state == ExecutionState::DONE);
     } else {
       ++_rowsRead;
-      if (_rowsPushed < _infos._limit || !compareInput(_rows.front(), input)) {
+      if (_rowsPushed < _infos.limit() || !compareInput(_rows.front(), input)) {
         // Push this row into the heap
         pushRow(input);
       }
@@ -255,7 +251,7 @@ std::pair<ExecutionState, size_t> ConstrainedSortExecutor::expectedNumberOfRows(
       return {state, 0};
     }
     // Return the minimum of upstream + limit
-    rowsLeft = (std::min)(expectedRows, _infos._limit);
+    rowsLeft = (std::min)(expectedRows, _infos.limit());
   } else {
     // We have exactly the following rows available:
     rowsLeft = _rows.size() - _returnNext;

arangod/Aql/CountCollectExecutor.cpp

@@ -37,10 +37,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool CountCollectExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough CountCollectExecutor::Properties::allowsBlockPassthrough;
-constexpr bool CountCollectExecutor::Properties::inputSizeRestrictsOutputSize;
-
 CountCollectExecutorInfos::CountCollectExecutorInfos(
     RegisterId collectRegister, RegisterId nrInputRegisters, RegisterId nrOutputRegisters,
     // cppcheck-suppress passedByValue

arangod/Aql/DistinctCollectExecutor.cpp

@@ -38,10 +38,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool DistinctCollectExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough DistinctCollectExecutor::Properties::allowsBlockPassthrough;
-constexpr bool DistinctCollectExecutor::Properties::inputSizeRestrictsOutputSize;
-
 DistinctCollectExecutorInfos::DistinctCollectExecutorInfos(
     RegisterId nrInputRegisters, RegisterId nrOutputRegisters,
     std::unordered_set<RegisterId> registersToClear,

arangod/Aql/DistributeConsumerNode.cpp

@@ -65,7 +65,7 @@ std::unique_ptr<ExecutionBlock> DistributeConsumerNode::createBlock(
              getRegisterPlan()->nrRegs[getDepth()]);
   IdExecutorInfos infos(getRegisterPlan()->nrRegs[getDepth()], calcRegsToKeep(),
                         getRegsToClear(), _distributeId, _isResponsibleForInitializeCursor);
-  return std::make_unique<ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, SingleRowFetcher<BlockPassthrough::Enable>>>>(
+  return std::make_unique<ExecutionBlockImpl<IdExecutor<SingleRowFetcher<BlockPassthrough::Enable>>>>(
       &engine, this, std::move(infos));
 }
 
@@ -75,11 +75,11 @@ void DistributeConsumerNode::cloneRegisterPlan(ScatterNode* dependency) {
   _registerPlan = dependency->getRegisterPlan();
   _depth = dependency->getDepth();
   {
-    auto later = dependency->getVarsUsedLater();
+    auto const& later = dependency->getVarsUsedLater();
     setVarsUsedLater(later);
   }
   {
-    auto valid = dependency->getVarsValid();
+    auto const& valid = dependency->getVarsValid();
     setVarsValid(valid);
   }
   setVarUsageValid();

arangod/Aql/DistributeConsumerNode.h

@@ -27,17 +27,19 @@
 #include "Basics/Exceptions.h"
 #include "Basics/voc-errors.h"
 
-namespace arangodb {
+#include <utility>
-namespace aql {
+
+namespace arangodb::aql {
+
 class ExecutionBlock;
 class ExecutionPlan;
 class ScatterNode;
 
 class DistributeConsumerNode : public ExecutionNode {
  public:
-  DistributeConsumerNode(ExecutionPlan* plan, size_t id, std::string const& distributeId)
+  DistributeConsumerNode(ExecutionPlan* plan, size_t id, std::string distributeId)
       : ExecutionNode(plan, id),
-        _distributeId(distributeId),
+        _distributeId(std::move(distributeId)),
         _isResponsibleForInitializeCursor(true) {}
 
   DistributeConsumerNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& base);
@@ -106,7 +108,6 @@ class DistributeConsumerNode : public ExecutionNode {
   bool _isResponsibleForInitializeCursor;
 };
 
-}  // namespace aql
+}  // namespace arangodb::aql
-}  // namespace arangodb
 
 #endif

arangod/Aql/EnumerateCollectionExecutor.cpp

@@ -43,10 +43,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool EnumerateCollectionExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough EnumerateCollectionExecutor::Properties::allowsBlockPassthrough;
-constexpr bool EnumerateCollectionExecutor::Properties::inputSizeRestrictsOutputSize;
-
 EnumerateCollectionExecutorInfos::EnumerateCollectionExecutorInfos(
     RegisterId outputRegister, RegisterId nrInputRegisters, RegisterId nrOutputRegisters,
     // cppcheck-suppress passedByValue

arangod/Aql/EnumerateListExecutor.cpp

@@ -47,10 +47,6 @@ void throwArrayExpectedException(AqlValue const& value) {
 }
 } // namespace
 
-constexpr bool EnumerateListExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough EnumerateListExecutor::Properties::allowsBlockPassthrough;
-constexpr bool EnumerateListExecutor::Properties::inputSizeRestrictsOutputSize;
-
 EnumerateListExecutorInfos::EnumerateListExecutorInfos(
     RegisterId inputRegister, RegisterId outputRegister,
     RegisterId nrInputRegisters, RegisterId nrOutputRegisters,

arangod/Aql/ExecutionBlock.h

@@ -55,7 +55,7 @@ class ExecutionBlock {
 
  public:
   /// @brief batch size value
-  static constexpr inline size_t DefaultBatchSize() { return 1000; }
+  [[nodiscard]] static constexpr inline size_t DefaultBatchSize() { return 1000; }
 
   /// @brief Number to use when we skip all. Should really be inf, but don't
   /// use something near std::numeric_limits<size_t>::max() to avoid overflows
@@ -63,7 +63,7 @@ class ExecutionBlock {
   /// This is used as an argument for skipSome(), e.g. when counting everything.
   /// Setting this to any other value >0 does not (and must not) affect the
   /// results. It's only to reduce the number of necessary skipSome calls.
-  static constexpr inline size_t SkipAllSize() { return 1000000000; }
+  [[nodiscard]] static constexpr inline size_t SkipAllSize() { return 1000000000; }
 
   /// @brief Methods for execution
   /// Lifecycle is:
@@ -78,10 +78,10 @@ class ExecutionBlock {
   ///    DESTRUCTOR
 
   /// @brief initializeCursor, could be called multiple times
-  virtual std::pair<ExecutionState, Result> initializeCursor(InputAqlItemRow const& input);
+  [[nodiscard]] virtual std::pair<ExecutionState, Result> initializeCursor(InputAqlItemRow const& input);
 
   /// @brief shutdown, will be called exactly once for the whole query
-  virtual std::pair<ExecutionState, Result> shutdown(int errorCode);
+  [[nodiscard]] virtual std::pair<ExecutionState, Result> shutdown(int errorCode);
 
   /// @brief getSome, gets some more items, semantic is as follows: not
   /// more than atMost items may be delivered. The method tries to
@@ -90,39 +90,39 @@ class ExecutionBlock {
   /// if it returns an actual block, it must contain at least one item.
   /// getSome() also takes care of tracing and clearing registers; don't do it
   /// in getOrSkipSome() implementations.
-  virtual std::pair<ExecutionState, SharedAqlItemBlockPtr> getSome(size_t atMost) = 0;
+  [[nodiscard]] virtual std::pair<ExecutionState, SharedAqlItemBlockPtr> getSome(size_t atMost) = 0;
 
   // Trace the start of a getSome call
   void traceGetSomeBegin(size_t atMost);
 
   // Trace the end of a getSome call, potentially with result
-  std::pair<ExecutionState, SharedAqlItemBlockPtr> traceGetSomeEnd(
+  [[nodiscard]] std::pair<ExecutionState, SharedAqlItemBlockPtr> traceGetSomeEnd(
       ExecutionState state, SharedAqlItemBlockPtr result);
 
   void traceSkipSomeBegin(size_t atMost);
 
-  std::pair<ExecutionState, size_t> traceSkipSomeEnd(std::pair<ExecutionState, size_t> res);
+  [[nodiscard]] std::pair<ExecutionState, size_t> traceSkipSomeEnd(std::pair<ExecutionState, size_t> res);
 
-  std::pair<ExecutionState, size_t> traceSkipSomeEnd(ExecutionState state, size_t skipped);
+  [[nodiscard]] std::pair<ExecutionState, size_t> traceSkipSomeEnd(ExecutionState state, size_t skipped);
 
   /// @brief skipSome, skips some more items, semantic is as follows: not
   /// more than atMost items may be skipped. The method tries to
   /// skip a block of at most atMost items, however, it may skip
   /// less (for example if there are not enough items to come). The number of
   /// elements skipped is returned.
-  virtual std::pair<ExecutionState, size_t> skipSome(size_t atMost) = 0;
+  [[nodiscard]] virtual std::pair<ExecutionState, size_t> skipSome(size_t atMost) = 0;
 
-  ExecutionState getHasMoreState();
+  [[nodiscard]] ExecutionState getHasMoreState();
 
   // TODO: Can we get rid of this? Problem: Subquery Executor is using it.
-  ExecutionNode const* getPlanNode() const;
+  [[nodiscard]] ExecutionNode const* getPlanNode() const;
 
   [[nodiscard]] velocypack::Options const* trxVpackOptions() const noexcept;
 
   /// @brief add a dependency
   void addDependency(ExecutionBlock* ep);
 
-  bool isInSplicedSubquery() const noexcept;
+  [[nodiscard]] bool isInSplicedSubquery() const noexcept;
 
  protected:
   /// @brief the execution engine

arangod/Aql/ExecutionBlockImpl.cpp

@@ -49,6 +49,7 @@
 #include "Aql/ModificationExecutor.h"
 #include "Aql/MultiDependencySingleRowFetcher.h"
 #include "Aql/NoResultsExecutor.h"
+#include "Aql/ParallelUnsortedGatherExecutor.h"
 #include "Aql/Query.h"
 #include "Aql/QueryOptions.h"
 #include "Aql/ReturnExecutor.h"
@@ -64,7 +65,6 @@
 #include "Aql/SubqueryStartExecutor.h"
 #include "Aql/TraversalExecutor.h"
 #include "Aql/UnsortedGatherExecutor.h"
-#include "Aql/UnsortingGatherExecutor.h"
 
 #include "Aql/SimpleModifier.h"
 #include "Aql/UpsertModifier.h"
@@ -375,8 +375,8 @@ static SkipVariants constexpr skipType() {
            std::is_same<Executor, LimitExecutor>::value ||
            std::is_same<Executor, ConstrainedSortExecutor>::value ||
            std::is_same<Executor, SortingGatherExecutor>::value ||
-           std::is_same<Executor, UnsortingGatherExecutor>::value ||
            std::is_same<Executor, UnsortedGatherExecutor>::value ||
+           std::is_same<Executor, ParallelUnsortedGatherExecutor>::value ||
            std::is_same<Executor, MaterializeExecutor<RegisterId>>::value ||
            std::is_same<Executor, MaterializeExecutor<std::string const&>>::value),
       "Unexpected executor for SkipVariants::EXECUTOR");
@@ -521,7 +521,7 @@ std::pair<ExecutionState, Result> ExecutionBlockImpl<Executor>::shutdown(int err
 namespace arangodb::aql {
 // TODO -- remove this specialization when cpp 17 becomes available
 template <>
-std::pair<ExecutionState, Result> ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, ConstFetcher>>::initializeCursor(
+std::pair<ExecutionState, Result> ExecutionBlockImpl<IdExecutor<ConstFetcher>>::initializeCursor(
     InputAqlItemRow const& input) {
   // reinitialize the DependencyProxy
   _dependencyProxy.reset();
@@ -632,12 +632,13 @@ std::pair<ExecutionState, Result> ExecutionBlockImpl<SubqueryExecutor<false>>::s
 
 template <>
 std::pair<ExecutionState, Result> ExecutionBlockImpl<
-    IdExecutor<BlockPassthrough::Enable, SingleRowFetcher<BlockPassthrough::Enable>>>::shutdown(int errorCode) {
+    IdExecutor<SingleRowFetcher<BlockPassthrough::Enable>>>::shutdown(int errorCode) {
   if (this->infos().isResponsibleForInitializeCursor()) {
     return ExecutionBlock::shutdown(errorCode);
   }
   return {ExecutionState::DONE, {errorCode}};
 }
+
 }  // namespace arangodb::aql
 
 namespace arangodb::aql {
@@ -863,7 +864,6 @@ template class ::arangodb::aql::ExecutionBlockImpl<EnumerateCollectionExecutor>;
 template class ::arangodb::aql::ExecutionBlockImpl<EnumerateListExecutor>;
 template class ::arangodb::aql::ExecutionBlockImpl<FilterExecutor>;
 template class ::arangodb::aql::ExecutionBlockImpl<HashedCollectExecutor>;
-
 template class ::arangodb::aql::ExecutionBlockImpl<IResearchViewExecutor<false, arangodb::iresearch::MaterializeType::Materialized>>;
 template class ::arangodb::aql::ExecutionBlockImpl<IResearchViewExecutor<false, arangodb::iresearch::MaterializeType::LateMaterialized>>;
 template class ::arangodb::aql::ExecutionBlockImpl<IResearchViewExecutor<false, arangodb::iresearch::MaterializeType::LateMaterializedWithVars>>;
@@ -875,9 +875,8 @@ template class ::arangodb::aql::ExecutionBlockImpl<IResearchViewMergeExecutor<fa
 template class ::arangodb::aql::ExecutionBlockImpl<IResearchViewMergeExecutor<true, arangodb::iresearch::MaterializeType::Materialized>>;
 template class ::arangodb::aql::ExecutionBlockImpl<IResearchViewMergeExecutor<true, arangodb::iresearch::MaterializeType::LateMaterialized>>;
 
-template class ::arangodb::aql::ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, ConstFetcher>>;
+template class ::arangodb::aql::ExecutionBlockImpl<IdExecutor<ConstFetcher>>;
-template class ::arangodb::aql::ExecutionBlockImpl<
+template class ::arangodb::aql::ExecutionBlockImpl<IdExecutor<SingleRowFetcher<BlockPassthrough::Enable>>>;
-    IdExecutor<BlockPassthrough::Enable, SingleRowFetcher<BlockPassthrough::Enable>>>;
 template class ::arangodb::aql::ExecutionBlockImpl<IndexExecutor>;
 template class ::arangodb::aql::ExecutionBlockImpl<LimitExecutor>;
 
@@ -901,8 +900,8 @@ template class ::arangodb::aql::ExecutionBlockImpl<SubqueryExecutor<false>>;
 template class ::arangodb::aql::ExecutionBlockImpl<SubqueryStartExecutor>;
 template class ::arangodb::aql::ExecutionBlockImpl<TraversalExecutor>;
 template class ::arangodb::aql::ExecutionBlockImpl<SortingGatherExecutor>;
+template class ::arangodb::aql::ExecutionBlockImpl<ParallelUnsortedGatherExecutor>;
 template class ::arangodb::aql::ExecutionBlockImpl<UnsortedGatherExecutor>;
-template class ::arangodb::aql::ExecutionBlockImpl<UnsortingGatherExecutor>;
 
 template class ::arangodb::aql::ExecutionBlockImpl<MaterializeExecutor<RegisterId>>;
 template class ::arangodb::aql::ExecutionBlockImpl<MaterializeExecutor<std::string const&>>;

arangod/Aql/ExecutionBlockImpl.h

@@ -170,7 +170,7 @@ class ExecutionBlockImpl final : public ExecutionBlock {
 
   [[nodiscard]] std::pair<ExecutionState, Result> initializeCursor(InputAqlItemRow const& input) override;
 
-  Infos const& infos() const;
+  [[nodiscard]] Infos const& infos() const;
 
   /// @brief shutdown, will be called exactly once for the whole query
   /// Special implementation for all Executors that need to implement Shutdown

arangod/Aql/ExecutionEngine.cpp

@@ -712,8 +712,7 @@ ExecutionEngine* ExecutionEngine::instantiateFromPlan(QueryRegistry& queryRegist
 
     bool const returnInheritedResults = !arangodb::ServerState::isDBServer(role);
     if (returnInheritedResults) {
-      auto returnNode =
+      auto returnNode = dynamic_cast<ExecutionBlockImpl<IdExecutor<void>>*>(root);
-          dynamic_cast<ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>>*>(root);
       TRI_ASSERT(returnNode != nullptr);
       engine->resultRegister(returnNode->getOutputRegisterId());
     } else {

arangod/Aql/ExecutionNode.cpp

@@ -1204,7 +1204,7 @@ const ::arangodb::containers::HashSet<const Variable*>& ExecutionNode::getVarsUs
   return _varsUsedLater;
 }
 
-void ExecutionNode::setVarsValid(::arangodb::containers::HashSet<const Variable*>& v) {
+void ExecutionNode::setVarsValid(::arangodb::containers::HashSet<const Variable*> const& v) {
   _varsValid = v;
 }
 
@@ -1283,8 +1283,8 @@ std::unique_ptr<ExecutionBlock> SingletonNode::createBlock(
 
   IdExecutorInfos infos(nrRegs, std::move(toKeep), getRegsToClear());
 
-  return std::make_unique<ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, ConstFetcher>>>(
+  return std::make_unique<ExecutionBlockImpl<IdExecutor<ConstFetcher>>>(&engine, this,
-      &engine, this, std::move(infos));
+                                                                        std::move(infos));
 }
 
 /// @brief toVelocyPack, for SingletonNode
@@ -2170,8 +2170,8 @@ std::unique_ptr<ExecutionBlock> ReturnNode::createBlock(
       returnInheritedResults ? getRegisterPlan()->nrRegs[getDepth()] : 1;
 
   if (returnInheritedResults) {
-    return std::make_unique<ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>>>(
+    return std::make_unique<ExecutionBlockImpl<IdExecutor<void>>>(&engine, this,
-        &engine, this, inputRegister, _count);
+                                                                  inputRegister, _count);
   } else {
     TRI_ASSERT(!returnInheritedResults);
     ReturnExecutorInfos infos(inputRegister, numberInputRegisters,

arangod/Aql/ExecutionNode.h

@@ -363,7 +363,7 @@ class ExecutionNode {
   ::arangodb::containers::HashSet<Variable const*> const& getVarsUsedLater() const;
 
   /// @brief setVarsValid
-  void setVarsValid(::arangodb::containers::HashSet<Variable const*>& v);
+  void setVarsValid(::arangodb::containers::HashSet<Variable const*> const& v);
 
   /// @brief getVarsValid, this returns the set of variables that is valid
   /// for items leaving this node, this includes those that will be set here

arangod/Aql/FilterExecutor.cpp

@@ -37,10 +37,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool FilterExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough FilterExecutor::Properties::allowsBlockPassthrough;
-constexpr bool FilterExecutor::Properties::inputSizeRestrictsOutputSize;
-
 FilterExecutorInfos::FilterExecutorInfos(RegisterId inputRegister, RegisterId nrInputRegisters,
                                          RegisterId nrOutputRegisters,
                                          // cppcheck-suppress passedByValue

arangod/Aql/HashedCollectExecutor.cpp

@@ -39,10 +39,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool HashedCollectExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough HashedCollectExecutor::Properties::allowsBlockPassthrough;
-constexpr bool HashedCollectExecutor::Properties::inputSizeRestrictsOutputSize;
-
 static const AqlValue EmptyValue;
 
 HashedCollectExecutorInfos::HashedCollectExecutorInfos(

arangod/Aql/IResearchViewExecutor.cpp

@@ -54,14 +54,6 @@
 
 using namespace arangodb;
 using namespace arangodb::aql;
-
-template <typename Impl, typename Traits>
-constexpr bool IResearchViewExecutorBase<Impl, Traits>::Properties::preservesOrder;
-template <typename Impl, typename Traits>
-constexpr BlockPassthrough IResearchViewExecutorBase<Impl, Traits>::Properties::allowsBlockPassthrough;
-template <typename Impl, typename Traits>
-constexpr bool IResearchViewExecutorBase<Impl, Traits>::Properties::inputSizeRestrictsOutputSize;
-
 using namespace arangodb::iresearch;
 
 namespace {

arangod/Aql/IdExecutor.cpp

@@ -37,14 +37,7 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-template <BlockPassthrough usePassThrough, class T>
+ExecutionBlockImpl<IdExecutor<void>>::ExecutionBlockImpl(ExecutionEngine* engine,
-constexpr bool IdExecutor<usePassThrough, T>::Properties::preservesOrder;
-template <BlockPassthrough usePassThrough, class T>
-constexpr BlockPassthrough IdExecutor<usePassThrough, T>::Properties::allowsBlockPassthrough;
-template <BlockPassthrough usePassThrough, class T>
-constexpr bool IdExecutor<usePassThrough, T>::Properties::inputSizeRestrictsOutputSize;
-
-ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>>::ExecutionBlockImpl(ExecutionEngine* engine,
                                                          ExecutionNode const* node,
                                                          RegisterId outputRegister, bool doCount)
     : ExecutionBlock(engine, node),
@@ -57,7 +50,7 @@ ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>>::ExecutionBlockIm
   }
 }
 
-std::pair<ExecutionState, size_t> ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>>::skipSome(size_t atMost) {
+std::pair<ExecutionState, size_t> ExecutionBlockImpl<IdExecutor<void>>::skipSome(size_t atMost) {
   traceSkipSomeBegin(atMost);
   if (isDone()) {
     return traceSkipSomeEnd(ExecutionState::DONE, 0);
@@ -74,7 +67,7 @@ std::pair<ExecutionState, size_t> ExecutionBlockImpl<IdExecutor<BlockPassthrough
   return traceSkipSomeEnd(state, skipped);
 }
 
-std::pair<ExecutionState, SharedAqlItemBlockPtr> ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>>::getSome(size_t atMost) {
+std::pair<ExecutionState, SharedAqlItemBlockPtr> ExecutionBlockImpl<IdExecutor<void>>::getSome(size_t atMost) {
   traceGetSomeBegin(atMost);
   if (isDone()) {
     return traceGetSomeEnd(ExecutionState::DONE, nullptr);
@@ -93,32 +86,32 @@ std::pair<ExecutionState, SharedAqlItemBlockPtr> ExecutionBlockImpl<IdExecutor<B
   return traceGetSomeEnd(state, block);
 }
 
-bool aql::ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>>::isDone() const noexcept {
+bool aql::ExecutionBlockImpl<IdExecutor<void>>::isDone() const noexcept {
   // I'd like to assert this in the constructor, but the dependencies are
   // added after construction.
   TRI_ASSERT(!_dependencies.empty());
   return _currentDependency >= _dependencies.size();
 }
 
-RegisterId ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>>::getOutputRegisterId() const noexcept {
+RegisterId ExecutionBlockImpl<IdExecutor<void>>::getOutputRegisterId() const noexcept {
   return _outputRegister;
 }
 
-ExecutionBlock& ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>>::currentDependency() const {
+ExecutionBlock& ExecutionBlockImpl<IdExecutor<void>>::currentDependency() const {
   TRI_ASSERT(_currentDependency < _dependencies.size());
   TRI_ASSERT(_dependencies[_currentDependency] != nullptr);
   return *_dependencies[_currentDependency];
 }
 
-void ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>>::nextDependency() noexcept {
+void ExecutionBlockImpl<IdExecutor<void>>::nextDependency() noexcept {
   ++_currentDependency;
 }
 
-bool ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>>::doCount() const noexcept {
+bool ExecutionBlockImpl<IdExecutor<void>>::doCount() const noexcept {
   return _doCount;
 }
 
-void ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>>::countStats(SharedAqlItemBlockPtr& block) {
+void ExecutionBlockImpl<IdExecutor<void>>::countStats(SharedAqlItemBlockPtr& block) {
   if (doCount() && block != nullptr) {
     CountStats stats;
     stats.setCounted(block->size());
@@ -144,19 +137,19 @@ bool IdExecutorInfos::isResponsibleForInitializeCursor() const {
   return _isResponsibleForInitializeCursor;
 }
 
-template <BlockPassthrough usePassThrough, class UsedFetcher>
+template <class UsedFetcher>
-IdExecutor<usePassThrough, UsedFetcher>::IdExecutor(Fetcher& fetcher, IdExecutorInfos& infos)
+IdExecutor<UsedFetcher>::IdExecutor(Fetcher& fetcher, IdExecutorInfos& infos)
     : _fetcher(fetcher) {
   if (!infos.distributeId().empty()) {
     _fetcher.setDistributeId(infos.distributeId());
   }
 }
 
-template <BlockPassthrough usePassThrough, class UsedFetcher>
+template <class UsedFetcher>
-IdExecutor<usePassThrough, UsedFetcher>::~IdExecutor() = default;
+IdExecutor<UsedFetcher>::~IdExecutor() = default;
 
-template <BlockPassthrough usePassThrough, class UsedFetcher>
+template <class UsedFetcher>
-std::pair<ExecutionState, NoStats> IdExecutor<usePassThrough, UsedFetcher>::produceRows(OutputAqlItemRow& output) {
+std::pair<ExecutionState, NoStats> IdExecutor<UsedFetcher>::produceRows(OutputAqlItemRow& output) {
   ExecutionState state = ExecutionState::HASMORE;
   NoStats stats;
   InputAqlItemRow inputRow = InputAqlItemRow{CreateInvalidInputRowHint{}};
@@ -191,34 +184,13 @@ std::pair<ExecutionState, NoStats> IdExecutor<usePassThrough, UsedFetcher>::prod
   return {state, stats};
 }
 
-template <BlockPassthrough usePassThrough, class UsedFetcher>
+template <class UsedFetcher>
-template <BlockPassthrough allowPass, typename>
+std::tuple<ExecutionState, typename IdExecutor<UsedFetcher>::Stats, SharedAqlItemBlockPtr>
-std::tuple<ExecutionState, NoStats, size_t> IdExecutor<usePassThrough, UsedFetcher>::skipRows(size_t atMost) {
+IdExecutor<UsedFetcher>::fetchBlockForPassthrough(size_t atMost) {
-  ExecutionState state;
-  size_t skipped;
-  std::tie(state, skipped) = _fetcher.skipRows(atMost);
-  return {state, NoStats{}, skipped};
-}
-
-template <BlockPassthrough usePassThrough, class UsedFetcher>
-template <BlockPassthrough allowPass, typename>
-std::tuple<ExecutionState, typename IdExecutor<usePassThrough, UsedFetcher>::Stats, SharedAqlItemBlockPtr>
-IdExecutor<usePassThrough, UsedFetcher>::fetchBlockForPassthrough(size_t atMost) {
   auto rv = _fetcher.fetchBlockForPassthrough(atMost);
   return {rv.first, {}, std::move(rv.second)};
 }
 
-template class ::arangodb::aql::IdExecutor<BlockPassthrough::Enable, ConstFetcher>;
+template class ::arangodb::aql::IdExecutor<ConstFetcher>;
 // ID can always pass through
-template class ::arangodb::aql::IdExecutor<BlockPassthrough::Enable, SingleRowFetcher<BlockPassthrough::Enable>>;
+template class ::arangodb::aql::IdExecutor<SingleRowFetcher<BlockPassthrough::Enable>>;
-// Local gather does NOT want to passThrough
-template class ::arangodb::aql::IdExecutor<BlockPassthrough::Disable, SingleRowFetcher<BlockPassthrough::Disable>>;
-
-template std::tuple<ExecutionState, typename IdExecutor<BlockPassthrough::Enable, ConstFetcher>::Stats, SharedAqlItemBlockPtr>
-IdExecutor<BlockPassthrough::Enable, ConstFetcher>::fetchBlockForPassthrough<BlockPassthrough::Enable, void>(size_t atMost);
-
-template std::tuple<ExecutionState, typename IdExecutor<BlockPassthrough::Enable, SingleRowFetcher<BlockPassthrough::Enable>>::Stats, SharedAqlItemBlockPtr>
-IdExecutor<BlockPassthrough::Enable, SingleRowFetcher<BlockPassthrough::Enable>>::fetchBlockForPassthrough<BlockPassthrough::Enable, void>(size_t atMost);
-
-template std::tuple<ExecutionState, NoStats, size_t>
-IdExecutor<BlockPassthrough::Disable, SingleRowFetcher<BlockPassthrough::Disable>>::skipRows<BlockPassthrough::Disable, void>(size_t atMost);

arangod/Aql/IdExecutor.h

@@ -31,7 +31,20 @@
 #include <tuple>
 #include <utility>
 
-// TODO Clean up unused variants of the IdExecutor - some of them aren't in use anymore.
+// There are currently three variants of IdExecutor in use:
+//
+// - IdExecutor<void>
+//     This is a variant of the ReturnBlock. It can optionally count and holds
+//     an output register id.
+// - IdExecutor<ConstFetcher>
+//     This is the SingletonBlock.
+// - IdExecutor<SingleRowFetcher<BlockPassthrough::Enable>>
+//     This is the DistributeConsumerBlock. It holds a distributeId and honors
+//     isResponsibleForInitializeCursor.
+//
+// The last variant using the SingleRowFetcher could be replaced by the (faster)
+// void variant. It only has to learn distributeId and
+// isResponsibleForInitializeCursor for that.
 
 namespace arangodb {
 namespace transaction {
@@ -57,9 +70,8 @@ class IdExecutorInfos : public ExecutorInfos {
   IdExecutorInfos(IdExecutorInfos const&) = delete;
   ~IdExecutorInfos() = default;
 
-  std::string const& distributeId();
+  [[nodiscard]] std::string const& distributeId();
 
-  // TODO This is probably needed only for UnsortingGather now, so can be removed here.
   [[nodiscard]] bool isResponsibleForInitializeCursor() const;
 
  private:
@@ -69,16 +81,16 @@ class IdExecutorInfos : public ExecutorInfos {
 };
 
 // forward declaration
-template <BlockPassthrough usePassThrough, class T>
+template <class Fetcher>
 class IdExecutor;
 
 // (empty) implementation of IdExecutor<void>
 template <>
-class IdExecutor<BlockPassthrough::Enable, void> {};
+class IdExecutor<void> {};
 
 // implementation of ExecutionBlockImpl<IdExecutor<void>>
 template <>
-class ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>> : public ExecutionBlock {
+class ExecutionBlockImpl<IdExecutor<void>> : public ExecutionBlock {
  public:
   ExecutionBlockImpl(ExecutionEngine* engine, ExecutionNode const* node,
                      RegisterId outputRegister, bool doCount);
@@ -108,13 +120,13 @@ class ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, void>> : public Ex
   bool const _doCount;
 };
 
-template <BlockPassthrough usePassThrough, class UsedFetcher>
+template <class UsedFetcher>
 // cppcheck-suppress noConstructor
 class IdExecutor {
  public:
   struct Properties {
     static constexpr bool preservesOrder = true;
-    static constexpr BlockPassthrough allowsBlockPassthrough = usePassThrough;
+    static constexpr BlockPassthrough allowsBlockPassthrough = BlockPassthrough::Enable;
     static constexpr bool inputSizeRestrictsOutputSize = false;
   };
   // Only Supports SingleRowFetcher and ConstFetcher
@@ -133,12 +145,8 @@ class IdExecutor {
    */
   std::pair<ExecutionState, Stats> produceRows(OutputAqlItemRow& output);
 
-  template <BlockPassthrough allowPass = usePassThrough, typename = std::enable_if_t<allowPass == BlockPassthrough::Enable>>
   std::tuple<ExecutionState, Stats, SharedAqlItemBlockPtr> fetchBlockForPassthrough(size_t atMost);
 
-  template <BlockPassthrough allowPass = usePassThrough, typename = std::enable_if_t<allowPass == BlockPassthrough::Disable>>
-  std::tuple<ExecutionState, NoStats, size_t> skipRows(size_t atMost);
-
  private:
   Fetcher& _fetcher;
 };

arangod/Aql/IndexExecutor.cpp

@@ -55,10 +55,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool IndexExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough IndexExecutor::Properties::allowsBlockPassthrough;
-constexpr bool IndexExecutor::Properties::inputSizeRestrictsOutputSize;
-
 namespace {
 /// resolve constant attribute accesses
 static void resolveFCallConstAttributes(AstNode* fcall) {

arangod/Aql/KShortestPathsExecutor.cpp

@@ -42,10 +42,6 @@
 
 using namespace arangodb;
 using namespace arangodb::aql;
-
-constexpr bool KShortestPathsExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough KShortestPathsExecutor::Properties::allowsBlockPassthrough;
-constexpr bool KShortestPathsExecutor::Properties::inputSizeRestrictsOutputSize;
 using namespace arangodb::graph;
 
 namespace {

arangod/Aql/LimitExecutor.cpp

@@ -37,10 +37,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool LimitExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough LimitExecutor::Properties::allowsBlockPassthrough;
-constexpr bool LimitExecutor::Properties::inputSizeRestrictsOutputSize;
-
 LimitExecutorInfos::LimitExecutorInfos(RegisterId nrInputRegisters, RegisterId nrOutputRegisters,
                                        // cppcheck-suppress passedByValue
                                        std::unordered_set<RegisterId> registersToClear,

arangod/Aql/NoResultsExecutor.cpp

@@ -30,10 +30,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool NoResultsExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough NoResultsExecutor::Properties::allowsBlockPassthrough;
-constexpr bool NoResultsExecutor::Properties::inputSizeRestrictsOutputSize;
-
 NoResultsExecutor::NoResultsExecutor(Fetcher& fetcher, ExecutorInfos& infos) {}
 NoResultsExecutor::~NoResultsExecutor() = default;
 

arangod/Aql/OptimizerRules.cpp

@@ -7283,7 +7283,7 @@ namespace {
 
 bool nodeMakesThisQueryLevelUnsuitableForSubquerySplicing(ExecutionNode const* const node) {
   // TODO Enable modification nodes again, as soon as the corresponding branch
-  //  is merged.
+  //  is merged. Fix them in the switch statement below, too!
   if (node->isModificationNode()) {
     return true;
   }

arangod/Aql/ParallelUnsortedGatherExecutor.cpp

@@ -0,0 +1,174 @@
+////////////////////////////////////////////////////////////////////////////////
+/// DISCLAIMER
+///
+/// Copyright 2019 ArangoDB GmbH, Cologne, Germany
+///
+/// Licensed under the Apache License, Version 2.0 (the "License");
+/// you may not use this file except in compliance with the License.
+/// You may obtain a copy of the License at
+///
+///     http://www.apache.org/licenses/LICENSE-2.0
+///
+/// Unless required by applicable law or agreed to in writing, software
+/// distributed under the License is distributed on an "AS IS" BASIS,
+/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+/// See the License for the specific language governing permissions and
+/// limitations under the License.
+///
+/// Copyright holder is ArangoDB GmbH, Cologne, Germany
+///
+/// @author Simon Grätzer
+////////////////////////////////////////////////////////////////////////////////
+
+#include "ParallelUnsortedGatherExecutor.h"
+
+#include "Aql/MultiDependencySingleRowFetcher.h"
+#include "Aql/OutputAqlItemRow.h"
+#include "Aql/Stats.h"
+#include "Transaction/Methods.h"
+
+#include "Logger/LogMacros.h"
+
+using namespace arangodb;
+using namespace arangodb::aql;
+
+ParallelUnsortedGatherExecutorInfos::ParallelUnsortedGatherExecutorInfos(
+    RegisterId nrInOutRegisters, std::unordered_set<RegisterId> registersToKeep,
+    std::unordered_set<RegisterId> registersToClear)
+    : ExecutorInfos(make_shared_unordered_set(), make_shared_unordered_set(),
+                    nrInOutRegisters, nrInOutRegisters,
+                    std::move(registersToClear), std::move(registersToKeep)) {}
+
+ParallelUnsortedGatherExecutor::ParallelUnsortedGatherExecutor(Fetcher& fetcher, Infos& infos)
+    : _fetcher(fetcher), _numberDependencies(0), _currentDependency(0), _skipped(0) {}
+
+ParallelUnsortedGatherExecutor::~ParallelUnsortedGatherExecutor() = default;
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief Guarantees requiredby this this block:
+///        1) For every dependency the input is sorted, according to the same strategy.
+///
+///        What this block does:
+///        InitPhase:
+///          Fetch 1 Block for every dependency.
+///        ExecPhase:
+///          Fetch row of scheduled block.
+///          Pick the next (sorted) element (by strategy)
+///          Schedule this block to fetch Row
+///
+////////////////////////////////////////////////////////////////////////////////
+
+std::pair<ExecutionState, NoStats> ParallelUnsortedGatherExecutor::produceRows(OutputAqlItemRow& output) {
+  initDependencies();
+  
+  ExecutionState state;
+  InputAqlItemRow inputRow = InputAqlItemRow{CreateInvalidInputRowHint{}};
+  
+  size_t x;
+  for (x = 0; x < _numberDependencies; ++x) {
+    size_t i = (_currentDependency + x) % _numberDependencies;
+  
+    if (_upstream[i] == ExecutionState::DONE) {
+      continue;
+    }
+    
+    size_t tmp = 0;
+    
+    state = ExecutionState::HASMORE;
+    while (!output.isFull() && state == ExecutionState::HASMORE) {
+      std::tie(state, inputRow) = _fetcher.fetchRowForDependency(i, output.numRowsLeft() /*atMost*/);
+      if (inputRow) {
+        output.copyRow(inputRow);
+        TRI_ASSERT(output.produced());
+        output.advanceRow();
+        tmp++;
+      }
+    }
+    
+    _upstream[i] = state;
+    if (output.isFull()) {
+      break;
+    }
+  }
+  _currentDependency = x;
+  
+  NoStats stats;
+  
+  // fix assert in ExecutionBlockImpl<E>::getSomeWithoutTrace
+  if (output.isFull()) {
+    return {ExecutionState::HASMORE, stats};
+  }
+  
+  size_t numWaiting = 0;
+  for (x = 0; x < _numberDependencies; ++x) {
+    if (_upstream[x] == ExecutionState::HASMORE) {
+      return {ExecutionState::HASMORE, stats};
+    } else if (_upstream[x] == ExecutionState::WAITING) {
+      numWaiting++;
+    }
+  }
+  if (numWaiting > 0) {
+    return {ExecutionState::WAITING, stats};
+  }
+  
+  TRI_ASSERT(std::all_of(_upstream.begin(), _upstream.end(), [](auto const& s) { return s == ExecutionState::DONE; } ));
+  return {ExecutionState::DONE, stats};
+}
+
+std::tuple<ExecutionState, ParallelUnsortedGatherExecutor::Stats, size_t>
+ParallelUnsortedGatherExecutor::skipRows(size_t const toSkip) {
+  initDependencies();
+  TRI_ASSERT(_skipped <= toSkip);
+  
+  ExecutionState state = ExecutionState::HASMORE;
+  while (_skipped < toSkip) {
+    
+    const size_t i = _currentDependency;
+    if (_upstream[i] == ExecutionState::DONE) {
+      if (std::all_of(_upstream.begin(), _upstream.end(),
+                      [](auto s) { return s == ExecutionState::DONE; })) {
+         state = ExecutionState::DONE;
+         break;
+       }
+      _currentDependency = (i + 1) % _numberDependencies;
+       continue;
+     }
+    
+    TRI_ASSERT(_skipped <= toSkip);
+
+    size_t skippedNow;
+    std::tie(state, skippedNow) = _fetcher.skipRowsForDependency(i, toSkip - _skipped);
+    _upstream[i] = state;
+    if (state == ExecutionState::WAITING) {
+      TRI_ASSERT(skippedNow == 0);
+      return {ExecutionState::WAITING, NoStats{}, 0};
+    }
+    _skipped += skippedNow;
+    
+    if (_upstream[i] == ExecutionState::DONE) {
+      if (std::all_of(_upstream.begin(), _upstream.end(),
+                      [](auto s) { return s == ExecutionState::DONE; })) {
+        break;
+      }
+       _currentDependency = (i + 1) % _numberDependencies;
+       continue;
+     }
+  }
+  
+  size_t skipped = _skipped;
+  _skipped = 0;
+
+  TRI_ASSERT(skipped <= toSkip);
+  return {state, NoStats{}, skipped};
+}
+
+void ParallelUnsortedGatherExecutor::initDependencies() {
+  if (_numberDependencies == 0) {
+    // We need to initialize the dependencies once, they are injected
+    // after the fetcher is created.
+    _numberDependencies = _fetcher.numberDependencies();
+    TRI_ASSERT(_numberDependencies > 0);
+    _upstream.resize(_numberDependencies, ExecutionState::HASMORE);
+    TRI_ASSERT(std::all_of(_upstream.begin(), _upstream.end(), [](auto const& s) { return s == ExecutionState::HASMORE; } ));
+  }
+}

arangod/Aql/ParallelUnsortedGatherExecutor.h

@@ -0,0 +1,104 @@
+////////////////////////////////////////////////////////////////////////////////
+/// DISCLAIMER
+///
+/// Copyright 2019 ArangoDB GmbH, Cologne, Germany
+///
+/// Licensed under the Apache License, Version 2.0 (the "License");
+/// you may not use this file except in compliance with the License.
+/// You may obtain a copy of the License at
+///
+///     http://www.apache.org/licenses/LICENSE-2.0
+///
+/// Unless required by applicable law or agreed to in writing, software
+/// distributed under the License is distributed on an "AS IS" BASIS,
+/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+/// See the License for the specific language governing permissions and
+/// limitations under the License.
+///
+/// Copyright holder is ArangoDB GmbH, Cologne, Germany
+///
+/// @author Simon Grätzer
+////////////////////////////////////////////////////////////////////////////////
+
+#ifndef ARANGOD_AQL_PARALLEL_UNSORTED_GATHER_EXECUTOR_H
+#define ARANGOD_AQL_PARALLEL_UNSORTED_GATHER_EXECUTOR_H
+
+#include "Aql/ClusterNodes.h"
+#include "Aql/ExecutionState.h"
+#include "Aql/ExecutorInfos.h"
+#include "Aql/InputAqlItemRow.h"
+#include "Containers/SmallVector.h"
+
+namespace arangodb {
+
+namespace transaction {
+class Methods;
+}
+
+namespace aql {
+
+class MultiDependencySingleRowFetcher;
+class NoStats;
+class OutputAqlItemRow;
+struct SortRegister;
+
+class ParallelUnsortedGatherExecutorInfos : public ExecutorInfos {
+ public:
+  ParallelUnsortedGatherExecutorInfos(RegisterId nrInOutRegisters,
+                                      std::unordered_set<RegisterId> registersToKeep,
+                                      std::unordered_set<RegisterId> registersToClear);
+  ParallelUnsortedGatherExecutorInfos() = delete;
+  ParallelUnsortedGatherExecutorInfos(ParallelUnsortedGatherExecutorInfos&&) = default;
+  ParallelUnsortedGatherExecutorInfos(ParallelUnsortedGatherExecutorInfos const&) = delete;
+  ~ParallelUnsortedGatherExecutorInfos() = default;
+};
+
+class ParallelUnsortedGatherExecutor {
+ public:
+
+ public:
+  struct Properties {
+    static constexpr bool preservesOrder = true;
+    static constexpr BlockPassthrough allowsBlockPassthrough = BlockPassthrough::Disable;
+    static constexpr bool inputSizeRestrictsOutputSize = false;
+  };
+
+  using Fetcher = MultiDependencySingleRowFetcher;
+  using Infos = ParallelUnsortedGatherExecutorInfos;
+  using Stats = NoStats;
+
+  ParallelUnsortedGatherExecutor(Fetcher& fetcher, Infos& info);
+  ~ParallelUnsortedGatherExecutor();
+
+  /**
+   * @brief produce the next Row of Aql Values.
+   *
+   * @return ExecutionState,
+   *         if something was written output.hasValue() == true
+   */
+  std::pair<ExecutionState, Stats> produceRows(OutputAqlItemRow& output);
+
+  std::tuple<ExecutionState, Stats, size_t> skipRows(size_t atMost);
+  
+ private:
+  
+  void initDependencies();
+  
+ private:
+  Fetcher& _fetcher;
+  // 64: default size of buffer; 8: Alignment size; computed to 4 but breaks in windows debug build.
+  ::arangodb::containers::SmallVector<ExecutionState, 64, 8>::allocator_type::arena_type _arena;
+  ::arangodb::containers::SmallVector<ExecutionState, 64, 8> _upstream{_arena};
+
+  // Total Number of dependencies
+  size_t _numberDependencies;
+  
+  size_t _currentDependency;
+  
+  size_t _skipped;
+};
+
+}  // namespace aql
+}  // namespace arangodb
+
+#endif  // ARANGOD_AQL_PARALLEL_UNSORTED_GATHER_EXECUTOR_H

arangod/Aql/Query.cpp

@@ -499,7 +499,7 @@ std::unique_ptr<ExecutionPlan> Query::preparePlan() {
   }
 #endif
 
-  auto trx = AqlTransaction::create(std::move(ctx), _collections.collections(),
+  auto trx = AqlTransaction::create(ctx, _collections.collections(),
                                     _queryOptions.transactionOptions, _part == PART_MAIN,
                                     std::move(inaccessibleCollections));
   // create the transaction object, but do not start it yet

arangod/Aql/ReturnExecutor.cpp

@@ -31,10 +31,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool ReturnExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough ReturnExecutor::Properties::allowsBlockPassthrough;
-constexpr bool ReturnExecutor::Properties::inputSizeRestrictsOutputSize;
-
 ReturnExecutorInfos::ReturnExecutorInfos(RegisterId inputRegister, RegisterId nrInputRegisters,
                                          RegisterId nrOutputRegisters, bool doCount)
     : ExecutorInfos(make_shared_unordered_set({inputRegister}),

arangod/Aql/ShadowAqlItemRow.h

@@ -66,24 +66,24 @@ class ShadowAqlItemRow {
   /// @brief get the number of data registers in the underlying block.
   ///        Not all of these registers are necessarily filled by this
   ///        ShadowRow. There might be empty registers on deeper levels.
-  RegisterCount getNrRegisters() const noexcept;
+  [[nodiscard]] RegisterCount getNrRegisters() const noexcept;
 
   /// @brief a ShadowRow is relevant iff it indicates an end of subquery block on the subquery context
   ///        we are in right now. This will only be of importance on nested subqueries.
-  ///        Within the inner subquery all shadowrows of this inner are relavant. All shadowRows
+  ///        Within the inner subquery all shadowrows of this inner are relevant. All shadowRows
   ///        of the outer subquery are NOT relevant
   ///        Also note: There is a guarantee that a non-relevant shadowrow, can only be encountered
   ///        right after a shadowrow. And only in descending nesting level. (eg 1. inner most, 2. inner, 3. outer most)
-  bool isRelevant() const noexcept;
+  [[nodiscard]] bool isRelevant() const noexcept;
 
   /// @brief Test if this shadow row is initialized, eg has a block and has a valid depth.
-  bool isInitialized() const;
+  [[nodiscard]] bool isInitialized() const;
 
 #ifdef ARANGODB_ENABLE_MAINTAINER_MODE
   /**
    * @brief Compare the underlying block. Only for assertions.
    */
-  bool internalBlockIs(SharedAqlItemBlockPtr const& other) const;
+  [[nodiscard]] bool internalBlockIs(SharedAqlItemBlockPtr const& other) const;
 #endif
 
   /**
@@ -93,21 +93,21 @@ class ShadowAqlItemRow {
    *
    * @return Reference to the AqlValue stored in that variable.
    */
-  AqlValue const& getValue(RegisterId registerId) const;
+  [[nodiscard]] AqlValue const& getValue(RegisterId registerId) const;
 
   /// @brief get the depthValue of the shadow row as AqlValue
-  AqlValue const& getShadowDepthValue() const;
+  [[nodiscard]] AqlValue const& getShadowDepthValue() const;
 
   /// @brief get the depthValue of the shadow row as int64_t >= 0
   ///        NOTE: Innermost query will have depth 0. Outermost query wil have highest depth.
-  uint64_t getDepth() const;
+  [[nodiscard]] uint64_t getDepth() const;
 
   // Note that == and != here check whether the rows are *identical*, that is,
   // the same row in the same block.
   // TODO Make this a named method
-  bool operator==(ShadowAqlItemRow const& other) const noexcept;
+  [[nodiscard]] bool operator==(ShadowAqlItemRow const& other) const noexcept;
 
-  bool operator!=(ShadowAqlItemRow const& other) const noexcept;
+  [[nodiscard]] bool operator!=(ShadowAqlItemRow const& other) const noexcept;
 
   // This checks whether the rows are equivalent, in the sense that they hold
   // the same number of registers and their entry-AqlValues compare equal,
@@ -120,9 +120,9 @@ class ShadowAqlItemRow {
                              velocypack::Options const* option) const noexcept;
 
  private:
-  AqlItemBlock& block() noexcept;
+  [[nodiscard]] AqlItemBlock& block() noexcept;
 
-  AqlItemBlock const& block() const noexcept;
+  [[nodiscard]] AqlItemBlock const& block() const noexcept;
 
  private:
   /**

arangod/Aql/ShortestPathExecutor.cpp

@@ -41,10 +41,6 @@
 
 using namespace arangodb;
 using namespace arangodb::aql;
-
-constexpr bool ShortestPathExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough ShortestPathExecutor::Properties::allowsBlockPassthrough;
-constexpr bool ShortestPathExecutor::Properties::inputSizeRestrictsOutputSize;
 using namespace arangodb::graph;
 
 namespace {

arangod/Aql/SingleRemoteModificationExecutor.cpp

@@ -38,13 +38,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-template <typename Modifier>
-constexpr bool SingleRemoteModificationExecutor<Modifier>::Properties::preservesOrder;
-template <typename Modifier>
-constexpr BlockPassthrough SingleRemoteModificationExecutor<Modifier>::Properties::allowsBlockPassthrough;
-template <typename Modifier>
-constexpr bool SingleRemoteModificationExecutor<Modifier>::Properties::inputSizeRestrictsOutputSize;
-
 namespace {
 std::unique_ptr<VPackBuilder> merge(VPackSlice document, std::string const& key,
                                     TRI_voc_rid_t revision) {

arangod/Aql/SingleRowFetcher.h

@@ -61,8 +61,6 @@ class SingleRowFetcher {
  public:
   /**
    * @brief Fetch one new AqlItemRow from upstream.
-   *        **Guarantee**: the row returned is valid only
-   *        until the next call to fetchRow.
    *
    * @param atMost may be passed if a block knows the maximum it might want to
    *        fetch from upstream (should apply only to the LimitExecutor). Will
@@ -71,7 +69,7 @@ class SingleRowFetcher {
    *
    * @return A pair with the following properties:
    *         ExecutionState:
-   *           WAITING => IO going on, immediatly return to caller.
+   *           WAITING => IO going on, immediately return to caller.
    *           DONE => No more to expect from Upstream, if you are done with
    *                   this row return DONE to caller.
    *           HASMORE => There is potentially more from above, call again if

arangod/Aql/SortExecutor.cpp

@@ -34,10 +34,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool SortExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough SortExecutor::Properties::allowsBlockPassthrough;
-constexpr bool SortExecutor::Properties::inputSizeRestrictsOutputSize;
-
 namespace {
 
 /// @brief OurLessThan
@@ -83,15 +79,12 @@ static std::shared_ptr<std::unordered_set<RegisterId>> mapSortRegistersToRegiste
   return set;
 }
 
-SortExecutorInfos::SortExecutorInfos(
+SortExecutorInfos::SortExecutorInfos(std::vector<SortRegister> sortRegisters,
-    // cppcheck-suppress passedByValue
+                                     std::size_t limit, AqlItemBlockManager& manager,
-    std::vector<SortRegister> sortRegisters, std::size_t limit,
+                                     RegisterId nrInputRegisters, RegisterId nrOutputRegisters,
-    AqlItemBlockManager& manager, RegisterId nrInputRegisters, RegisterId nrOutputRegisters,
+                                     std::unordered_set<RegisterId> registersToClear,
-    // cppcheck-suppress passedByValue
+                                     std::unordered_set<RegisterId> registersToKeep,
-    std::unordered_set<RegisterId> registersToClear,
+                                     velocypack::Options const* options, bool stable)
-    // cppcheck-suppress passedByValue
-    std::unordered_set<RegisterId> registersToKeep,
-    velocypack::Options const* options, bool stable)
     : ExecutorInfos(mapSortRegistersToRegisterIds(sortRegisters), nullptr,
                     nrInputRegisters, nrOutputRegisters,
                     std::move(registersToClear), std::move(registersToKeep)),
@@ -103,7 +96,7 @@ SortExecutorInfos::SortExecutorInfos(
   TRI_ASSERT(!_sortRegisters.empty());
 }
 
-std::vector<SortRegister>& SortExecutorInfos::sortRegisters() {
+std::vector<SortRegister> const& SortExecutorInfos::sortRegisters() const noexcept {
   return _sortRegisters;
 }
 
@@ -113,6 +106,12 @@ velocypack::Options const* SortExecutorInfos::vpackOptions() const noexcept {
   return _vpackOptions;
 }
 
+size_t SortExecutorInfos::limit() const noexcept { return _limit; }
+
+AqlItemBlockManager& SortExecutorInfos::itemBlockManager() noexcept {
+  return _manager;
+}
+
 SortExecutor::SortExecutor(Fetcher& fetcher, SortExecutorInfos& infos)
     : _infos(infos), _fetcher(fetcher), _input(nullptr), _returnNext(0) {}
 SortExecutor::~SortExecutor() = default;

arangod/Aql/SortExecutor.h

@@ -63,10 +63,15 @@ class SortExecutorInfos : public ExecutorInfos {
 
   [[nodiscard]] velocypack::Options const* vpackOptions() const noexcept;
 
-  std::vector<SortRegister>& sortRegisters();
+  [[nodiscard]] std::vector<SortRegister> const& sortRegisters() const noexcept;
 
-  bool stable() const;
+  [[nodiscard]] bool stable() const;
 
+  [[nodiscard]] size_t limit() const noexcept;
+
+  [[nodiscard]] AqlItemBlockManager& itemBlockManager() noexcept;
+
+ private:
   std::size_t _limit;
   AqlItemBlockManager& _manager;
   velocypack::Options const* _vpackOptions;

arangod/Aql/SortNode.cpp

@@ -235,7 +235,7 @@ std::unique_ptr<ExecutionBlock> SortNode::createBlock(
     auto it = getRegisterPlan()->varInfo.find(element.var->id);
     TRI_ASSERT(it != getRegisterPlan()->varInfo.end());
     RegisterId id = it->second.registerId;
-    sortRegs.push_back(SortRegister{id, element});
+    sortRegs.emplace_back(id, element);
   }
   SortExecutorInfos infos(std::move(sortRegs), _limit, engine.itemBlockManager(),
                           getRegisterPlan()->nrRegs[previousNode->getDepth()],

arangod/Aql/SortedCollectExecutor.cpp

@@ -40,10 +40,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool SortedCollectExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough SortedCollectExecutor::Properties::allowsBlockPassthrough;
-constexpr bool SortedCollectExecutor::Properties::inputSizeRestrictsOutputSize;
-
 static const AqlValue EmptyValue;
 
 SortedCollectExecutor::CollectGroup::CollectGroup(bool count, Infos& infos)

arangod/Aql/SortingGatherExecutor.cpp

@@ -31,10 +31,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool SortingGatherExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough SortingGatherExecutor::Properties::allowsBlockPassthrough;
-constexpr bool SortingGatherExecutor::Properties::inputSizeRestrictsOutputSize;
-
 namespace {
 
 /// @brief OurLessThan: comparison method for elements of SortingGatherBlock

arangod/Aql/SubqueryEndExecutor.cpp

@@ -35,10 +35,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool SubqueryEndExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough SubqueryEndExecutor::Properties::allowsBlockPassthrough;
-constexpr bool SubqueryEndExecutor::Properties::inputSizeRestrictsOutputSize;
-
 SubqueryEndExecutorInfos::SubqueryEndExecutorInfos(
     std::shared_ptr<std::unordered_set<RegisterId>> readableInputRegisters,
     std::shared_ptr<std::unordered_set<RegisterId>> writeableOutputRegisters,
@@ -53,11 +49,9 @@ SubqueryEndExecutorInfos::SubqueryEndExecutorInfos(
       _outReg(outReg),
       _inReg(inReg) {}
 
-SubqueryEndExecutorInfos::SubqueryEndExecutorInfos(SubqueryEndExecutorInfos&& other) = default;
-
 SubqueryEndExecutorInfos::~SubqueryEndExecutorInfos() = default;
 
-bool SubqueryEndExecutorInfos::usesInputRegister() const {
+bool SubqueryEndExecutorInfos::usesInputRegister() const noexcept {
   return _inReg != RegisterPlan::MaxRegisterId;
 }
 
@@ -65,6 +59,12 @@ velocypack::Options const* SubqueryEndExecutorInfos::vpackOptions() const noexce
   return _vpackOptions;
 }
 
+RegisterId SubqueryEndExecutorInfos::getOutputRegister() const noexcept {
+  return _outReg;
+}
+
+RegisterId SubqueryEndExecutorInfos::getInputRegister() const noexcept { return _inReg; }
+
 SubqueryEndExecutor::SubqueryEndExecutor(Fetcher& fetcher, SubqueryEndExecutorInfos& infos)
     : _fetcher(fetcher), _infos(infos), _accumulator(nullptr), _state(ACCUMULATE) {
   resetAccumulator();

arangod/Aql/SubqueryEndExecutor.h

@@ -52,14 +52,14 @@ class SubqueryEndExecutorInfos : public ExecutorInfos {
                            RegisterId outReg);
 
   SubqueryEndExecutorInfos() = delete;
-  SubqueryEndExecutorInfos(SubqueryEndExecutorInfos&&);
+  SubqueryEndExecutorInfos(SubqueryEndExecutorInfos&&) noexcept = default;
   SubqueryEndExecutorInfos(SubqueryEndExecutorInfos const&) = delete;
   ~SubqueryEndExecutorInfos();
 
   [[nodiscard]] velocypack::Options const* vpackOptions() const noexcept;
-  inline RegisterId getOutputRegister() const { return _outReg; }
+  [[nodiscard]] RegisterId getOutputRegister() const noexcept;
-  bool usesInputRegister() const;
+  [[nodiscard]] bool usesInputRegister() const noexcept;
-  inline RegisterId getInputRegister() const { return _inReg; }
+  [[nodiscard]] RegisterId getInputRegister() const noexcept;
 
  private:
   velocypack::Options const* _vpackOptions;

arangod/Aql/SubqueryExecutor.cpp

@@ -30,13 +30,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-template <bool isModificationSubquery>
-constexpr bool SubqueryExecutor<isModificationSubquery>::Properties::preservesOrder;
-template <bool isModificationSubquery>
-constexpr BlockPassthrough SubqueryExecutor<isModificationSubquery>::Properties::allowsBlockPassthrough;
-template <bool isModificationSubquery>
-constexpr bool SubqueryExecutor<isModificationSubquery>::Properties::inputSizeRestrictsOutputSize;
-
 SubqueryExecutorInfos::SubqueryExecutorInfos(
     std::shared_ptr<std::unordered_set<RegisterId>> readableInputRegisters,
     std::shared_ptr<std::unordered_set<RegisterId>> writeableOutputRegisters,

arangod/Aql/SubqueryStartExecutor.cpp

@@ -30,10 +30,6 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool SubqueryStartExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough SubqueryStartExecutor::Properties::allowsBlockPassthrough;
-constexpr bool SubqueryStartExecutor::Properties::inputSizeRestrictsOutputSize;
-
 SubqueryStartExecutor::SubqueryStartExecutor(Fetcher& fetcher, Infos& infos)
     : _fetcher(fetcher),
       _state(ExecutionState::HASMORE),

arangod/Aql/TraversalExecutor.cpp

@@ -37,10 +37,6 @@ using namespace arangodb;
 using namespace arangodb::aql;
 using namespace arangodb::traverser;
 
-constexpr bool TraversalExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough TraversalExecutor::Properties::allowsBlockPassthrough;
-constexpr bool TraversalExecutor::Properties::inputSizeRestrictsOutputSize;
-
 TraversalExecutorInfos::TraversalExecutorInfos(
     std::shared_ptr<std::unordered_set<RegisterId>> inputRegisters,
     std::shared_ptr<std::unordered_set<RegisterId>> outputRegisters, RegisterId nrInputRegisters,

arangod/Aql/UnsortedGatherExecutor.cpp

@@ -17,167 +17,109 @@
 ///
 /// Copyright holder is ArangoDB GmbH, Cologne, Germany
 ///
-/// @author Simon Grätzer
+/// @author Tobias Gödderz
 ////////////////////////////////////////////////////////////////////////////////
 
 #include "UnsortedGatherExecutor.h"
 
+#include "Aql/IdExecutor.h"  // for IdExecutorInfos
 #include "Aql/MultiDependencySingleRowFetcher.h"
 #include "Aql/OutputAqlItemRow.h"
-#include "Aql/SortRegister.h"
 #include "Aql/Stats.h"
-#include "Transaction/Methods.h"
+#include "Basics/debugging.h"
-
-#include "Logger/LogMacros.h"
 
 using namespace arangodb;
 using namespace arangodb::aql;
 
-constexpr bool UnsortedGatherExecutor::Properties::preservesOrder;
-constexpr BlockPassthrough UnsortedGatherExecutor::Properties::allowsBlockPassthrough;
-constexpr bool UnsortedGatherExecutor::Properties::inputSizeRestrictsOutputSize;
-
-UnsortedGatherExecutorInfos::UnsortedGatherExecutorInfos(
-    RegisterId nrInOutRegisters, std::unordered_set<RegisterId> registersToKeep,
-    std::unordered_set<RegisterId> registersToClear)
-    : ExecutorInfos(make_shared_unordered_set(), make_shared_unordered_set(),
-                    nrInOutRegisters, nrInOutRegisters,
-                    std::move(registersToClear), std::move(registersToKeep)) {}
-
 UnsortedGatherExecutor::UnsortedGatherExecutor(Fetcher& fetcher, Infos& infos)
-    : _fetcher(fetcher),
+    : _fetcher(fetcher) {}
-      _numberDependencies(0),
-      _currentDependency(0),
-      _skipped(0) {}
 
 UnsortedGatherExecutor::~UnsortedGatherExecutor() = default;
 
-////////////////////////////////////////////////////////////////////////////////
+auto UnsortedGatherExecutor::produceRows(OutputAqlItemRow& output)
-/// @brief Guarantees requiredby this this block:
+    -> std::pair<ExecutionState, Stats> {
-///        1) For every dependency the input is sorted, according to the same strategy.
+  while (!output.isFull() && !done()) {
-///
+    // Note that fetchNextRow may return DONE (because the current dependency is
-///        What this block does:
+    // DONE), and also return an unitialized row in that case, but we are not
-///        InitPhase:
+    // DONE completely - that's what `done()` is for.
-///          Fetch 1 Block for every dependency.
+    auto [state, inputRow] = fetchNextRow(output.numRowsLeft());
-///        ExecPhase:
-///          Fetch row of scheduled block.
-///          Pick the next (sorted) element (by strategy)
-///          Schedule this block to fetch Row
-///
-////////////////////////////////////////////////////////////////////////////////
-
-std::pair<ExecutionState, NoStats> UnsortedGatherExecutor::produceRows(OutputAqlItemRow& output) {
-
-  initDependencies();
-  
-  ExecutionState state;
-  InputAqlItemRow inputRow = InputAqlItemRow{CreateInvalidInputRowHint{}};
-  
-  size_t x;
-  for (x = 0; x < _numberDependencies; ++x) {
-    size_t i = (_currentDependency + x) % _numberDependencies;
-  
-    if (_upstream[i] == ExecutionState::DONE) {
-      continue;
-    }
-    
-    size_t tmp = 0;
-    
-    state = ExecutionState::HASMORE;
-    while (!output.isFull() && state == ExecutionState::HASMORE) {
-      std::tie(state, inputRow) = _fetcher.fetchRowForDependency(i, output.numRowsLeft() /*atMost*/);
-      if (inputRow) {
-        output.copyRow(inputRow);
-        TRI_ASSERT(output.produced());
-        output.advanceRow();
-        tmp++;
-      }
-    }
-    
-    _upstream[i] = state;
-    if (output.isFull()) {
-      break;
-    }
-  }
-  _currentDependency = x;
-  
-  NoStats stats;
-  
-  // fix assert in ExecutionBlockImpl<E>::getSomeWithoutTrace
-  if (output.isFull()) {
-    return {ExecutionState::HASMORE, stats};
-  }
-  
-  size_t numWaiting = 0;
-  for (x = 0; x < _numberDependencies; ++x) {
-    if (_upstream[x] == ExecutionState::HASMORE) {
-      return {ExecutionState::HASMORE, stats};
-    } else if (_upstream[x] == ExecutionState::WAITING) {
-      numWaiting++;
-    }
-  }
-  if (numWaiting > 0) {
-    return {ExecutionState::WAITING, stats};
-  }
-  
-  TRI_ASSERT(std::all_of(_upstream.begin(), _upstream.end(), [](auto const& s) { return s == ExecutionState::DONE; } ));
-  return {ExecutionState::DONE, stats};
-}
-
-std::tuple<ExecutionState, UnsortedGatherExecutor::Stats, size_t> UnsortedGatherExecutor::skipRows(size_t const toSkip) {
-  
-  initDependencies();
-  TRI_ASSERT(_skipped <= toSkip);
-  
-  ExecutionState state = ExecutionState::HASMORE;
-  while (_skipped < toSkip) {
-    
-    const size_t i = _currentDependency;
-    if (_upstream[i] == ExecutionState::DONE) {
-      if (std::all_of(_upstream.begin(), _upstream.end(),
-                      [](auto s) { return s == ExecutionState::DONE; })) {
-         state = ExecutionState::DONE;
-         break;
-       }
-      _currentDependency = (i + 1) % _numberDependencies;
-       continue;
-     }
-    
-    TRI_ASSERT(_skipped <= toSkip);
-
-    size_t skippedNow;
-    std::tie(state, skippedNow) = _fetcher.skipRowsForDependency(i, toSkip - _skipped);
-    _upstream[i] = state;
     if (state == ExecutionState::WAITING) {
-      TRI_ASSERT(skippedNow == 0);
+      return {state, {}};
-      return {ExecutionState::WAITING, NoStats{}, 0};
+    }
+    // HASMORE => inputRow.isInitialized()
+    TRI_ASSERT(state == ExecutionState::DONE || inputRow.isInitialized());
+    if (inputRow.isInitialized()) {
+      output.copyRow(inputRow);
+      TRI_ASSERT(output.produced());
+      output.advanceRow();
     }
-    _skipped += skippedNow;
-    
-    if (_upstream[i] == ExecutionState::DONE) {
-      if (std::all_of(_upstream.begin(), _upstream.end(),
-                      [](auto s) { return s == ExecutionState::DONE; })) {
-        break;
-      }
-       _currentDependency = (i + 1) % _numberDependencies;
-       continue;
-     }
   }
-  
-  size_t skipped = _skipped;
-  _skipped = 0;
 
-  TRI_ASSERT(skipped <= toSkip);
+  auto state = done() ? ExecutionState::DONE : ExecutionState::HASMORE;
-  return {state, NoStats{}, skipped};
+  return {state, {}};
 }
 
-void UnsortedGatherExecutor::initDependencies() {
+auto UnsortedGatherExecutor::fetcher() const noexcept -> const Fetcher& {
-  if (_numberDependencies == 0) {
+  return _fetcher;
-    // We need to initialize the dependencies once, they are injected
+}
-    // after the fetcher is created.
+
-    _numberDependencies = _fetcher.numberDependencies();
+auto UnsortedGatherExecutor::fetcher() noexcept -> Fetcher& { return _fetcher; }
-    TRI_ASSERT(_numberDependencies > 0);
+
-    _upstream.resize(_numberDependencies, ExecutionState::HASMORE);
+auto UnsortedGatherExecutor::numDependencies() const
-    TRI_ASSERT(std::all_of(_upstream.begin(), _upstream.end(), [](auto const& s) { return s == ExecutionState::HASMORE; } ));
+    noexcept(noexcept(_fetcher.numberDependencies())) -> size_t {
-  }
+  return _fetcher.numberDependencies();
+}
+
+auto UnsortedGatherExecutor::fetchNextRow(size_t atMost)
+    -> std::pair<ExecutionState, InputAqlItemRow> {
+  auto res = fetcher().fetchRowForDependency(currentDependency(), atMost);
+  if (res.first == ExecutionState::DONE) {
+    advanceDependency();
+  }
+  return res;
+}
+
+auto UnsortedGatherExecutor::skipNextRows(size_t atMost)
+    -> std::pair<ExecutionState, size_t> {
+  auto res = fetcher().skipRowsForDependency(currentDependency(), atMost);
+  if (res.first == ExecutionState::DONE) {
+    advanceDependency();
+  }
+  return res;
+}
+
+auto UnsortedGatherExecutor::done() const noexcept -> bool {
+  return _currentDependency >= numDependencies();
+}
+
+auto UnsortedGatherExecutor::currentDependency() const noexcept -> size_t {
+  return _currentDependency;
+}
+
+auto UnsortedGatherExecutor::advanceDependency() noexcept -> void {
+  TRI_ASSERT(_currentDependency < numDependencies());
+  ++_currentDependency;
+}
+
+auto UnsortedGatherExecutor::skipRows(size_t const atMost)
+    -> std::tuple<ExecutionState, UnsortedGatherExecutor::Stats, size_t> {
+  auto const rowsLeftToSkip = [&atMost, &skipped = this->_skipped]() {
+    TRI_ASSERT(atMost >= skipped);
+    return atMost - skipped;
+  };
+  while (rowsLeftToSkip() > 0 && !done()) {
+    // Note that skipNextRow may return DONE (because the current dependency is
+    // DONE), and also return an unitialized row in that case, but we are not
+    // DONE completely - that's what `done()` is for.
+    auto [state, skipped] = skipNextRows(rowsLeftToSkip());
+    _skipped += skipped;
+    if (state == ExecutionState::WAITING) {
+      return {state, {}, 0};
+    }
+  }
+
+  auto state = done() ? ExecutionState::DONE : ExecutionState::HASMORE;
+  auto skipped = size_t{0};
+  std::swap(skipped, _skipped);
+  return {state, {}, skipped};
 }

arangod/Aql/UnsortedGatherExecutor.h

@@ -17,56 +17,58 @@
 ///
 /// Copyright holder is ArangoDB GmbH, Cologne, Germany
 ///
-/// @author Simon Grätzer
+/// @author Tobias Gödderz
 ////////////////////////////////////////////////////////////////////////////////
 
-#ifndef ARANGOD_AQL_UNSORTED_GATHER_EXECUTOR_H
+#ifndef ARANGOD_AQL_UNSORTEDGATHEREXECUTOR_H
-#define ARANGOD_AQL_UNSORTED_GATHER_EXECUTOR_H
+#define ARANGOD_AQL_UNSORTEDGATHEREXECUTOR_H
 
-#include "Aql/ClusterNodes.h"
 #include "Aql/ExecutionState.h"
 #include "Aql/ExecutorInfos.h"
-#include "Aql/InputAqlItemRow.h"
+#include "Aql/MultiDependencySingleRowFetcher.h"
-#include "Containers/SmallVector.h"
+#include "Aql/types.h"
 
-namespace arangodb {
+#include <string>
+#include <unordered_set>
 
-namespace transaction {
+namespace arangodb::aql {
-class Methods;
-}
 
-namespace aql {
-
-class MultiDependencySingleRowFetcher;
 class NoStats;
+class InputAqlItemRow;
 class OutputAqlItemRow;
-struct SortRegister;
+class IdExecutorInfos;
-
+class SharedAqlItemBlockPtr;
-class UnsortedGatherExecutorInfos : public ExecutorInfos {
- public:
-  UnsortedGatherExecutorInfos(RegisterId nrInOutRegisters, std::unordered_set<RegisterId> registersToKeep,
-                              std::unordered_set<RegisterId> registersToClear);
-  UnsortedGatherExecutorInfos() = delete;
-  UnsortedGatherExecutorInfos(UnsortedGatherExecutorInfos&&) = default;
-  UnsortedGatherExecutorInfos(UnsortedGatherExecutorInfos const&) = delete;
-  ~UnsortedGatherExecutorInfos() = default;
-};
 
+/**
+* @brief Produces all rows from its dependencies, which may be more than one,
+* in some unspecified order. It is, purposefully, strictly synchronous, and
+* always waits for an answer before requesting the next row(s). This is as
+* opposed to the ParallelUnsortedGather, which already starts fetching the next
+* dependenci(es) while waiting for an answer.
+*
+* The actual implementation fetches all available rows from the first
+* dependency, then from the second, and so forth. But that is not guaranteed.
+*/
 class UnsortedGatherExecutor {
- public:
-
  public:
   struct Properties {
-    static constexpr bool preservesOrder = true;
+    static constexpr bool preservesOrder = false;
     static constexpr BlockPassthrough allowsBlockPassthrough = BlockPassthrough::Disable;
+    // This (inputSizeRestrictsOutputSize) could be set to true, but its
+    // usefulness would be limited.
+    // We either can only use it for the last dependency, in which case it's
+    // already too late to avoid a large allocation for a small result set; or
+    // we'd have to prefetch all dependencies (at least until we got >=1000
+    // rows) before answering hasExpectedNumberOfRows(). This might be okay,
+    // but would increase the latency.
     static constexpr bool inputSizeRestrictsOutputSize = false;
   };
-
   using Fetcher = MultiDependencySingleRowFetcher;
-  using Infos = UnsortedGatherExecutorInfos;
+  // TODO I should probably implement custom Infos, we don't need distributeId().
+  using Infos = IdExecutorInfos;
   using Stats = NoStats;
 
-  UnsortedGatherExecutor(Fetcher& fetcher, Infos& info);
+  UnsortedGatherExecutor(Fetcher& fetcher, Infos&);
   ~UnsortedGatherExecutor();
 
   /**
@@ -75,29 +77,29 @@ class UnsortedGatherExecutor {
    * @return ExecutionState,
    *         if something was written output.hasValue() == true
    */
-  std::pair<ExecutionState, Stats> produceRows(OutputAqlItemRow& output);
+  [[nodiscard]] auto produceRows(OutputAqlItemRow& output)
+      -> std::pair<ExecutionState, Stats>;
+
+  [[nodiscard]] auto skipRows(size_t atMost) -> std::tuple<ExecutionState, NoStats, size_t>;
 
-  std::tuple<ExecutionState, Stats, size_t> skipRows(size_t atMost);
-  
  private:
-  
+  [[nodiscard]] auto numDependencies() const
-  void initDependencies();
+      noexcept(noexcept(static_cast<Fetcher*>(nullptr)->numberDependencies())) -> size_t;
-  
+  [[nodiscard]] auto fetcher() const noexcept -> Fetcher const&;
+  [[nodiscard]] auto fetcher() noexcept -> Fetcher&;
+  [[nodiscard]] auto done() const noexcept -> bool;
+  [[nodiscard]] auto currentDependency() const noexcept -> size_t;
+  [[nodiscard]] auto fetchNextRow(size_t atMost)
+      -> std::pair<ExecutionState, InputAqlItemRow>;
+  [[nodiscard]] auto skipNextRows(size_t atMost) -> std::pair<ExecutionState, size_t>;
+  auto advanceDependency() noexcept -> void;
+
  private:
   Fetcher& _fetcher;
-  // 64: default size of buffer; 8: Alignment size; computed to 4 but breaks in windows debug build.
+  size_t _currentDependency{0};
-  ::arangodb::containers::SmallVector<ExecutionState, 64, 8>::allocator_type::arena_type _arena;
+  size_t _skipped{0};
-  ::arangodb::containers::SmallVector<ExecutionState, 64, 8> _upstream{_arena};
-
-  // Total Number of dependencies
-  size_t _numberDependencies;
-  
-  size_t _currentDependency;
-  
-  size_t _skipped;
 };
 
-}  // namespace aql
+}  // namespace arangodb::aql
-}  // namespace arangodb
 
-#endif
+#endif  // ARANGOD_AQL_UNSORTEDGATHEREXECUTOR_H

arangod/Aql/UnsortingGatherExecutor.cpp

@@ -1,130 +0,0 @@
-////////////////////////////////////////////////////////////////////////////////
-/// DISCLAIMER
-///
-/// Copyright 2019 ArangoDB GmbH, Cologne, Germany
-///
-/// Licensed under the Apache License, Version 2.0 (the "License");
-/// you may not use this file except in compliance with the License.
-/// You may obtain a copy of the License at
-///
-///     http://www.apache.org/licenses/LICENSE-2.0
-///
-/// Unless required by applicable law or agreed to in writing, software
-/// distributed under the License is distributed on an "AS IS" BASIS,
-/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-/// See the License for the specific language governing permissions and
-/// limitations under the License.
-///
-/// Copyright holder is ArangoDB GmbH, Cologne, Germany
-///
-/// @author Tobias Gödderz
-////////////////////////////////////////////////////////////////////////////////
-
-#include "UnsortingGatherExecutor.h"
-#include <Logger/LogMacros.h>
-
-#include "Aql/IdExecutor.h"  // for IdExecutorInfos
-#include "Aql/MultiDependencySingleRowFetcher.h"
-#include "Aql/OutputAqlItemRow.h"
-#include "Aql/Stats.h"
-#include "Basics/Exceptions.h"
-#include "Basics/debugging.h"
-#include "Basics/voc-errors.h"
-
-using namespace arangodb;
-using namespace arangodb::aql;
-
-UnsortingGatherExecutor::UnsortingGatherExecutor(Fetcher& fetcher, Infos& infos)
-    : _fetcher(fetcher) {}
-
-UnsortingGatherExecutor::~UnsortingGatherExecutor() = default;
-
-auto UnsortingGatherExecutor::produceRows(OutputAqlItemRow& output)
-    -> std::pair<ExecutionState, Stats> {
-  while (!output.isFull() && !done()) {
-    // Note that fetchNextRow may return DONE (because the current dependency is
-    // DONE), and also return an unitialized row in that case, but we are not
-    // DONE completely - that's what `done()` is for.
-    auto [state, inputRow] = fetchNextRow(output.numRowsLeft());
-    if (state == ExecutionState::WAITING) {
-      return {state, {}};
-    }
-    // HASMORE => inputRow.isInitialized()
-    TRI_ASSERT(state == ExecutionState::DONE || inputRow.isInitialized());
-    if (inputRow.isInitialized()) {
-      output.copyRow(inputRow);
-      TRI_ASSERT(output.produced());
-      output.advanceRow();
-    }
-  }
-
-  auto state = done() ? ExecutionState::DONE : ExecutionState::HASMORE;
-  return {state, {}};
-}
-
-auto UnsortingGatherExecutor::fetcher() const noexcept -> const Fetcher& {
-  return _fetcher;
-}
-
-auto UnsortingGatherExecutor::fetcher() noexcept -> Fetcher& {
-  return _fetcher;
-}
-
-auto UnsortingGatherExecutor::numDependencies() const
-    noexcept(noexcept(_fetcher.numberDependencies())) -> size_t {
-  return _fetcher.numberDependencies();
-}
-
-auto UnsortingGatherExecutor::fetchNextRow(size_t atMost)
-    -> std::pair<ExecutionState, InputAqlItemRow> {
-  auto res = fetcher().fetchRowForDependency(currentDependency(), atMost);
-  if (res.first == ExecutionState::DONE) {
-    advanceDependency();
-  }
-  return res;
-}
-
-auto UnsortingGatherExecutor::skipNextRows(size_t atMost)
-    -> std::pair<ExecutionState, size_t> {
-  auto res = fetcher().skipRowsForDependency(currentDependency(), atMost);
-  if (res.first == ExecutionState::DONE) {
-    advanceDependency();
-  }
-  return res;
-}
-
-auto UnsortingGatherExecutor::done() const noexcept -> bool {
-  return _currentDependency >= numDependencies();
-}
-
-auto UnsortingGatherExecutor::currentDependency() const noexcept -> size_t {
-  return _currentDependency;
-}
-
-auto UnsortingGatherExecutor::advanceDependency() noexcept -> void {
-  TRI_ASSERT(_currentDependency < numDependencies());
-  ++_currentDependency;
-}
-
-auto UnsortingGatherExecutor::skipRows(size_t const atMost)
-    -> std::tuple<ExecutionState, UnsortingGatherExecutor::Stats, size_t> {
-  auto const rowsLeftToSkip = [&atMost, &skipped = this->_skipped]() {
-    TRI_ASSERT(atMost >= skipped);
-    return atMost - skipped;
-  };
-  while (rowsLeftToSkip() > 0 && !done()) {
-    // Note that skipNextRow may return DONE (because the current dependency is
-    // DONE), and also return an unitialized row in that case, but we are not
-    // DONE completely - that's what `done()` is for.
-    auto [state, skipped] = skipNextRows(rowsLeftToSkip());
-    _skipped += skipped;
-    if (state == ExecutionState::WAITING) {
-      return {state, {}, 0};
-    }
-  }
-
-  auto state = done() ? ExecutionState::DONE : ExecutionState::HASMORE;
-  auto skipped = size_t{0};
-  std::swap(skipped, _skipped);
-  return {state, {}, skipped};
-}

arangod/Aql/UnsortingGatherExecutor.h

@@ -1,98 +0,0 @@
-////////////////////////////////////////////////////////////////////////////////
-/// DISCLAIMER
-///
-/// Copyright 2019 ArangoDB GmbH, Cologne, Germany
-///
-/// Licensed under the Apache License, Version 2.0 (the "License");
-/// you may not use this file except in compliance with the License.
-/// You may obtain a copy of the License at
-///
-///     http://www.apache.org/licenses/LICENSE-2.0
-///
-/// Unless required by applicable law or agreed to in writing, software
-/// distributed under the License is distributed on an "AS IS" BASIS,
-/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-/// See the License for the specific language governing permissions and
-/// limitations under the License.
-///
-/// Copyright holder is ArangoDB GmbH, Cologne, Germany
-///
-/// @author Tobias Gödderz
-////////////////////////////////////////////////////////////////////////////////
-
-#ifndef ARANGOD_AQL_UNSORTINGGATHEREXECUTOR_H
-#define ARANGOD_AQL_UNSORTINGGATHEREXECUTOR_H
-
-#include "Aql/ExecutionState.h"
-#include "Aql/ExecutorInfos.h"
-#include "Aql/MultiDependencySingleRowFetcher.h"
-#include "Aql/types.h"
-
-#include <string>
-#include <unordered_set>
-
-namespace arangodb::aql {
-
-class NoStats;
-class InputAqlItemRow;
-class OutputAqlItemRow;
-class IdExecutorInfos;
-class SharedAqlItemBlockPtr;
-
-/**
-* @brief Produces all rows from its dependencies, which may be more than one,
-* in some unspecified order. It is, purposefully, strictly synchronous, and
-* always waits for an answer before requesting the next row(s).
-*
-* The actual implementation fetches all available rows from the first
-* dependency, then from the second, and so forth. But that is not guaranteed.
-*/
-class UnsortingGatherExecutor {
- public:
-  struct Properties {
-    static constexpr bool preservesOrder = false;
-    static constexpr BlockPassthrough allowsBlockPassthrough = BlockPassthrough::Disable;
-    // TODO I think we can set this to true (but needs to implement
-    //  hasExpectedNumberOfRows for that)
-    static constexpr bool inputSizeRestrictsOutputSize = false;
-  };
-  using Fetcher = MultiDependencySingleRowFetcher;
-  // TODO I should probably implement custom Infos, we don't need distributeId().
-  using Infos = IdExecutorInfos;
-  using Stats = NoStats;
-
-  UnsortingGatherExecutor(Fetcher& fetcher, Infos&);
-  ~UnsortingGatherExecutor();
-
-  /**
-   * @brief produce the next Row of Aql Values.
-   *
-   * @return ExecutionState,
-   *         if something was written output.hasValue() == true
-   */
-  [[nodiscard]] auto produceRows(OutputAqlItemRow& output)
-      -> std::pair<ExecutionState, Stats>;
-
-  [[nodiscard]] auto skipRows(size_t atMost) -> std::tuple<ExecutionState, NoStats, size_t>;
-
- private:
-  [[nodiscard]] auto numDependencies() const
-      noexcept(noexcept(static_cast<Fetcher*>(nullptr)->numberDependencies())) -> size_t;
-  [[nodiscard]] auto fetcher() const noexcept -> Fetcher const&;
-  [[nodiscard]] auto fetcher() noexcept -> Fetcher&;
-  [[nodiscard]] auto done() const noexcept -> bool;
-  [[nodiscard]] auto currentDependency() const noexcept -> size_t;
-  [[nodiscard]] auto fetchNextRow(size_t atMost)
-      -> std::pair<ExecutionState, InputAqlItemRow>;
-  [[nodiscard]] auto skipNextRows(size_t atMost) -> std::pair<ExecutionState, size_t>;
-  auto advanceDependency() noexcept -> void;
-
- private:
-  Fetcher& _fetcher;
-  size_t _currentDependency{0};
-  size_t _skipped{0};
-};
-
-}  // namespace arangodb::aql
-
-#endif  // ARANGOD_AQL_UNSORTINGGATHEREXECUTOR_H

arangod/CMakeLists.txt

@@ -350,8 +350,8 @@ set(LIB_ARANGO_AQL_SOURCES
   Aql/TraversalConditionFinder.cpp
   Aql/TraversalExecutor.cpp
   Aql/TraversalNode.cpp
+  Aql/ParallelUnsortedGatherExecutor.cpp
   Aql/UnsortedGatherExecutor.cpp
-  Aql/UnsortingGatherExecutor.cpp
   Aql/UpdateReplaceModifier.cpp
   Aql/UpsertModifier.cpp
   Aql/V8Executor.cpp
@@ -1008,7 +1008,7 @@ foreach(TARGET
     if (MSVC)
       target_compile_options(${TARGET} PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
     else ()
-      target_compile_options(${TARGET} PRIVATE -Werror)
+      target_compile_options(${TARGET} PRIVATE -Werror -Wno-error=deprecated-declarations)
     endif ()
   endif ()
 endforeach()

arangosh/CMakeLists.txt

@@ -80,9 +80,9 @@ endif ()
 
 if (USE_FAIL_ON_WARNINGS)
  if (MSVC)
-   target_compile_options(arangobench PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
+    target_compile_options(arangobench PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
  else ()
-   target_compile_options(arangobench PRIVATE -Werror)
+   target_compile_options(arangobench PRIVATE -Werror -Wno-error=deprecated-declarations)
  endif ()
 endif ()
 
@@ -139,9 +139,9 @@ endif ()
 
 if (USE_FAIL_ON_WARNINGS)
  if (MSVC)
-   target_compile_options(arangobackup PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
+    target_compile_options(arangobackup PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
  else ()
-   target_compile_options(arangobackup PRIVATE -Werror)
+   target_compile_options(arangobackup PRIVATE -Werror -Wno-error=deprecated-declarations)
  endif ()
 endif ()
 
@@ -198,9 +198,9 @@ endif ()
 
 if (USE_FAIL_ON_WARNINGS)
  if (MSVC)
-   target_compile_options(arangodump PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
+    target_compile_options(arangodump PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
  else ()
-   target_compile_options(arangodump PRIVATE -Werror)
+   target_compile_options(arangodump PRIVATE -Werror -Wno-error=deprecated-declarations)
  endif ()
 endif ()
 
@@ -254,9 +254,9 @@ endif ()
 
 if (USE_FAIL_ON_WARNINGS)
  if (MSVC)
-   target_compile_options(arangoexport PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
+    target_compile_options(arangoexport PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
  else ()
-   target_compile_options(arangoexport PRIVATE -Werror)
+   target_compile_options(arangoexport PRIVATE -Werror -Wno-error=deprecated-declarations)
  endif ()
 endif ()
 
@@ -322,9 +322,9 @@ install_command_alias(arangoimport
 
 if (USE_FAIL_ON_WARNINGS)
  if (MSVC)
-   target_compile_options(arangoimport PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
+    target_compile_options(arangoimport PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
  else ()
-   target_compile_options(arangoimport PRIVATE -Werror)
+   target_compile_options(arangoimport PRIVATE -Werror -Wno-error=deprecated-declarations)
  endif ()
 endif ()
 
@@ -379,9 +379,9 @@ endif ()
 
 if (USE_FAIL_ON_WARNINGS)
  if (MSVC)
-   target_compile_options(arangorestore PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
+    target_compile_options(arangorestore PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
  else ()
-   target_compile_options(arangorestore PRIVATE -Werror)
+   target_compile_options(arangorestore PRIVATE -Werror -Wno-error=deprecated-declarations)
  endif ()
 endif ()
 
@@ -444,9 +444,9 @@ endif ()
 
 if (USE_FAIL_ON_WARNINGS)
  if (MSVC)
-   target_compile_options(arangosh PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
+    target_compile_options(arangosh PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
  else ()
-   target_compile_options(arangosh PRIVATE -Werror)
+   target_compile_options(arangosh PRIVATE -Werror -Wno-error=deprecated-declarations)
  endif ()
 endif ()
 
@@ -500,9 +500,9 @@ endif ()
 
 if (USE_FAIL_ON_WARNINGS)
  if (MSVC)
-   target_compile_options(arangovpack PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
+    target_compile_options(arangovpack PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
  else ()
-   target_compile_options(arangovpack PRIVATE -Werror)
+   target_compile_options(arangovpack PRIVATE -Werror -Wno-error=deprecated-declarations)
  endif ()
 endif ()
 

lib/CMakeLists.txt

@@ -309,11 +309,11 @@ endif()
 if (USE_FAIL_ON_WARNINGS)
   if (MSVC)
     target_compile_options(arango_v8 PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
-    target_compile_options(arango_geo PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
+    target_compile_options(arango_geo PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS96)
     target_compile_options(arango PRIVATE /WX /D_WINSOCK_DEPRECATED_NO_WARNINGS)
   else ()
-    target_compile_options(arango_v8 PRIVATE -Werror)
+    target_compile_options(arango_v8 PRIVATE -Werror -Wno-error=deprecated-declarations)
-    target_compile_options(arango_geo PRIVATE -Werror)
+    target_compile_options(arango_geo PRIVATE -Werror -Wno-error=deprecated-declarations)
-    target_compile_options(arango PRIVATE -Werror)
+    target_compile_options(arango PRIVATE -Werror -Wno-error=deprecated-declarations)
   endif ()
 endif ()

tests/Aql/DependencyProxyMock.cpp

@@ -26,9 +26,7 @@
 
 #include <velocypack/Options.h>
 
-namespace arangodb {
+namespace arangodb::tests::aql {
-namespace tests {
-namespace aql {
 
 using namespace arangodb::aql;
 
@@ -216,9 +214,7 @@ size_t MultiDependencyProxyMock<passBlocksThrough>::numFetchBlockCalls() const {
   return res;
 }
 
-}  // namespace aql
+}  // namespace arangodb::tests::aql
-}  // namespace tests
-}  // namespace arangodb
 
 template class ::arangodb::tests::aql::DependencyProxyMock<::arangodb::aql::BlockPassthrough::Enable>;
 template class ::arangodb::tests::aql::DependencyProxyMock<::arangodb::aql::BlockPassthrough::Disable>;

tests/Aql/ExecutionBlockImplTest.cpp

@@ -154,7 +154,7 @@ TEST_F(ExecutionBlockImplTest,
 }
 
 TEST_F(ExecutionBlockImplTest,
-       there_is_a_block_in_the_upstream_with_now_rows_inside_the_executor_waits_using_skipsome) {
+       there_is_a_block_in_the_upstream_with_no_rows_inside_the_executor_waits_using_skipsome) {
   std::deque<SharedAqlItemBlockPtr> blockDeque;
   SharedAqlItemBlockPtr block = buildBlock<1>(itemBlockManager, {{42}});
   blockDeque.push_back(std::move(block));

tests/Aql/IdExecutorTest.cpp

@@ -32,15 +32,12 @@
 #include "Aql/ResourceUsage.h"
 #include "Aql/Stats.h"
 
-#include <velocypack/Builder.h>
 #include <velocypack/velocypack-aliases.h>
 
 using namespace arangodb;
 using namespace arangodb::aql;
 
-namespace arangodb {
+namespace arangodb::tests::aql {
-namespace tests {
-namespace aql {
 
 class IdExecutorTest : public ::testing::Test {
  protected:
@@ -66,7 +63,7 @@ class IdExecutorTest : public ::testing::Test {
 
 TEST_F(IdExecutorTest, there_are_no_rows_upstream) {
   ConstFetcherHelper fetcher(itemBlockManager, nullptr);
-  IdExecutor<::arangodb::aql::BlockPassthrough::Enable, ConstFetcher> testee(fetcher, infos);
+  IdExecutor<ConstFetcher> testee(fetcher, infos);
   NoStats stats{};
 
   std::tie(state, stats) = testee.produceRows(row);
@@ -77,7 +74,7 @@ TEST_F(IdExecutorTest, there_are_no_rows_upstream) {
 TEST_F(IdExecutorTest, there_are_rows_in_the_upstream) {
   auto input = VPackParser::fromJson("[ [true], [false], [true] ]");
   ConstFetcherHelper fetcher(itemBlockManager, input->buffer());
-  IdExecutor<::arangodb::aql::BlockPassthrough::Enable, ConstFetcher> testee(fetcher, infos);
+  IdExecutor<ConstFetcher> testee(fetcher, infos);
   NoStats stats{};
 
   // This block consumes all rows at once.
@@ -98,6 +95,4 @@ TEST_F(IdExecutorTest, there_are_rows_in_the_upstream) {
   }
 }
 
-}  // namespace aql
-}  // namespace tests
 }  // namespace arangodb

tests/Aql/SortExecutorTest.cpp

@@ -50,9 +50,7 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-namespace arangodb {
+namespace arangodb::tests::aql {
-namespace tests {
-namespace aql {
 
 class SortExecutorTest : public ::testing::Test {
  protected:
@@ -198,6 +196,4 @@ TEST_F(SortExecutorTest, rows_upstream_we_are_waiting_for_list_of_numbers) {
   ASSERT_EQ(number, 5);
 }
 
-}  // namespace aql
+}  // namespace arangodb::tests::aql
-}  // namespace tests
-}  // namespace arangodb

tests/Aql/WaitingExecutionBlockMock.cpp

@@ -25,8 +25,6 @@
 #include "Aql/AqlItemBlock.h"
 #include "Aql/ExecutionEngine.h"
 #include "Aql/ExecutionState.h"
-#include "Aql/ExecutionStats.h"
-#include "Aql/ExecutorInfos.h"
 #include "Aql/QueryOptions.h"
 
 #include <velocypack/velocypack-aliases.h>
@@ -92,24 +90,20 @@ std::pair<arangodb::aql::ExecutionState, size_t> WaitingExecutionBlockMock::skip
   traceSkipSomeBegin(atMost);
   if (!_hasWaited) {
     _hasWaited = true;
-    traceSkipSomeEnd(ExecutionState::WAITING, 0);
+    return traceSkipSomeEnd(ExecutionState::WAITING, 0);
-    return {ExecutionState::WAITING, 0};
   }
   _hasWaited = false;
 
   if (_data.empty()) {
-    traceSkipSomeEnd(ExecutionState::DONE, 0);
+    return traceSkipSomeEnd(ExecutionState::DONE, 0);
-    return {ExecutionState::DONE, 0};
   }
 
   size_t skipped = _data.front()->size();
   _data.pop_front();
 
   if (_data.empty()) {
-    traceSkipSomeEnd(ExecutionState::DONE, skipped);
+    return traceSkipSomeEnd(ExecutionState::DONE, skipped);
-    return {ExecutionState::DONE, skipped};
   } else {
-    traceSkipSomeEnd(ExecutionState::HASMORE, skipped);
+    return traceSkipSomeEnd(ExecutionState::HASMORE, skipped);
-    return {ExecutionState::HASMORE, skipped};
   }
 }

tests/CMakeLists.txt

@@ -281,7 +281,7 @@ endif()
 
 if (DARWIN AND FAIL_ON_WARNINGS)
   # Missing Braces is broken in older CLangs - disable them here.
-  target_compile_options(arangodbtests PRIVATE -Werror -Wno-missing-braces)
+  target_compile_options(arangodbtests PRIVATE -Werror -Wno-error=deprecated-declarations -Wno-missing-braces)
 endif()
 
 # add these includes as system includes because otherwise