Merge branch 'feature/AqlSubqueryExecutionBlockImplExecuteImplementat…

…ion' of ssh://github.com/arangodb/ArangoDB into feature/AqlSubqueryExecutionBlockImplExecuteImplementation-bypass-skip
arangodb · mchacki · Mar 12, 2020 · Feb 29, 2020 · Feb 29, 2020 · Feb 29, 2020
commit f0bca7c7c211ffb85b574276dc5953dd0fb2c44e
diff --git a/arangod/Aql/AqlCallStack.cpp b/arangod/Aql/AqlCallStack.cpp
@@ -99,6 +99,12 @@ void AqlCallStack::pushCall(AqlCall&& call) {
   _operations.emplace_back(std::move(call));
 }
 
+void AqlCallStack::pushCall(AqlCall const& call) {
+  // TODO is this correct on subqueries?
+  TRI_ASSERT(isRelevant());
+  _operations.emplace_back(call);
+}
+
 void AqlCallStack::pop() {
   if (isRelevant()) {
     // We have one element to pop

diff --git a/arangod/Aql/AqlCallStack.h b/arangod/Aql/AqlCallStack.h
@@ -63,6 +63,9 @@ class AqlCallStack {
   // Put another call on top of the stack.
   void pushCall(AqlCall&& call);
 
+  // Put another call on top of the stack.
+  void pushCall(AqlCall const& call);
+
   // Pops one subquery level.
   // if this isRelevent it pops the top-most call from the stack.
   // if this is not revelent it reduces the depth by 1.

diff --git a/arangod/Aql/ExecutionBlockImpl.cpp b/arangod/Aql/ExecutionBlockImpl.cpp
@@ -606,6 +606,13 @@ std::pair<ExecutionState, Result> ExecutionBlockImpl<Executor>::initializeCursor
   _rowFetcher.~Fetcher();
   new (&_rowFetcher) Fetcher(_dependencyProxy);
 
+  if constexpr (isMultiDepExecutor<Executor>) {
+    _lastRange.reset();
+    _rowFetcher.init();
+  } else {
+    _lastRange = DataRange(ExecutorState::HASMORE);
+  }
+
   TRI_ASSERT(_skipped.nothingSkipped());
   _skipped.reset();
   TRI_ASSERT(_state == InternalState::DONE || _state == InternalState::FETCH_DATA);
@@ -1210,42 +1217,28 @@ static auto fastForwardType(AqlCall const& call, Executor const& e) -> FastForwa
 }
 
 template <class Executor>
-auto ExecutionBlockImpl<Executor>::executeFetcher(AqlCallStack& stack, size_t const dependency)
+auto ExecutionBlockImpl<Executor>::executeFetcher(AqlCallStack& stack, AqlCallType const& aqlCall)
     -> std::tuple<ExecutionState, SkipResult, typename Fetcher::DataRange> {
-  // Silence compiler about unused dependency
-  (void)dependency;
   if constexpr (isNewStyleExecutor<Executor>) {
-    if constexpr (is_one_of_v<Fetcher, MultiDependencySingleRowFetcher>) {
+    // TODO The logic in the MultiDependencySingleRowFetcher branch should be
+    //      moved into the MultiDependencySingleRowFetcher.
+    static_assert(isMultiDepExecutor<Executor> ==
+                  std::is_same_v<Fetcher, MultiDependencySingleRowFetcher>);
+    if constexpr (std::is_same_v<Fetcher, MultiDependencySingleRowFetcher>) {
       static_assert(
           !executorHasSideEffects<Executor>,
           "there is a special implementation for side-effect executors to "
           "exchange the stack. For the MultiDependencyFetcher this special "
           "case is not implemented. There is no reason to disallow this "
           "case here however, it is just not needed thus far.");
-      // TODO: This is a hack to guarantee we have enough space in our range
-      //       to fit all inputs, in particular the one executed below
-      TRI_ASSERT(dependency < _dependencies.size());
-      _lastRange.resizeIfNecessary(ExecutorState::HASMORE, 0, _dependencies.size());
-
-      if constexpr (!isParallelExecutor) {
-        auto [state, skipped, range] = _rowFetcher.executeForDependency(dependency, stack);
+      // Note the aqlCall is an AqlCallSet in this case:
+      static_assert(std::is_same_v<AqlCallSet, std::decay_t<decltype(aqlCall)>>);
+      TRI_ASSERT(_lastRange.numberDependencies() == _dependencies.size());
+      auto const& [state, skipped, ranges] = _rowFetcher.execute(stack, aqlCall);
+      for (auto const& [dependency, range] : ranges) {
         _lastRange.setDependency(dependency, range);
-        return {state, skipped, _lastRange};
-      } else {
-        _callsInFlight.resize(_dependencyProxy.numberDependencies());
-        if (!_callsInFlight[dependency].has_value()) {
-          _callsInFlight[dependency] = stack;
-        }
-        TRI_ASSERT(_callsInFlight[dependency].has_value());
-        auto [state, skipped, range] =
-            _rowFetcher.executeForDependency(dependency,
-                                             _callsInFlight[dependency].value());
-        if (state != ExecutionState::WAITING) {
-          _callsInFlight[dependency] = std::nullopt;
-        }
-        _lastRange.setDependency(dependency, range);
-        return {state, skipped, _lastRange};
       }
+      return {state, skipped, _lastRange};
     } else if constexpr (executorHasSideEffects<Executor>) {
       // If the executor has side effects, we cannot bypass any subqueries
       // by skipping them. SO we need to fetch all shadow rows in order to
@@ -1646,17 +1639,16 @@ auto ExecutionBlockImpl<Executor>::executeFastForward(typename Fetcher::DataRang
       AqlCall dummy;
       dummy.hardLimit = 0;
       dummy.fullCount = true;
-      auto [state, stats, skippedLocal, call, dependency] =
-          executeSkipRowsRange(_lastRange, dummy);
-      _requestedDependency = dependency;
+      auto [state, stats, skippedLocal, call] = executeSkipRowsRange(_lastRange, dummy);
+
       if constexpr (is_one_of_v<DataRange, AqlItemBlockInputMatrix, MultiAqlItemBlockInputRange>) {
         // The executor will have used all Rows.
         // However we need to drop them from the input
         // here.
         inputRange.skipAllRemainingDataRows();
       }
 
-      return {state, stats, 0, call, dependency};
+      return {state, stats, 0, call};
     }
     case FastForwardVariant::FETCHER: {
       LOG_QUERY("fa327", DEBUG) << printTypeInfo() << " bypass unused rows.";
@@ -1667,8 +1659,8 @@ auto ExecutionBlockImpl<Executor>::executeFastForward(typename Fetcher::DataRang
           return fastForwardCall;
         } else {
 #ifndef _WIN32
-          // For some reason our Windows compiler complains about this static assert
-          // in the cases that should be in the above constexpr path.
+          // For some reason our Windows compiler complains about this static
+          // assert in the cases that should be in the above constexpr path.
           // So simply not compile it in.
           static_assert(std::is_same_v<AqlCallType, AqlCallSet>);
 #endif
@@ -1967,15 +1959,18 @@ ExecutionBlockImpl<Executor>::executeWithoutTrace(AqlCallStack stack) {
         case ExecState::FASTFORWARD: {
           LOG_QUERY("96e2c", DEBUG)
               << printTypeInfo() << " all produced, fast forward to end up (sub-)query.";
+
           AqlCall callCopy = clientCall;
           if constexpr (executorHasSideEffects<Executor>) {
             if (stack.needToSkipSubquery()) {
               // Fast Forward call.
               callCopy = AqlCall{0, false, 0, AqlCall::LimitType::HARD};
             }
           }
-          auto [state, stats, skippedLocal, call, dependency] =
+
+          auto [state, stats, skippedLocal, call] =
               executeFastForward(_lastRange, callCopy);
+
           if constexpr (executorHasSideEffects<Executor>) {
             if (!stack.needToSkipSubquery()) {
               // We need to modify the original call.
@@ -1987,7 +1982,6 @@ ExecutionBlockImpl<Executor>::executeWithoutTrace(AqlCallStack stack) {
             clientCall = callCopy;
           }
 
-          _requestedDependency = dependency;
           _skipped.didSkip(skippedLocal);
           _engine->_stats += stats;
           localExecutorState = state;
@@ -2021,16 +2015,7 @@ ExecutionBlockImpl<Executor>::executeWithoutTrace(AqlCallStack stack) {
           auto subqueryLevelBefore = stack.subqueryLevel();
 #endif
           std::tie(_upstreamState, skippedLocal, _lastRange) =
-              executeFetcher(stack, _requestedDependency);
-          if constexpr (std::is_same_v<Executor, SubqueryStartExecutor>) {
-            // Do not pop the call, we did not put it on.
-            // However we need it for accounting later.
-          } else {
-            // As the stack is copied into the fetcher, we need to pop off our call again.
-            // If we use other datastructures or moving we may hand over ownership of the stack here
-            // instead and no popCall is necessary.
-            stack.popCall();
-          }
+              executeFetcher(stack, _upstreamRequest);
 
 #ifdef ARANGODB_ENABLE_MAINTAINER_MODE
           TRI_ASSERT(subqueryLevelBefore == stack.subqueryLevel());

diff --git a/arangod/Aql/ExecutionBlockImpl.h b/arangod/Aql/ExecutionBlockImpl.h
@@ -54,6 +54,7 @@ class Query;
 class ShadowAqlItemRow;
 class SkipResult;
 class ParallelUnsortedGatherExecutor;
+class MultiDependencySingleRowFetcher;
 
 template <typename T, typename... Es>
 constexpr bool is_one_of_v = (std::is_same_v<T, Es> || ...);
@@ -257,7 +258,7 @@ class ExecutionBlockImpl final : public ExecutionBlock {
   std::tuple<ExecutionState, SkipResult, SharedAqlItemBlockPtr> executeWithoutTrace(AqlCallStack stack);
 
   std::tuple<ExecutionState, SkipResult, typename Fetcher::DataRange> executeFetcher(
-      AqlCallStack& stack, size_t const dependency);
+      AqlCallStack& stack, AqlCallType const& aqlCall);
 
   std::tuple<ExecutorState, typename Executor::Stats, AqlCallType> executeProduceRows(
       typename Fetcher::DataRange& input, OutputAqlItemRow& output);
@@ -350,6 +351,10 @@ class ExecutionBlockImpl final : public ExecutionBlock {
                                               DataRange const& range) const
       noexcept -> ExecState;
 
+  void initOnce();
+
+  [[nodiscard]] auto executorNeedsCall(AqlCallType& call) const noexcept -> bool;
+
  private:
   /**
    * @brief Used to allow the row Fetcher to access selected methods of this

diff --git a/arangod/Aql/MultiDependencySingleRowFetcher.cpp b/arangod/Aql/MultiDependencySingleRowFetcher.cpp
@@ -371,9 +371,6 @@ auto MultiDependencySingleRowFetcher::useStack(AqlCallStack const& stack) -> voi
 auto MultiDependencySingleRowFetcher::executeForDependency(size_t const dependency,
                                                            AqlCallStack& stack)
     -> std::tuple<ExecutionState, SkipResult, AqlItemBlockInputRange> {
-  if (_dependencyStates.empty()) {
-    initDependencies();
-  }
   auto [state, skipped, block] = _dependencyProxy->executeForDependency(dependency, stack);
 
   if (state == ExecutionState::WAITING) {
@@ -395,7 +392,7 @@ auto MultiDependencySingleRowFetcher::executeForDependency(size_t const dependen
 
 auto MultiDependencySingleRowFetcher::execute(AqlCallStack const& stack,
                                               AqlCallSet const& aqlCallSet)
-    -> std::tuple<ExecutionState, size_t, std::vector<std::pair<size_t, AqlItemBlockInputRange>>> {
+    -> std::tuple<ExecutionState, SkipResult, std::vector<std::pair<size_t, AqlItemBlockInputRange>>> {
   TRI_ASSERT(_callsInFlight.size() == numberDependencies());
 
   auto ranges = std::vector<std::pair<size_t, AqlItemBlockInputRange>>{};
@@ -404,7 +401,7 @@ auto MultiDependencySingleRowFetcher::execute(AqlCallStack const& stack,
   auto depCallIdx = size_t{0};
   auto allAskedDepsAreWaiting = true;
   auto askedAtLeastOneDep = false;
-  auto skippedTotal = size_t{0};
+  auto skippedTotal = SkipResult{};
   // Iterate in parallel over `_callsInFlight` and `aqlCall.calls`.
   // _callsInFlight[i] corresponds to aqlCalls.calls[k] iff
   // aqlCalls.calls[k].dependency = i.
@@ -440,7 +437,7 @@ auto MultiDependencySingleRowFetcher::execute(AqlCallStack const& stack,
         maybeCallInFlight = std::nullopt;
         allAskedDepsAreWaiting = false;
       } else {
-        TRI_ASSERT(skipped == 0);
+        TRI_ASSERT(skipped.nothingSkipped());
       }
       skippedTotal += skipped;
       ranges.emplace_back(dependency, range);

diff --git a/arangod/Aql/MultiDependencySingleRowFetcher.h b/arangod/Aql/MultiDependencySingleRowFetcher.h
@@ -137,7 +137,10 @@ class MultiDependencySingleRowFetcher {
   //@deprecated
   auto useStack(AqlCallStack const& stack) -> void;
 
-  auto executeForDependency(size_t const dependency, AqlCallStack& stack)
+  [[nodiscard]] auto execute(AqlCallStack const&, AqlCallSet const&)
+      -> std::tuple<ExecutionState, SkipResult, std::vector<std::pair<size_t, AqlItemBlockInputRange>>>;
+
+  [[nodiscard]] auto executeForDependency(size_t dependency, AqlCallStack& stack)
       -> std::tuple<ExecutionState, SkipResult, AqlItemBlockInputRange>;
 
   [[nodiscard]] auto upstreamState() const -> ExecutionState;