Feature/aql subquery splicing with gather (#10341)

Allow Splicing with GATHER nodes

arangod/Aql/Aggregator.cpp

@@ -24,6 +24,7 @@
 #include "Aggregator.h"
 
 #include "Aql/AqlValue.h"
+#include "Aql/AqlValueMaterializer.h"
 #include "Basics/VelocyPackHelper.h"
 #include "Transaction/Context.h"
 #include "Transaction/Helpers.h"

arangod/Aql/AqlItemBlock.cpp

@@ -33,6 +33,8 @@
 #include "Aql/SharedAqlItemBlockPtr.h"
 #include "Basics/StaticStrings.h"
 #include "Basics/VelocyPackHelper.h"
+#include "Transaction/Context.h"
+#include "Transaction/Methods.h"
 
 #include <velocypack/Iterator.h>
 #include <velocypack/velocypack-aliases.h>
@@ -566,7 +568,7 @@ SharedAqlItemBlockPtr AqlItemBlock::steal(std::vector<size_t> const& chosen,
 ///                  corresponding position
 ///  "raw":     List of actual values, positions 0 and 1 are always null
 ///                  such that actual indices start at 2
-void AqlItemBlock::toVelocyPack(transaction::Methods* trx, VPackBuilder& result) const {
+void AqlItemBlock::toVelocyPack(velocypack::Options const* const trxOptions, VPackBuilder& result) const {
   TRI_ASSERT(result.isOpenObject());
   VPackOptions options(VPackOptions::Defaults);
   options.buildUnindexedArrays = true;
@@ -650,7 +652,7 @@ void AqlItemBlock::toVelocyPack(transaction::Methods* trx, VPackBuilder& result)
 
         if (it == table.end()) {
           currentState = Next;
-          a.toVelocyPack(trx, raw, false);
+          a.toVelocyPack(trxOptions, raw, false);
           table.try_emplace(a, pos++);
         } else {
           currentState = Positional;
@@ -699,20 +701,21 @@ void AqlItemBlock::toVelocyPack(transaction::Methods* trx, VPackBuilder& result)
   result.add("raw", raw.slice());
 }
 
-void AqlItemBlock::rowToSimpleVPack(size_t const row, transaction::Methods* trx, arangodb::velocypack::Builder& builder) const {
+void AqlItemBlock::rowToSimpleVPack(size_t const row, velocypack::Options const* options, arangodb::velocypack::Builder& builder) const {
   VPackArrayBuilder rowBuilder{&builder};
 
   if (isShadowRow(row)) {
-    getShadowRowDepth(row).toVelocyPack(trx, *rowBuilder, false);
+    getShadowRowDepth(row).toVelocyPack(options, *rowBuilder, false);
   } else {
-    AqlValue{AqlValueHintNull{}}.toVelocyPack(trx, *rowBuilder, false);
+    AqlValue{AqlValueHintNull{}}.toVelocyPack(options, *rowBuilder, false);
   }
   for (RegisterId reg = 0; reg < getNrRegs(); ++reg) {
-    getValueReference(row, reg).toVelocyPack(trx, *rowBuilder, false);
+    getValueReference(row, reg).toVelocyPack(options, *rowBuilder, false);
   }
 }
 
-void AqlItemBlock::toSimpleVPack(transaction::Methods* trx, arangodb::velocypack::Builder& builder) const {
+void AqlItemBlock::toSimpleVPack(velocypack::Options const* options,
+                                 arangodb::velocypack::Builder& builder) const {
   VPackObjectBuilder block{&builder};
   block->add("nrItems", VPackValue(size()));
   block->add("nrRegs", VPackValue(getNrRegs()));
@@ -720,7 +723,7 @@ void AqlItemBlock::toSimpleVPack(transaction::Methods* trx, arangodb::velocypack
   {
     VPackArrayBuilder matrixBuilder{block.builder};
     for (size_t row = 0; row < size(); ++row) {
-      rowToSimpleVPack(row, trx, *matrixBuilder.builder);
+      rowToSimpleVPack(row, options, *matrixBuilder.builder);
     }
   }
 }

arangod/Aql/AqlItemBlock.h

@@ -208,7 +208,7 @@ class AqlItemBlock {
 
   /// @brief toJson, transfer a whole AqlItemBlock to Json, the result can
   /// be used to recreate the AqlItemBlock via the Json constructor
-  void toVelocyPack(transaction::Methods* trx, arangodb::velocypack::Builder&) const;
+  void toVelocyPack(velocypack::Options const*, arangodb::velocypack::Builder&) const;
 
   /// @brief Creates a human-readable velocypack of the block. Adds an object
   /// `{nrItems, nrRegs, matrix}` to the builder.
@@ -217,9 +217,9 @@ class AqlItemBlock {
   // (of length nrRegs+1 (sic)). The first entry contains the shadow row depth,
   // or `null` for data rows. The entries with indexes 1..nrRegs contain the
   // registers 0..nrRegs-1, respectively.
-  void toSimpleVPack(transaction::Methods* trx, arangodb::velocypack::Builder&) const;
+  void toSimpleVPack(velocypack::Options const*, arangodb::velocypack::Builder&) const;
 
-  void rowToSimpleVPack(size_t row, transaction::Methods* trx,
+  void rowToSimpleVPack(size_t row, velocypack::Options const*,
                         velocypack::Builder& builder) const;
 
   /// @brief test if the given row is a shadow row and conveys subquery

arangod/Aql/AqlValue.cpp

@@ -40,8 +40,6 @@
 #include <velocypack/StringRef.h>
 #include <velocypack/velocypack-aliases.h>
 
-#include <array>
-
 using namespace arangodb;
 using namespace arangodb::aql;
 
@@ -933,7 +931,7 @@ v8::Handle<v8::Value> AqlValue::toV8(v8::Isolate* isolate, transaction::Methods*
 }
 
 /// @brief materializes a value into the builder
-void AqlValue::toVelocyPack(transaction::Methods* trx, arangodb::velocypack::Builder& builder,
+void AqlValue::toVelocyPack(VPackOptions const* options, arangodb::velocypack::Builder& builder,
                             bool resolveExternals) const {
   switch (type()) {
     case VPACK_SLICE_POINTER:
@@ -949,7 +947,7 @@ void AqlValue::toVelocyPack(transaction::Methods* trx, arangodb::velocypack::Bui
         bool const sanitizeCustom = true;
         arangodb::basics::VelocyPackHelper::sanitizeNonClientTypes(
             slice(), VPackSlice::noneSlice(), builder,
-            trx->transactionContextPtr()->getVPackOptions(), sanitizeExternals,
+            options, sanitizeExternals,
             sanitizeCustom);
       } else {
         builder.add(slice());
@@ -961,7 +959,7 @@ void AqlValue::toVelocyPack(transaction::Methods* trx, arangodb::velocypack::Bui
       for (auto const& it : *_data.docvec) {
         size_t const n = it->size();
         for (size_t i = 0; i < n; ++i) {
-          it->getValueReference(i, 0).toVelocyPack(trx, builder, resolveExternals);
+          it->getValueReference(i, 0).toVelocyPack(options, builder, resolveExternals);
         }
       }
       builder.close();
@@ -980,8 +978,13 @@ void AqlValue::toVelocyPack(transaction::Methods* trx, arangodb::velocypack::Bui
   }
 }
 
+void AqlValue::toVelocyPack(transaction::Methods* trx, arangodb::velocypack::Builder& builder,
+                            bool resolveExternals) const {
+  toVelocyPack(trx->transactionContextPtr()->getVPackOptions(), builder, resolveExternals);
+}
+
 /// @brief materializes a value into the builder
-AqlValue AqlValue::materialize(transaction::Methods* trx, bool& hasCopied,
+AqlValue AqlValue::materialize(VPackOptions const* options, bool& hasCopied,
                                bool resolveExternals) const {
   switch (type()) {
     case VPACK_INLINE:
@@ -997,7 +1000,7 @@ AqlValue AqlValue::materialize(transaction::Methods* trx, bool& hasCopied,
       ConditionalDeleter<VPackBuffer<uint8_t>> deleter(shouldDelete);
       std::shared_ptr<VPackBuffer<uint8_t>> buffer(new VPackBuffer<uint8_t>, deleter);
       VPackBuilder builder(buffer);
-      toVelocyPack(trx, builder, resolveExternals);
+      toVelocyPack(options, builder, resolveExternals);
       hasCopied = true;
       return AqlValue(buffer.get(), shouldDelete);
     }
@@ -1008,6 +1011,11 @@ AqlValue AqlValue::materialize(transaction::Methods* trx, bool& hasCopied,
   return AqlValue();
 }
 
+AqlValue AqlValue::materialize(transaction::Methods* trx, bool& hasCopied,
+                               bool resolveExternals) const {
+  return materialize(trx->transactionContextPtr()->getVPackOptions(), hasCopied, resolveExternals);
+}
+
 /// @brief clone a value
 AqlValue AqlValue::clone() const {
   switch (type()) {
@@ -1173,23 +1181,24 @@ AqlValue AqlValue::CreateFromBlocks(transaction::Methods* trx,
 }
 
 /// @brief comparison for AqlValue objects
-int AqlValue::Compare(transaction::Methods* trx, AqlValue const& left,
+int AqlValue::Compare(velocypack::Options const* options, AqlValue const& left,
                       AqlValue const& right, bool compareUtf8) {
   AqlValue::AqlValueType const leftType = left.type();
   AqlValue::AqlValueType const rightType = right.type();
 
   if (leftType != rightType) {
+    // TODO implement this case more efficiently
     if (leftType == RANGE || rightType == RANGE || leftType == DOCVEC || rightType == DOCVEC) {
       // range|docvec against x
-      transaction::BuilderLeaser leftBuilder(trx);
+      VPackBuilder leftBuilder;
-      left.toVelocyPack(trx, *leftBuilder.get(), false);
+      left.toVelocyPack(options, leftBuilder, false);
 
-      transaction::BuilderLeaser rightBuilder(trx);
+      VPackBuilder rightBuilder;
-      right.toVelocyPack(trx, *rightBuilder.get(), false);
+      right.toVelocyPack(options, rightBuilder, false);
 
-      return arangodb::basics::VelocyPackHelper::compare(
+      return arangodb::basics::VelocyPackHelper::compare(leftBuilder.slice(),
-          leftBuilder->slice(), rightBuilder->slice(), compareUtf8,
+                                                         rightBuilder.slice(),
-          trx->transactionContextPtr()->getVPackOptions());
+                                                         compareUtf8, options);
     }
     // fall-through to other types intentional
   }
@@ -1201,9 +1210,8 @@ int AqlValue::Compare(transaction::Methods* trx, AqlValue const& left,
     case VPACK_SLICE_POINTER:
     case VPACK_MANAGED_SLICE:
     case VPACK_MANAGED_BUFFER: {
-      return arangodb::basics::VelocyPackHelper::compare(
+      return arangodb::basics::VelocyPackHelper::compare(left.slice(), right.slice(),
-          left.slice(), right.slice(), compareUtf8,
+                                                         compareUtf8, options);
-          trx->transactionContextPtr()->getVPackOptions());
     }
     case DOCVEC: {
       // use lexicographic ordering of AqlValues regardless of block,
@@ -1232,7 +1240,7 @@ int AqlValue::Compare(transaction::Methods* trx, AqlValue const& left,
         AqlValue const& rval =
             right._data.docvec->at(rblock)->getValueReference(ritem, 0);
 
-        int cmp = Compare(trx, lval, rval, compareUtf8);
+        int cmp = Compare(options, lval, rval, compareUtf8);
 
         if (cmp != 0) {
           return cmp;
@@ -1280,6 +1288,11 @@ int AqlValue::Compare(transaction::Methods* trx, AqlValue const& left,
   return 0;
 }
 
+int AqlValue::Compare(transaction::Methods* trx, AqlValue const& left,
+                      AqlValue const& right, bool compareUtf8) {
+  return Compare(trx->transactionContextPtr()->getVPackOptions(), left, right, compareUtf8);
+}
+
 AqlValue::AqlValue(std::vector<arangodb::aql::SharedAqlItemBlockPtr>* docvec) noexcept {
   TRI_ASSERT(docvec != nullptr);
   _data.docvec = docvec;
@@ -1610,68 +1623,8 @@ AqlValueGuard::~AqlValueGuard() {
 }
 
 void AqlValueGuard::steal() { _destroy = false; }
+
 AqlValue& AqlValueGuard::value() { return _value; }
-AqlValueMaterializer::AqlValueMaterializer(transaction::Methods* trx)
-    : trx(trx), materialized(), hasCopied(false) {}
-AqlValueMaterializer::AqlValueMaterializer(AqlValueMaterializer const& other)
-    : trx(other.trx), materialized(other.materialized), hasCopied(other.hasCopied) {
-  if (other.hasCopied) {
-    // copy other's slice
-    materialized = other.materialized.clone();
-  }
-}
-
-AqlValueMaterializer& AqlValueMaterializer::operator=(AqlValueMaterializer const& other) {
-  if (this != &other) {
-    TRI_ASSERT(trx == other.trx);  // must be from same transaction
-    trx = other.trx; // to shut up cppcheck
-    if (hasCopied) {
-      // destroy our own slice
-      materialized.destroy();
-      hasCopied = false;
-    }
-    // copy other's slice
-    materialized = other.materialized.clone();
-    hasCopied = other.hasCopied;
-  }
-  return *this;
-}
-
-AqlValueMaterializer::AqlValueMaterializer(AqlValueMaterializer&& other) noexcept
-    : trx(other.trx), materialized(other.materialized), hasCopied(other.hasCopied) {
-  // reset other
-  other.hasCopied = false;
-  // cppcheck-suppress *
-  other.materialized = AqlValue();
-}
-
-AqlValueMaterializer& AqlValueMaterializer::operator=(AqlValueMaterializer&& other) noexcept {
-  if (this != &other) {
-    TRI_ASSERT(trx == other.trx);  // must be from same transaction
-    trx = other.trx; // to shut up cppcheck
-    if (hasCopied) {
-      // destroy our own slice
-      materialized.destroy();
-    }
-    // reset other
-    materialized = other.materialized;
-    hasCopied = other.hasCopied;
-    other.materialized = AqlValue();
-  }
-  return *this;
-}
-
-AqlValueMaterializer::~AqlValueMaterializer() {
-  if (hasCopied) {
-    materialized.destroy();
-  }
-}
-
-arangodb::velocypack::Slice AqlValueMaterializer::slice(AqlValue const& value,
-                                                        bool resolveExternals) {
-  materialized = value.materialize(trx, hasCopied, resolveExternals);
-  return materialized.slice();
-}
 
 size_t std::hash<arangodb::aql::AqlValue>::operator()(arangodb::aql::AqlValue const& x) const
     noexcept {

arangod/Aql/AqlValue.h

@@ -47,6 +47,7 @@ namespace velocypack {
 template <typename T>
 class Buffer;
 class Builder;
+struct Options;
 class Slice;
 class StringRef;
 }
@@ -329,11 +330,12 @@ struct AqlValue final {
   v8::Handle<v8::Value> toV8(v8::Isolate* isolate, transaction::Methods*) const;
 
   /// @brief materializes a value into the builder
-  void toVelocyPack(transaction::Methods*, arangodb::velocypack::Builder& builder,
+  void toVelocyPack(velocypack::Options const*, arangodb::velocypack::Builder&, bool resolveExternals) const;
-                    bool resolveExternals) const;
+  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;
   AqlValue materialize(transaction::Methods*, bool& hasCopied, bool resolveExternals) const;
 
   /// @brief return the slice for the value
@@ -364,6 +366,8 @@ struct AqlValue final {
                                    arangodb::aql::RegisterId);
 
   /// @brief compare function for two values
+  static int Compare(velocypack::Options const*, AqlValue const& left,
+                     AqlValue const& right, bool useUtf8);
   static int Compare(transaction::Methods*, AqlValue const& left,
                      AqlValue const& right, bool useUtf8);
 
@@ -399,28 +403,6 @@ class AqlValueGuard {
   bool _destroy;
 };
 
-struct AqlValueMaterializer {
-  explicit AqlValueMaterializer(transaction::Methods* trx);
-
-  AqlValueMaterializer(AqlValueMaterializer const& other);
-
-  // cppcheck-suppress operatorEqVarError
-  AqlValueMaterializer& operator=(AqlValueMaterializer const& other);
-
-  AqlValueMaterializer(AqlValueMaterializer&& other) noexcept;
-
-  // cppcheck-suppress operatorEqVarError
-  AqlValueMaterializer& operator=(AqlValueMaterializer&& other) noexcept;
-
-  ~AqlValueMaterializer();
-
-  arangodb::velocypack::Slice slice(AqlValue const& value, bool resolveExternals);
-
-  transaction::Methods* trx;
-  AqlValue materialized;
-  bool hasCopied;
-};
-
 static_assert(sizeof(AqlValue) == 16, "invalid AqlValue size");
 
 }  // namespace aql

arangod/Aql/AqlValueMaterializer.cpp

@@ -0,0 +1,107 @@
+////////////////////////////////////////////////////////////////////////////////
+/// 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 Max Neunhoeffer
+/// @author Jan Steemann
+/// @author Tobias Gödderz
+////////////////////////////////////////////////////////////////////////////////
+
+#include "AqlValueMaterializer.h"
+
+#include "Basics/debugging.h"
+#include "Transaction/Context.h"
+#include "Transaction/Methods.h"
+
+#include <velocypack/Slice.h>
+
+using namespace arangodb;
+using namespace arangodb::aql;
+
+AqlValueMaterializer::AqlValueMaterializer(velocypack::Options const* options)
+    : options(options),
+      materialized(),
+      hasCopied(false) {}
+AqlValueMaterializer::AqlValueMaterializer(transaction::Methods* trx)
+    : options(trx->transactionContextPtr()->getVPackOptions()),
+      materialized(),
+      hasCopied(false) {}
+
+AqlValueMaterializer::AqlValueMaterializer(AqlValueMaterializer const& other)
+    : options(other.options),
+      materialized(other.materialized),
+      hasCopied(other.hasCopied) {
+  if (other.hasCopied) {
+    // copy other's slice
+    materialized = other.materialized.clone();
+  }
+}
+
+AqlValueMaterializer& AqlValueMaterializer::operator=(AqlValueMaterializer const& other) {
+  if (this != &other) {
+    TRI_ASSERT(options == other.options);  // must be from same transaction
+    options = other.options;
+    if (hasCopied) {
+      // destroy our own slice
+      materialized.destroy();
+      hasCopied = false;
+    }
+    // copy other's slice
+    materialized = other.materialized.clone();
+    hasCopied = other.hasCopied;
+  }
+  return *this;
+}
+
+AqlValueMaterializer::AqlValueMaterializer(AqlValueMaterializer&& other) noexcept
+    : options(other.options),
+      materialized(other.materialized),
+      hasCopied(other.hasCopied) {
+  // reset other
+  other.hasCopied = false;
+  // cppcheck-suppress *
+  other.materialized = AqlValue();
+}
+
+AqlValueMaterializer& AqlValueMaterializer::operator=(AqlValueMaterializer&& other) noexcept {
+  if (this != &other) {
+    TRI_ASSERT(options == other.options);  // must be from same transaction
+    options = other.options;
+    if (hasCopied) {
+      // destroy our own slice
+      materialized.destroy();
+    }
+    // reset other
+    materialized = other.materialized;
+    hasCopied = other.hasCopied;
+    other.materialized = AqlValue();
+  }
+  return *this;
+}
+
+AqlValueMaterializer::~AqlValueMaterializer() {
+  if (hasCopied) {
+    materialized.destroy();
+  }
+}
+
+arangodb::velocypack::Slice AqlValueMaterializer::slice(AqlValue const& value,
+                                                        bool resolveExternals) {
+  materialized = value.materialize(options, hasCopied, resolveExternals);
+  return materialized.slice();
+}

arangod/Aql/AqlValueMaterializer.h

@@ -0,0 +1,70 @@
+////////////////////////////////////////////////////////////////////////////////
+/// 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 Max Neunhoeffer
+/// @author Jan Steemann
+/// @author Tobias Gödderz
+////////////////////////////////////////////////////////////////////////////////
+
+#ifndef ARANGOD_AQL_AQLVALUEMATERIALIZER_H
+#define ARANGOD_AQL_AQLVALUEMATERIALIZER_H
+
+#include "Aql/AqlValue.h"
+
+namespace arangodb {
+
+namespace transaction {
+class Methods;
+}
+
+namespace velocypack {
+class Slice;
+struct Options;
+}  // namespace velocypack
+
+namespace aql {
+
+/// @brief Helper class to materialize AqlValues (see AqlValue::materialize).
+struct AqlValueMaterializer {
+  explicit AqlValueMaterializer(velocypack::Options const* options);
+  explicit AqlValueMaterializer(arangodb::transaction::Methods* trx);
+
+  AqlValueMaterializer(AqlValueMaterializer const& other);
+
+  // cppcheck-suppress operatorEqVarError
+  AqlValueMaterializer& operator=(AqlValueMaterializer const& other);
+
+  AqlValueMaterializer(AqlValueMaterializer&& other) noexcept;
+
+  // cppcheck-suppress operatorEqVarError
+  AqlValueMaterializer& operator=(AqlValueMaterializer&& other) noexcept;
+
+  ~AqlValueMaterializer();
+
+  arangodb::velocypack::Slice slice(arangodb::aql::AqlValue const& value, bool resolveExternals);
+
+  arangodb::velocypack::Options const* options;
+  arangodb::aql::AqlValue materialized;
+  bool hasCopied;
+};
+
+}  // namespace aql
+}  // namespace arangodb
+
+#endif  // ARANGOD_AQL_AQLVALUEMATERIALIZER_H

arangod/Aql/Ast.cpp

@@ -37,6 +37,7 @@
 #include "Aql/Graphs.h"
 #include "Aql/ModificationOptions.h"
 #include "Aql/Query.h"
+#include "Aql/AqlValueMaterializer.h"
 #include "Basics/Exceptions.h"
 #include "Basics/StringUtils.h"
 #include "Basics/tri-strings.h"

arangod/Aql/ClusterNodes.cpp

@@ -49,6 +49,7 @@
 #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"
@@ -492,27 +493,16 @@ std::unique_ptr<ExecutionBlock> GatherNode::createBlock(
   if (_elements.empty()) {
     TRI_ASSERT(getRegisterPlan()->nrRegs[previousNode->getDepth()] ==
                getRegisterPlan()->nrRegs[getDepth()]);
-
+    if (ServerState::instance()->isCoordinator() && _parallelism == Parallelism::Parallel) {
-    if (ServerState::instance()->isCoordinator()) {
-      // In the coordinator case the GatherBlock will fetch from RemoteBlocks.
-      if (_parallelism == Parallelism::Parallel) {
       UnsortedGatherExecutorInfos infos(getRegisterPlan()->nrRegs[getDepth()],
                                         calcRegsToKeep(), getRegsToClear());
       return std::make_unique<ExecutionBlockImpl<UnsortedGatherExecutor>>(&engine, this, std::move(infos));
-      }
-      IdExecutorInfos infos(getRegisterPlan()->nrRegs[getDepth()],
-                            calcRegsToKeep(), getRegsToClear());
-      // We want to immediately move the block on and not wait for additional requests here (hence passthrough)
-      return std::make_unique<ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, SingleRowFetcher<BlockPassthrough::Enable>>>>(
-          &engine, this, std::move(infos));
     } else {
       IdExecutorInfos infos(getRegisterPlan()->nrRegs[getDepth()],
                             calcRegsToKeep(), getRegsToClear());
-      // In the DBServer case the GatherBlock will merge local results and then expose them (directly or indirectly)
+
-      // to the RemoteBlock on coordinator. We want to trigger as few requests as possible, so we invest the little
+      return std::make_unique<ExecutionBlockImpl<UnsortingGatherExecutor>>(&engine, this,
-      // memory inefficiency that we have here in favor of a better grouping of requests.
+                                                                           std::move(infos));
-      return std::make_unique<ExecutionBlockImpl<IdExecutor<BlockPassthrough::Disable, SingleRowFetcher<BlockPassthrough::Disable>>>>(
-          &engine, this, std::move(infos));
     }
   }
   

arangod/Aql/ConstrainedSortExecutor.cpp

@@ -54,9 +54,9 @@ void eraseRow(SharedAqlItemBlockPtr& block, size_t row) {
 /// @brief OurLessThan
 class arangodb::aql::ConstrainedLessThan {
  public:
-  ConstrainedLessThan(arangodb::transaction::Methods* trx,
+  ConstrainedLessThan(velocypack::Options const* options,
                       std::vector<arangodb::aql::SortRegister>& sortRegisters) noexcept
-      : _trx(trx), _heapBuffer(nullptr), _sortRegisters(sortRegisters) {}
+      : _vpackOptions(options), _heapBuffer(nullptr), _sortRegisters(sortRegisters) {}
 
   void setBuffer(arangodb::aql::AqlItemBlock* heap) { _heapBuffer = heap; }
 
@@ -67,7 +67,7 @@ class arangodb::aql::ConstrainedLessThan {
       auto const& lhs = _heapBuffer->getValueReference(a, sortReg.reg);
       auto const& rhs = _heapBuffer->getValueReference(b, sortReg.reg);
 
-      int const cmp = arangodb::aql::AqlValue::Compare(_trx, lhs, rhs, true);
+      int const cmp = arangodb::aql::AqlValue::Compare(_vpackOptions, lhs, rhs, true);
 
       if (cmp < 0) {
         return sortReg.asc;
@@ -80,7 +80,7 @@ class arangodb::aql::ConstrainedLessThan {
   }
 
  private:
-  arangodb::transaction::Methods* _trx;
+  velocypack::Options const* const _vpackOptions;
   arangodb::aql::AqlItemBlock* _heapBuffer;
   std::vector<arangodb::aql::SortRegister>& _sortRegisters;
 };  // ConstrainedLessThan
@@ -123,7 +123,7 @@ bool ConstrainedSortExecutor::compareInput(size_t const& rowPos, InputAqlItemRow
     auto const& lhs = _heapBuffer->getValueReference(rowPos, reg.reg);
     auto const& rhs = row.getValue(reg.reg);
 
-    int const cmp = arangodb::aql::AqlValue::Compare(_infos.trx(), lhs, rhs, true);
+    int const cmp = arangodb::aql::AqlValue::Compare(_infos.vpackOptions(), lhs, rhs, true);
 
     if (cmp < 0) {
       return reg.asc;
@@ -144,7 +144,8 @@ ConstrainedSortExecutor::ConstrainedSortExecutor(Fetcher& fetcher, SortExecutorI
       _skippedAfter(0),
       _heapBuffer(_infos._manager.requestBlock(_infos._limit,
                                                _infos.numberOfOutputRegisters())),
-      _cmpHeap(std::make_unique<ConstrainedLessThan>(_infos.trx(), _infos.sortRegisters())),
+      _cmpHeap(std::make_unique<ConstrainedLessThan>(_infos.vpackOptions(),
+                                                     _infos.sortRegisters())),
       _heapOutputRow{_heapBuffer, make_shared_unordered_set(),
                      make_shared_unordered_set(_infos.numberOfOutputRegisters()),
                      _infos.registersToClear()} {

arangod/Aql/DependencyProxy.cpp

@@ -255,7 +255,8 @@ template <BlockPassthrough blockPassthrough>
 DependencyProxy<blockPassthrough>::DependencyProxy(
     std::vector<ExecutionBlock*> const& dependencies, AqlItemBlockManager& itemBlockManager,
     std::shared_ptr<std::unordered_set<RegisterId> const> inputRegisters,
-    RegisterId nrInputRegisters)
+    RegisterId nrInputRegisters,
+    velocypack::Options const* const options)
     : _dependencies(dependencies),
       _itemBlockManager(itemBlockManager),
       _inputRegisters(std::move(inputRegisters)),
@@ -264,7 +265,8 @@ DependencyProxy<blockPassthrough>::DependencyProxy(
       _blockQueue(),
       _blockPassThroughQueue(),
       _currentDependency(0),
-      _skipped(0) {}
+      _skipped(0),
+      _vpackOptions(options) {}
 
 template <BlockPassthrough blockPassthrough>
 RegisterId DependencyProxy<blockPassthrough>::getNrInputRegisters() const {
@@ -316,5 +318,11 @@ bool DependencyProxy<blockPassthrough>::advanceDependency() {
   return true;
 }
 
+template <BlockPassthrough allowBlockPassthrough>
+velocypack::Options const* DependencyProxy<allowBlockPassthrough>::velocypackOptions() const
+    noexcept {
+  return _vpackOptions;
+}
+
 template class ::arangodb::aql::DependencyProxy<BlockPassthrough::Enable>;
 template class ::arangodb::aql::DependencyProxy<BlockPassthrough::Disable>;

arangod/Aql/DependencyProxy.h

@@ -68,7 +68,8 @@ class DependencyProxy {
   DependencyProxy(std::vector<ExecutionBlock*> const& dependencies,
                   AqlItemBlockManager& itemBlockManager,
                   std::shared_ptr<std::unordered_set<RegisterId> const> inputRegisters,
-                  RegisterId nrInputRegisters);
+                  RegisterId nrInputRegisters,
+                  velocypack::Options const*);
 
   TEST_VIRTUAL ~DependencyProxy() = default;
 
@@ -109,6 +110,8 @@ class DependencyProxy {
 
   void setDistributeId(std::string const& distId) { _distributeId = distId; }
 
+  [[nodiscard]] velocypack::Options const* velocypackOptions() const noexcept;
+
  protected:
   [[nodiscard]] AqlItemBlockManager& itemBlockManager();
   [[nodiscard]] AqlItemBlockManager const& itemBlockManager() const;
@@ -134,6 +137,7 @@ class DependencyProxy {
   // only modified in case of multiple dependencies + Passthrough otherwise always 0
   size_t _currentDependency;
   size_t _skipped;
+  velocypack::Options const* const _vpackOptions;
 };
 
 }  // namespace arangodb::aql

arangod/Aql/DistributeExecutor.cpp

@@ -251,11 +251,18 @@ std::pair<ExecutionState, bool> ExecutionBlockImpl<DistributeExecutor>::getBlock
 
     SharedAqlItemBlockPtr cur = _buffer[_index];
 
-    while (_pos < cur->size()) {
+    for (; _pos < cur->size(); ++_pos) {
+      if (!cur->isShadowRow(_pos)) {
         // this may modify the input item buffer in place
         size_t const id = sendToClient(cur);
 
-      _distBuffer[id].emplace_back(_index, _pos++);
+        _distBuffer[id].emplace_back(_index, _pos);
+      } else {
+        // A shadow row must always be distributed to all clients.
+        for (auto& dist : _distBuffer) {
+          dist.emplace_back(_index, _pos);
+        }
+      }
     }
 
     if (_pos == cur->size()) {

arangod/Aql/ExecutionBlock.cpp

@@ -70,7 +70,7 @@ std::string const& stateToString(aql::ExecutionState state) {
 
 ExecutionBlock::ExecutionBlock(ExecutionEngine* engine, ExecutionNode const* ep)
     : _engine(engine),
-      _trx(engine->getQuery()->trx()),
+      _trxVpackOptions(engine->getQuery()->trx()->transactionContextPtr()->getVPackOptions()),
       _shutdownResult(TRI_ERROR_NO_ERROR),
       _done(false),
       _isInSplicedSubquery(ep != nullptr ? ep->isInSplicedSubquery() : false),
@@ -190,8 +190,8 @@ std::pair<ExecutionState, SharedAqlItemBlockPtr> ExecutionBlock::traceGetSomeEnd
               << "getSome type=" << node->getTypeString() << " result: nullptr";
         } else {
           VPackBuilder builder;
-          result->toSimpleVPack(transaction(), builder);
+          auto const options = trxVpackOptions();
-          auto options = transaction()->transactionContextPtr()->getVPackOptions();
+          result->toSimpleVPack(options, builder);
           LOG_TOPIC("fcd9c", INFO, Logger::QUERIES)
               << "[query#" << queryId << "] "
               << "getSome type=" << node->getTypeString()
@@ -272,7 +272,9 @@ ExecutionState ExecutionBlock::getHasMoreState() {
 
 ExecutionNode const* ExecutionBlock::getPlanNode() const { return _exeNode; }
 
-transaction::Methods* ExecutionBlock::transaction() const { return _trx; }
+velocypack::Options const* ExecutionBlock::trxVpackOptions() const noexcept {
+  return _trxVpackOptions;
+}
 
 void ExecutionBlock::addDependency(ExecutionBlock* ep) {
   TRI_ASSERT(ep != nullptr);

arangod/Aql/ExecutionBlock.h

@@ -117,7 +117,7 @@ class ExecutionBlock {
   // TODO: Can we get rid of this? Problem: Subquery Executor is using it.
   ExecutionNode const* getPlanNode() const;
 
-  transaction::Methods* transaction() const;
+  [[nodiscard]] velocypack::Options const* trxVpackOptions() const noexcept;
 
   /// @brief add a dependency
   void addDependency(ExecutionBlock* ep);
@@ -128,8 +128,7 @@ class ExecutionBlock {
   /// @brief the execution engine
   ExecutionEngine* _engine;
 
-  /// @brief the transaction for this query
+  velocypack::Options const* _trxVpackOptions;
-  transaction::Methods* _trx;
 
   /// @brief the Result returned during the shutdown phase. Is kept for multiple
   ///        waiting phases.

arangod/Aql/ExecutionBlockImpl.cpp

@@ -42,7 +42,6 @@
 #include "Aql/IResearchViewExecutor.h"
 #include "Aql/IdExecutor.h"
 #include "Aql/IndexExecutor.h"
-#include "Aql/IndexNode.h"
 #include "Aql/InputAqlItemRow.h"
 #include "Aql/KShortestPathsExecutor.h"
 #include "Aql/LimitExecutor.h"
@@ -65,6 +64,7 @@
 #include "Aql/SubqueryStartExecutor.h"
 #include "Aql/TraversalExecutor.h"
 #include "Aql/UnsortedGatherExecutor.h"
+#include "Aql/UnsortingGatherExecutor.h"
 
 #include "Aql/SimpleModifier.h"
 #include "Aql/UpsertModifier.h"
@@ -108,7 +108,8 @@ ExecutionBlockImpl<Executor>::ExecutionBlockImpl(ExecutionEngine* engine,
                                                  typename Executor::Infos&& infos)
     : ExecutionBlock(engine, node),
       _dependencyProxy(_dependencies, engine->itemBlockManager(),
-                       infos.getInputRegisters(), infos.numberOfInputRegisters()),
+                       infos.getInputRegisters(), infos.numberOfInputRegisters(),
+                       trxVpackOptions()),
       _rowFetcher(_dependencyProxy),
       _infos(std::move(infos)),
       _executor(_rowFetcher, _infos),
@@ -370,9 +371,9 @@ static SkipVariants constexpr skipType() {
            std::is_same<Executor, IResearchViewMergeExecutor<true, false>>::value ||
            std::is_same<Executor, EnumerateCollectionExecutor>::value ||
            std::is_same<Executor, LimitExecutor>::value ||
-           std::is_same<Executor, IdExecutor<BlockPassthrough::Disable, SingleRowFetcher<BlockPassthrough::Disable>>>::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, MaterializeExecutor<RegisterId>>::value ||
            std::is_same<Executor, MaterializeExecutor<std::string const&>>::value),
@@ -871,8 +872,6 @@ template class ::arangodb::aql::ExecutionBlockImpl<IResearchViewMergeExecutor<tr
 template class ::arangodb::aql::ExecutionBlockImpl<IdExecutor<BlockPassthrough::Enable, ConstFetcher>>;
 template class ::arangodb::aql::ExecutionBlockImpl<
     IdExecutor<BlockPassthrough::Enable, SingleRowFetcher<BlockPassthrough::Enable>>>;
-template class ::arangodb::aql::ExecutionBlockImpl<
-    IdExecutor<BlockPassthrough::Disable, SingleRowFetcher<BlockPassthrough::Disable>>>;
 template class ::arangodb::aql::ExecutionBlockImpl<IndexExecutor>;
 template class ::arangodb::aql::ExecutionBlockImpl<LimitExecutor>;
 
@@ -897,6 +896,7 @@ template class ::arangodb::aql::ExecutionBlockImpl<SubqueryStartExecutor>;
 template class ::arangodb::aql::ExecutionBlockImpl<TraversalExecutor>;
 template class ::arangodb::aql::ExecutionBlockImpl<SortingGatherExecutor>;
 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/ExecutionNode.cpp

@@ -567,6 +567,7 @@ bool ExecutionNode::isEqualTo(ExecutionNode const& other) const {
 
   return ((this->getType() == other.getType()) && (_id == other._id) &&
           (_depth == other._depth) &&
+          (isInSplicedSubquery() == other.isInSplicedSubquery()) &&
           (std::equal(_dependencies.begin(), _dependencies.end(),
                       other._dependencies.begin(), comparator)));
 }

arangod/Aql/Expression.cpp

@@ -37,6 +37,7 @@
 #include "Aql/Range.h"
 #include "Aql/V8Executor.h"
 #include "Aql/Variable.h"
+#include "Aql/AqlValueMaterializer.h"
 #include "Basics/Exceptions.h"
 #include "Basics/NumberUtils.h"
 #include "Basics/StringBuffer.h"

arangod/Aql/Functions.cpp

@@ -32,6 +32,7 @@
 #include "Aql/Query.h"
 #include "Aql/Range.h"
 #include "Aql/V8Executor.h"
+#include "Aql/AqlValueMaterializer.h"
 #include "Basics/Exceptions.h"
 #include "Basics/HybridLogicalClock.h"
 #include "Basics/Mutex.h"

arangod/Aql/IdExecutor.h

@@ -31,6 +31,8 @@
 #include <tuple>
 #include <utility>
 
+// TODO Clean up unused variants of the IdExecutor - some of them aren't in use anymore.
+
 namespace arangodb {
 namespace transaction {
 class Methods;
@@ -57,6 +59,7 @@ class IdExecutorInfos : public ExecutorInfos {
 
   std::string const& distributeId();
 
+  // TODO This is probably needed only for UnsortingGather now, so can be removed here.
   [[nodiscard]] bool isResponsibleForInitializeCursor() const;
 
  private:

arangod/Aql/IndexExecutor.cpp

@@ -38,6 +38,7 @@
 #include "Aql/OutputAqlItemRow.h"
 #include "Aql/Query.h"
 #include "Aql/SingleRowFetcher.h"
+#include "Aql/AqlValueMaterializer.h"
 #include "Basics/ScopeGuard.h"
 #include "Cluster/ServerState.h"
 #include "ExecutorExpressionContext.h"

arangod/Aql/InputAqlItemRow.cpp

@@ -119,7 +119,7 @@ SharedAqlItemBlockPtr InputAqlItemRow::cloneToBlock(AqlItemBlockManager& manager
 ///                  corresponding position
 ///  "raw":     List of actual values, positions 0 and 1 are always null
 ///                  such that actual indices start at 2
-void InputAqlItemRow::toVelocyPack(transaction::Methods* trx, VPackBuilder& result) const {
+void InputAqlItemRow::toVelocyPack(velocypack::Options const* const trxOptions, VPackBuilder& result) const {
   TRI_ASSERT(isInitialized());
   TRI_ASSERT(result.isOpenObject());
   VPackOptions options(VPackOptions::Defaults);
@@ -203,7 +203,7 @@ void InputAqlItemRow::toVelocyPack(transaction::Methods* trx, VPackBuilder& resu
 
       if (it == table.end()) {
         currentState = Next;
-        a.toVelocyPack(trx, raw, false);
+        a.toVelocyPack(trxOptions, raw, false);
         table.try_emplace(a, pos++);
       } else {
         currentState = Positional;
@@ -290,7 +290,8 @@ bool InputAqlItemRow::operator!=(InputAqlItemRow const& other) const noexcept {
   return !(*this == other);
 }
 
-bool InputAqlItemRow::equates(InputAqlItemRow const& other) const noexcept {
+bool InputAqlItemRow::equates(InputAqlItemRow const& other,
+                              velocypack::Options const* const options) const noexcept {
   if (!isInitialized() || !other.isInitialized()) {
     return isInitialized() == other.isInitialized();
   }
@@ -298,8 +299,9 @@ bool InputAqlItemRow::equates(InputAqlItemRow const& other) const noexcept {
   if (getNrRegisters() != other.getNrRegisters()) {
     return false;
   }
-  // NOLINTNEXTLINE(modernize-use-transparent-functors)
+  auto const eq = [options](auto left, auto right) {
-  auto const eq = std::equal_to<AqlValue>{};
+    return 0 == AqlValue::Compare(options, left, right, false);
+  };
   for (RegisterId i = 0; i < getNrRegisters(); ++i) {
     if (!eq(getValue(i), other.getValue(i))) {
       return false;

arangod/Aql/InputAqlItemRow.h

@@ -33,11 +33,9 @@
 #include <unordered_set>
 
 namespace arangodb {
-namespace transaction {
-class Methods;
-}
 namespace velocypack {
 class Builder;
+struct Options;
 }
 namespace aql {
 
@@ -103,7 +101,8 @@ class InputAqlItemRow {
   // blocks are equal, because comparing rows of blocks with different layouts
   // does not make sense.
   // Invalid rows are considered equivalent.
-  bool equates(InputAqlItemRow const& other) const noexcept;
+  [[nodiscard]] bool equates(InputAqlItemRow const& other,
+                             velocypack::Options const* options) const noexcept;
 
   bool isInitialized() const noexcept;
 
@@ -132,7 +131,7 @@ class InputAqlItemRow {
   /// @brief toVelocyPack, transfer a single AqlItemRow to Json, the result can
   /// be used to recreate the AqlItemBlock via the Json constructor
   /// Uses the same API as an AqlItemBlock with only a single row
-  void toVelocyPack(transaction::Methods* trx, arangodb::velocypack::Builder&) const;
+  void toVelocyPack(velocypack::Options const*, arangodb::velocypack::Builder&) const;
 
  private:
   AqlItemBlock& block() noexcept;

arangod/Aql/ModificationExecutor.cpp

@@ -47,6 +47,7 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 using namespace arangodb::basics;
+
 namespace arangodb {
 namespace aql {
 

arangod/Aql/MultiDependencySingleRowFetcher.cpp

@@ -25,6 +25,12 @@
 #include "Aql/AqlItemBlock.h"
 #include "Aql/DependencyProxy.h"
 #include "Aql/ShadowAqlItemRow.h"
+#include "Logger/LogMacros.h"
+#include "Transaction/Context.h"
+#include "Transaction/Methods.h"
+
+#include <velocypack/Builder.h>
+#include <velocypack/velocypack-aliases.h>
 
 using namespace arangodb;
 using namespace arangodb::aql;
@@ -119,7 +125,8 @@ std::pair<ExecutionState, ShadowAqlItemRow> MultiDependencySingleRowFetcher::fet
       } else {
         TRI_ASSERT(row.isInitialized());
         // All shadow rows must be equal!
-        TRI_ASSERT(row.equates(ShadowAqlItemRow{dep._currentBlock, dep._rowIndex}));
+        auto const options = _dependencyProxy->velocypackOptions();
+        TRI_ASSERT(row.equates(ShadowAqlItemRow{dep._currentBlock, dep._rowIndex}, options));
       }
     }
   }

arangod/Aql/OptimizerRules.cpp

@@ -7319,6 +7319,8 @@ bool nodeMakesThisQueryLevelUnsuitableForSubquerySplicing(ExecutionNode const* c
     case ExecutionNode::RETURN:
     case ExecutionNode::DISTRIBUTE:
     case ExecutionNode::SCATTER:
+    case ExecutionNode::GATHER:
+    case ExecutionNode::REMOTE:
     case ExecutionNode::REMOTESINGLE:
     case ExecutionNode::MATERIALIZE:
     case ExecutionNode::DISTRIBUTE_CONSUMER:
@@ -7326,10 +7328,6 @@ bool nodeMakesThisQueryLevelUnsuitableForSubquerySplicing(ExecutionNode const* c
     case ExecutionNode::SUBQUERY_END:
       // These nodes do not initiate a skip themselves, and thus are fine.
       return false;
-    // UnsortingGather currently does not work. Also, as we would possibly add
-    // them with remote nodes in the query, we exclude these here, too.
-    case ExecutionNode::REMOTE:
-    case ExecutionNode::GATHER:
     case ExecutionNode::NORESULTS:
       // no results currently cannot work, as they do not fetch from above.
     case ExecutionNode::LIMIT:
@@ -7558,6 +7556,8 @@ void arangodb::aql::spliceSubqueriesRule(Optimizer* opt, std::unique_ptr<Executi
               plan->createNode<RemoteNode>(plan.get(), plan->nextId(),
                                            &plan->getAst()->query()->vocbase(),
                                            "", "", "");
+          scatterNode->setIsInSplicedSubquery(true);
+          remoteNode->setIsInSplicedSubquery(true);
           plan->insertAfter(start, scatterNode);
           plan->insertAfter(scatterNode, remoteNode);
 

arangod/Aql/OutputAqlItemRow.cpp

@@ -192,9 +192,9 @@ void OutputAqlItemRow::advanceRow() {
   _numValuesWritten = 0;
 }
 
-void OutputAqlItemRow::toVelocyPack(transaction::Methods& trx, VPackBuilder& builder) {
+void OutputAqlItemRow::toVelocyPack(velocypack::Options const* options, VPackBuilder& builder) {
   TRI_ASSERT(produced());
-  block().rowToSimpleVPack(_baseIndex, &trx, builder);
+  block().rowToSimpleVPack(_baseIndex, options, builder);
 }
 
 SharedAqlItemBlockPtr OutputAqlItemRow::stealBlock() {

arangod/Aql/OutputAqlItemRow.h

@@ -159,7 +159,7 @@ class OutputAqlItemRow {
   // The data of this row will be copied.
   void decreaseShadowRowDepth(ShadowAqlItemRow const& sourceRow);
 
-  void toVelocyPack(transaction::Methods& trx, velocypack::Builder& builder);
+  void toVelocyPack(velocypack::Options const* options, velocypack::Builder& builder);
 
  private:
   [[nodiscard]] std::unordered_set<RegisterId> const& outputRegisters() const {

arangod/Aql/QuerySnippet.cpp

@@ -425,6 +425,7 @@ DistributeConsumerNode* QuerySnippet::createConsumerNode(ExecutionPlan* plan,
   TRI_ASSERT(consumer != nullptr);
   // Hand over responsibility to plan, s.t. it can clean up if one of the below fails
   plan->registerNode(uniq_consumer.release());
+  consumer->setIsInSplicedSubquery(internalScatter->isInSplicedSubquery());
   consumer->addDependency(internalScatter);
   consumer->cloneRegisterPlan(internalScatter);
   internalScatter->addClient(consumer);

arangod/Aql/RegexCache.cpp

@@ -22,9 +22,11 @@
 ////////////////////////////////////////////////////////////////////////////////
 
 #include "RegexCache.h"
+
+#include "Aql/AqlValueMaterializer.h"
+#include "Basics/StringUtils.h"
 #include "Basics/Utf8Helper.h"
-#include <Basics/StringUtils.h>
+#include "Basics/tryEmplaceHelper.h"
-#include <Basics/tryEmplaceHelper.h>
 
 #include <velocypack/Collection.h>
 #include <velocypack/Dumper.h>

arangod/Aql/RemoteExecutor.cpp

@@ -37,6 +37,8 @@
 #include "Network/NetworkFeature.h"
 #include "Network/Utils.h"
 #include "Rest/CommonDefines.h"
+#include "Transaction/Context.h"
+#include "Transaction/Methods.h"
 #include "VocBase/vocbase.h"
 
 #include <fuerte/connection.h>
@@ -304,7 +306,7 @@ std::pair<ExecutionState, Result> ExecutionBlockImpl<RemoteExecutor>::initialize
   builder.add("pos", VPackValue(0));
   builder.add(VPackValue("items"));
   builder.openObject(/*unindexed*/ true);
-  input.toVelocyPack(_engine->getQuery()->trx(), builder);
+  input.toVelocyPack(_engine->getQuery()->trx()->transactionContextPtr()->getVPackOptions(), builder);
   builder.close();
 
   builder.close();

arangod/Aql/RestAqlHandler.cpp

@@ -692,7 +692,8 @@ RestStatus RestAqlHandler::handleUseQuery(std::string const& operation,
       // Backwards Compatibility
       answerBuilder.add(StaticStrings::Error, VPackValue(false));
     } else {
-      items->toVelocyPack(_query->trx(), answerBuilder);
+      items->toVelocyPack(_query->trx()->transactionContextPtr()->getVPackOptions(),
+                          answerBuilder);
     }
   } else if (operation == "skipSome") {
     auto atMost =

arangod/Aql/ShadowAqlItemRow.cpp

@@ -22,6 +22,10 @@
 
 #include "ShadowAqlItemRow.h"
 
+#include "Basics/VelocyPackHelper.h"
+#include "Transaction/Methods.h"
+#include "Transaction/Context.h"
+
 using namespace arangodb;
 using namespace arangodb::aql;
 
@@ -92,7 +96,8 @@ bool ShadowAqlItemRow::operator!=(ShadowAqlItemRow const& other) const noexcept
   return !(*this == other);
 }
 
-bool ShadowAqlItemRow::equates(ShadowAqlItemRow const& other) const noexcept {
+bool ShadowAqlItemRow::equates(ShadowAqlItemRow const& other,
+                               velocypack::Options const* options) const noexcept {
   if (!isInitialized() || !other.isInitialized()) {
     return isInitialized() == other.isInitialized();
   }
@@ -103,8 +108,9 @@ bool ShadowAqlItemRow::equates(ShadowAqlItemRow const& other) const noexcept {
   if (getDepth() != other.getDepth()) {
     return false;
   }
-  // NOLINTNEXTLINE(modernize-use-transparent-functors)
+  auto const eq = [options](auto left, auto right) {
-  auto const eq = std::equal_to<AqlValue>{};
+    return 0 == AqlValue::Compare(options, left, right, false);
+  };
   for (RegisterId i = 0; i < getNrRegisters(); ++i) {
     if (!eq(getValue(i), other.getValue(i))) {
       return false;

arangod/Aql/ShadowAqlItemRow.h

@@ -28,6 +28,9 @@
 #include <cstddef>
 
 namespace arangodb {
+namespace velocypack {
+struct Options;
+}
 namespace aql {
 
 struct CreateInvalidShadowRowHint {
@@ -113,7 +116,8 @@ class ShadowAqlItemRow {
   // blocks are equal, because comparing rows of blocks with different layouts
   // does not make sense.
   // Invalid rows are considered equivalent.
-  bool equates(ShadowAqlItemRow const& other) const noexcept;
+  [[nodiscard]] bool equates(ShadowAqlItemRow const& other,
+                             velocypack::Options const* option) const noexcept;
 
  private:
   AqlItemBlock& block() noexcept;

arangod/Aql/SortExecutor.cpp

@@ -43,9 +43,9 @@ namespace {
 /// @brief OurLessThan
 class OurLessThan {
  public:
-  OurLessThan(arangodb::transaction::Methods* trx, AqlItemMatrix const& input,
+  OurLessThan(velocypack::Options const* options, AqlItemMatrix const& input,
               std::vector<SortRegister> const& sortRegisters) noexcept
-      : _trx(trx), _input(input), _sortRegisters(sortRegisters) {}
+      : _vpackOptions(options), _input(input), _sortRegisters(sortRegisters) {}
 
   bool operator()(AqlItemMatrix::RowIndex const& a, AqlItemMatrix::RowIndex const& b) const {
     InputAqlItemRow left = _input.getRow(a);
@@ -54,7 +54,7 @@ class OurLessThan {
       AqlValue const& lhs = left.getValue(reg.reg);
       AqlValue const& rhs = right.getValue(reg.reg);
 
-      int const cmp = AqlValue::Compare(_trx, lhs, rhs, true);
+      int const cmp = AqlValue::Compare(_vpackOptions, lhs, rhs, true);
 
       if (cmp < 0) {
         return reg.asc;
@@ -67,7 +67,7 @@ class OurLessThan {
   }
 
  private:
-  arangodb::transaction::Methods* _trx;
+  velocypack::Options const* _vpackOptions;
   AqlItemMatrix const& _input;
   std::vector<SortRegister> const& _sortRegisters;
 };  // OurLessThan
@@ -90,27 +90,29 @@ SortExecutorInfos::SortExecutorInfos(
     // cppcheck-suppress passedByValue
     std::unordered_set<RegisterId> registersToClear,
     // cppcheck-suppress passedByValue
-    std::unordered_set<RegisterId> registersToKeep, transaction::Methods* trx, bool stable)
+    std::unordered_set<RegisterId> registersToKeep,
+    velocypack::Options const* options, bool stable)
     : ExecutorInfos(mapSortRegistersToRegisterIds(sortRegisters), nullptr,
                     nrInputRegisters, nrOutputRegisters,
                     std::move(registersToClear), std::move(registersToKeep)),
       _limit(limit),
       _manager(manager),
-      _trx(trx),
+      _vpackOptions(options),
       _sortRegisters(std::move(sortRegisters)),
       _stable(stable) {
-  TRI_ASSERT(trx != nullptr);
   TRI_ASSERT(!_sortRegisters.empty());
 }
 
-transaction::Methods* SortExecutorInfos::trx() const { return _trx; }
-
 std::vector<SortRegister>& SortExecutorInfos::sortRegisters() {
   return _sortRegisters;
 }
 
 bool SortExecutorInfos::stable() const { return _stable; }
 
+velocypack::Options const* SortExecutorInfos::vpackOptions() const noexcept {
+  return _vpackOptions;
+}
+
 SortExecutor::SortExecutor(Fetcher& fetcher, SortExecutorInfos& infos)
     : _infos(infos), _fetcher(fetcher), _input(nullptr), _returnNext(0) {}
 SortExecutor::~SortExecutor() = default;
@@ -160,7 +162,7 @@ void SortExecutor::doSorting() {
   TRI_ASSERT(_input != nullptr);
   _sortedIndexes = _input->produceRowIndexes();
   // comparison function
-  OurLessThan ourLessThan(_infos.trx(), *_input, _infos.sortRegisters());
+  OurLessThan ourLessThan(_infos.vpackOptions(), *_input, _infos.sortRegisters());
   if (_infos.stable()) {
     std::stable_sort(_sortedIndexes.begin(), _sortedIndexes.end(), ourLessThan);
   } else {

arangod/Aql/SortExecutor.h

@@ -54,14 +54,14 @@ class SortExecutorInfos : public ExecutorInfos {
                     AqlItemBlockManager& manager, RegisterId nrInputRegisters,
                     RegisterId nrOutputRegisters, std::unordered_set<RegisterId> registersToClear,
                     std::unordered_set<RegisterId> registersToKeep,
-                    transaction::Methods* trx, bool stable);
+                    velocypack::Options const*, bool stable);
 
   SortExecutorInfos() = delete;
   SortExecutorInfos(SortExecutorInfos&&) = default;
   SortExecutorInfos(SortExecutorInfos const&) = delete;
   ~SortExecutorInfos() = default;
 
-  arangodb::transaction::Methods* trx() const;
+  [[nodiscard]] velocypack::Options const* vpackOptions() const noexcept;
 
   std::vector<SortRegister>& sortRegisters();
 
@@ -69,9 +69,7 @@ class SortExecutorInfos : public ExecutorInfos {
 
   std::size_t _limit;
   AqlItemBlockManager& _manager;
-
+  velocypack::Options const* _vpackOptions;
- private:
-  arangodb::transaction::Methods* _trx;
   std::vector<SortRegister> _sortRegisters;
   bool _stable;
 };

arangod/Aql/SortNode.cpp

@@ -37,6 +37,8 @@
 #include "Aql/WalkerWorker.h"
 #include "Basics/StringBuffer.h"
 #include "Basics/VelocyPackHelper.h"
+#include "Transaction/Context.h"
+#include "Transaction/Methods.h"
 
 namespace {
 std::string const ConstrainedHeap = "constrained-heap";
@@ -237,8 +239,10 @@ std::unique_ptr<ExecutionBlock> SortNode::createBlock(
   }
   SortExecutorInfos infos(std::move(sortRegs), _limit, engine.itemBlockManager(),
                           getRegisterPlan()->nrRegs[previousNode->getDepth()],
-                          getRegisterPlan()->nrRegs[getDepth()], getRegsToClear(),
+                          getRegisterPlan()->nrRegs[getDepth()],
-                          calcRegsToKeep(), engine.getQuery()->trx(), _stable);
+                          getRegsToClear(), calcRegsToKeep(),
+                          engine.getQuery()->trx()->transactionContextPtr()->getVPackOptions(),
+                          _stable);
   if (sorterType() == SorterType::Standard) {
     return std::make_unique<ExecutionBlockImpl<SortExecutor>>(&engine, this,
                                                               std::move(infos));

arangod/Aql/SubqueryEndExecutionNode.cpp

@@ -30,6 +30,8 @@
 #include "Aql/RegisterPlan.h"
 #include "Aql/SubqueryEndExecutor.h"
 #include "Meta/static_assert_size.h"
+#include "Transaction/Context.h"
+#include "Transaction/Methods.h"
 
 #include <velocypack/Iterator.h>
 #include <velocypack/velocypack-aliases.h>
@@ -85,10 +87,11 @@ std::unique_ptr<ExecutionBlock> SubqueryEndNode::createBlock(
   auto outReg = variableToRegisterId(_outVariable);
   outputRegisters->emplace(outReg);
 
+  auto const vpackOptions = trx->transactionContextPtr()->getVPackOptions();
   SubqueryEndExecutorInfos infos(inputRegisters, outputRegisters,
                                  getRegisterPlan()->nrRegs[previousNode->getDepth()],
-                                 getRegisterPlan()->nrRegs[getDepth()],
+                                 getRegisterPlan()->nrRegs[getDepth()], getRegsToClear(),
-                                 getRegsToClear(), calcRegsToKeep(), trx, inReg, outReg);
+                                 calcRegsToKeep(), vpackOptions, inReg, outReg);
 
   return std::make_unique<ExecutionBlockImpl<SubqueryEndExecutor>>(&engine, this,
                                                                    std::move(infos));

arangod/Aql/SubqueryEndExecutor.cpp

@@ -45,10 +45,10 @@ SubqueryEndExecutorInfos::SubqueryEndExecutorInfos(
     RegisterId nrInputRegisters, RegisterId nrOutputRegisters,
     std::unordered_set<RegisterId> const& registersToClear,
     std::unordered_set<RegisterId> registersToKeep,
-    transaction::Methods* trxPtr, RegisterId inReg, RegisterId outReg)
+    velocypack::Options const* const options, RegisterId inReg, RegisterId outReg)
     : ExecutorInfos(std::move(readableInputRegisters), std::move(writeableOutputRegisters), nrInputRegisters,
                     nrOutputRegisters, registersToClear, std::move(registersToKeep)),
-      _trxPtr(trxPtr),
+      _vpackOptions(options),
       _outReg(outReg),
       _inReg(inReg) {}
 
@@ -60,6 +60,10 @@ bool SubqueryEndExecutorInfos::usesInputRegister() const {
   return _inReg != RegisterPlan::MaxRegisterId;
 }
 
+velocypack::Options const* SubqueryEndExecutorInfos::vpackOptions() const noexcept {
+  return _vpackOptions;
+}
+
 SubqueryEndExecutor::SubqueryEndExecutor(Fetcher& fetcher, SubqueryEndExecutorInfos& infos)
     : _fetcher(fetcher), _infos(infos), _accumulator(nullptr), _state(ACCUMULATE) {
   resetAccumulator();
@@ -87,7 +91,7 @@ std::pair<ExecutionState, NoStats> SubqueryEndExecutor::produceRows(OutputAqlIte
         if (inputRow.isInitialized() && _infos.usesInputRegister()) {
           TRI_ASSERT(_accumulator->isOpenArray());
           AqlValue value = inputRow.getValue(_infos.getInputRegister());
-          value.toVelocyPack(_infos.getTrxPtr(), *_accumulator, false);
+          value.toVelocyPack(_infos.vpackOptions(), *_accumulator, false);
         }
 
         // We have received DONE on data rows, so now

arangod/Aql/SubqueryEndExecutor.h

@@ -48,7 +48,7 @@ class SubqueryEndExecutorInfos : public ExecutorInfos {
                            RegisterId nrInputRegisters, RegisterId nrOutputRegisters,
                            std::unordered_set<RegisterId> const& registersToClear,
                            std::unordered_set<RegisterId> registersToKeep,
-                           transaction::Methods* trxPtr, RegisterId inReg,
+                           velocypack::Options const* options, RegisterId inReg,
                            RegisterId outReg);
 
   SubqueryEndExecutorInfos() = delete;
@@ -56,13 +56,13 @@ class SubqueryEndExecutorInfos : public ExecutorInfos {
   SubqueryEndExecutorInfos(SubqueryEndExecutorInfos const&) = delete;
   ~SubqueryEndExecutorInfos();
 
-  transaction::Methods* getTrxPtr() const noexcept { return _trxPtr; }
+  [[nodiscard]] velocypack::Options const* vpackOptions() const noexcept;
   inline RegisterId getOutputRegister() const { return _outReg; }
   bool usesInputRegister() const;
   inline RegisterId getInputRegister() const { return _inReg; }
 
  private:
-  transaction::Methods* _trxPtr;
+  velocypack::Options const* _vpackOptions;
   RegisterId const _outReg;
   RegisterId const _inReg;
 };

arangod/Aql/UnsortingGatherExecutor.cpp

@@ -0,0 +1,130 @@
+////////////////////////////////////////////////////////////////////////////////
+/// 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

@@ -0,0 +1,98 @@
+////////////////////////////////////////////////////////////////////////////////
+/// 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

@@ -223,6 +223,7 @@ set(LIB_ARANGO_AQL_SOURCES
   Aql/AqlTransaction.cpp
   Aql/AqlValue.cpp
   Aql/AqlValueGroup.cpp
+  Aql/AqlValueMaterializer.cpp
   Aql/Arithmetic.cpp
   Aql/Ast.cpp
   Aql/AstHelper.cpp
@@ -349,6 +350,7 @@ set(LIB_ARANGO_AQL_SOURCES
   Aql/TraversalExecutor.cpp
   Aql/TraversalNode.cpp
   Aql/UnsortedGatherExecutor.cpp
+  Aql/UnsortingGatherExecutor.cpp
   Aql/UpdateReplaceModifier.cpp
   Aql/UpsertModifier.cpp
   Aql/V8Executor.cpp

arangod/IResearch/AqlHelper.h

@@ -326,8 +326,10 @@ class ScopedAqlValue : private irs::util::noncopyable {
   }
 
   void toVelocyPack(velocypack::Builder& builder) const {
-    _node->isConstant() ? _node->toVelocyPackValue(builder)
+    _node->isConstant()
-                        : _value.toVelocyPack(nullptr, builder, false);
+        ? _node->toVelocyPackValue(builder)
+        : _value.toVelocyPack(static_cast<velocypack::Options const*>(nullptr),
+                              builder, false);
   }
 
  private:

tests/Aql/AqlItemRowTest.cpp

@@ -47,6 +47,7 @@ class AqlItemRowsTest : public ::testing::Test {
  protected:
   ResourceMonitor monitor;
   AqlItemBlockManager itemBlockManager{&monitor, SerializationFormat::SHADOWROWS};
+  velocypack::Options const* const options{&velocypack::Options::Defaults};
 
   void AssertResultMatrix(AqlItemBlock* in, VPackSlice result,
                           std::unordered_set<RegisterId> const& regsToKeep,
@@ -320,6 +321,7 @@ class AqlItemRowsCommonEqTest : public ::testing::Test {
  protected:
   ResourceMonitor monitor;
   AqlItemBlockManager itemBlockManager{&monitor, SerializationFormat::SHADOWROWS};
+  velocypack::Options const* const options{&velocypack::Options::Defaults};
 };
 
 using RowTypes = ::testing::Types<InputAqlItemRow, ShadowAqlItemRow>;
@@ -404,6 +406,7 @@ TYPED_TEST(AqlItemRowsCommonEqTest, row_eq_operators) {
 
 TYPED_TEST(AqlItemRowsCommonEqTest, row_equivalence) {
   using RowType = TypeParam;
+  auto const options = this->options;
   SharedAqlItemBlockPtr block =
       buildBlock<1>(this->itemBlockManager, {{{0}}, {{1}}});
   SharedAqlItemBlockPtr otherBlock =
@@ -418,33 +421,34 @@ TYPED_TEST(AqlItemRowsCommonEqTest, row_equivalence) {
   RowType const otherInvalidRow = createInvalidRow<RowType>();
 
   // same rows must be considered equivalent
-  EXPECT_TRUE((RowType{block, 0}.equates(RowType{block, 0})));
+  EXPECT_TRUE((RowType{block, 0}.equates(RowType{block, 0}, options)));
-  EXPECT_TRUE((RowType{block, 1}.equates(RowType{block, 1})));
+  EXPECT_TRUE((RowType{block, 1}.equates(RowType{block, 1}, options)));
 
   // different rows must be non-equivalent
-  EXPECT_FALSE((RowType{block, 0}.equates(RowType{block, 1})));
+  EXPECT_FALSE((RowType{block, 0}.equates(RowType{block, 1}, options)));
-  EXPECT_FALSE((RowType{block, 1}.equates(RowType{block, 0})));
+  EXPECT_FALSE((RowType{block, 1}.equates(RowType{block, 0}, options)));
 
   // different row in different block must be non-equivalent, even with the same index
-  EXPECT_FALSE((RowType{block, 0}.equates(RowType{otherBlock, 0})));
+  EXPECT_FALSE((RowType{block, 0}.equates(RowType{otherBlock, 0}, options)));
-  EXPECT_FALSE((RowType{otherBlock, 0}.equates(RowType{block, 0})));
+  EXPECT_FALSE((RowType{otherBlock, 0}.equates(RowType{block, 0}, options)));
 
   // an equivalent row in a different block must be considered equivalent, even with a different index
-  EXPECT_TRUE((RowType{block, 1}.equates(RowType{otherBlock, 0})));
+  EXPECT_TRUE((RowType{block, 1}.equates(RowType{otherBlock, 0}, options)));
-  EXPECT_TRUE((RowType{otherBlock, 0}.equates(RowType{block, 1})));
+  EXPECT_TRUE((RowType{otherBlock, 0}.equates(RowType{block, 1}, options)));
 
   // comparisons with an invalid row must be false
-  EXPECT_FALSE((RowType{block, 0}.equates(invalidRow)));
+  EXPECT_FALSE((RowType{block, 0}.equates(invalidRow, options)));
-  EXPECT_FALSE((invalidRow.equates(RowType{block, 0})));
+  EXPECT_FALSE((invalidRow.equates(RowType{block, 0}, options)));
 
   // two invalid rows must be equal
-  EXPECT_TRUE((invalidRow.equates(otherInvalidRow)));
+  EXPECT_TRUE((invalidRow.equates(otherInvalidRow, options)));
 }
 
 class AqlShadowRowsEqTest : public ::testing::Test {
  protected:
   ResourceMonitor monitor;
   AqlItemBlockManager itemBlockManager{&monitor, SerializationFormat::SHADOWROWS};
+  velocypack::Options const* const options{&velocypack::Options::Defaults};
 };
 
 TEST_F(AqlShadowRowsEqTest, shadow_row_depth_equivalence) {
@@ -460,20 +464,20 @@ TEST_F(AqlShadowRowsEqTest, shadow_row_depth_equivalence) {
   otherBlock->setShadowRowDepth(0, AqlValue{AqlValueHintUInt{1}});
 
   // same rows must be considered equivalent
-  EXPECT_TRUE((ShadowAqlItemRow{block, 0}.equates(ShadowAqlItemRow{block, 0})));
+  EXPECT_TRUE((ShadowAqlItemRow{block, 0}.equates(ShadowAqlItemRow{block, 0}, options)));
-  EXPECT_TRUE((ShadowAqlItemRow{block, 1}.equates(ShadowAqlItemRow{block, 1})));
+  EXPECT_TRUE((ShadowAqlItemRow{block, 1}.equates(ShadowAqlItemRow{block, 1}, options)));
 
   // different rows must be non-equivalent
-  EXPECT_FALSE((ShadowAqlItemRow{block, 0}.equates(ShadowAqlItemRow{block, 1})));
+  EXPECT_FALSE((ShadowAqlItemRow{block, 0}.equates(ShadowAqlItemRow{block, 1}, options)));
-  EXPECT_FALSE((ShadowAqlItemRow{block, 1}.equates(ShadowAqlItemRow{block, 0})));
+  EXPECT_FALSE((ShadowAqlItemRow{block, 1}.equates(ShadowAqlItemRow{block, 0}, options)));
 
   // different row in different block must be non-equivalent, even with the same index
-  EXPECT_FALSE((ShadowAqlItemRow{block, 0}.equates(ShadowAqlItemRow{otherBlock, 0})));
+  EXPECT_FALSE((ShadowAqlItemRow{block, 0}.equates(ShadowAqlItemRow{otherBlock, 0}, options)));
-  EXPECT_FALSE((ShadowAqlItemRow{otherBlock, 0}.equates(ShadowAqlItemRow{block, 0})));
+  EXPECT_FALSE((ShadowAqlItemRow{otherBlock, 0}.equates(ShadowAqlItemRow{block, 0}, options)));
 
   // an equivalent row in a different block must be considered equivalent, even with a different index
-  EXPECT_TRUE((ShadowAqlItemRow{block, 1}.equates(ShadowAqlItemRow{otherBlock, 0})));
+  EXPECT_TRUE((ShadowAqlItemRow{block, 1}.equates(ShadowAqlItemRow{otherBlock, 0}, options)));
-  EXPECT_TRUE((ShadowAqlItemRow{otherBlock, 0}.equates(ShadowAqlItemRow{block, 1})));
+  EXPECT_TRUE((ShadowAqlItemRow{otherBlock, 0}.equates(ShadowAqlItemRow{block, 1}, options)));
 }
 
 }  // namespace aql

tests/Aql/DependencyProxyMock.cpp

@@ -24,6 +24,8 @@
 
 #include "gtest/gtest.h"
 
+#include <velocypack/Options.h>
+
 namespace arangodb {
 namespace tests {
 namespace aql {
@@ -42,7 +44,7 @@ DependencyProxyMock<passBlocksThrough>::DependencyProxyMock(arangodb::aql::Resou
                                                             ::arangodb::aql::RegisterId nrRegisters)
     : DependencyProxy<passBlocksThrough>({}, _itemBlockManager,
                                          std::shared_ptr<std::unordered_set<RegisterId>>(),
-                                         nrRegisters),
+                                         nrRegisters, &velocypack::Options::Defaults),
       _itemsToReturn(),
       _numFetchBlockCalls(0),
       _monitor(monitor),
@@ -174,7 +176,7 @@ MultiDependencyProxyMock<passBlocksThrough>::MultiDependencyProxyMock(
     ::arangodb::aql::RegisterId nrRegisters, size_t nrDeps)
     : DependencyProxy<passBlocksThrough>({}, _itemBlockManager,
                                          std::shared_ptr<std::unordered_set<RegisterId>>(),
-                                         nrRegisters),
+                                         nrRegisters, &velocypack::Options::Defaults),
       _itemBlockManager(&monitor, SerializationFormat::SHADOWROWS) {
   _dependencyMocks.reserve(nrDeps);
   for (size_t i = 0; i < nrDeps; ++i) {

tests/Aql/ExecutionBlockImplTest.cpp

@@ -36,6 +36,7 @@
 #include "Aql/ExecutionEngine.h"
 #include "Aql/Query.h"
 #include "Aql/SingleRowFetcher.h"
+#include "Transaction/Context.h"
 #include "Transaction/Methods.h"
 
 using namespace arangodb;
@@ -65,6 +66,10 @@ class ExecutionBlockImplTest : public ::testing::Test {
   fakeit::Mock<transaction::Methods> mockTrx;
   transaction::Methods& trx;
 
+  // Mock of the transaction context
+  fakeit::Mock<transaction::Context> mockContext;
+  transaction::Context& context;
+
   // Mock of the Query
   fakeit::Mock<Query> mockQuery;
   Query& query;
@@ -90,6 +95,7 @@ class ExecutionBlockImplTest : public ::testing::Test {
       : engine(mockEngine.get()),
         itemBlockManager(mockBlockManager.get()),
         trx(mockTrx.get()),
+        context(mockContext.get()),
         query(mockQuery.get()),
         lqueryOptions(mockQueryOptions.get()),
         profile(ProfileLevel(PROFILE_LEVEL_NONE)),
@@ -116,6 +122,9 @@ class ExecutionBlockImplTest : public ::testing::Test {
     fakeit::When(Method(mockQuery, trx)).AlwaysReturn(&trx);
 
     fakeit::When(Method(mockQueryOptions, getProfileLevel)).AlwaysReturn(profile);
+
+    fakeit::When(Method(mockTrx, transactionContextPtr)).AlwaysReturn(&context);
+    fakeit::When(Method(mockContext, getVPackOptions)).AlwaysReturn(&velocypack::Options::Defaults);
   }
 };
 

tests/Aql/MultiDepFetcherHelper.cpp

@@ -35,6 +35,7 @@ using namespace arangodb::aql;
 using namespace arangodb::tests;
 using namespace arangodb::tests::aql;
 
+constexpr auto options = &velocypack::Options::Defaults;
 
 void arangodb::tests::aql::runFetcher(arangodb::aql::MultiDependencySingleRowFetcher& testee,
                                       std::vector<FetcherIOPair> const& inputOutputPairs) {
@@ -58,7 +59,9 @@ void arangodb::tests::aql::runFetcher(arangodb::aql::MultiDependencySingleRowFet
         auto const& actualState = actual.first;
         auto const& actualRow = actual.second;
         EXPECT_EQ(expectedState, actualState) << "during step " << i;
-        EXPECT_TRUE(expectedRow.equates(actualRow)) << "  expected: " << expectedRow << "\n  actual: " << actualRow << "\n  during step " << i;
+        EXPECT_TRUE(expectedRow.equates(actualRow, options))
+            << "  expected: " << expectedRow << "\n  actual: " << actualRow
+            << "\n  during step " << i;
       }
       void operator()(ConcreteFetcherIOPair<SkipRowsForDependency> const& iop) {
         auto const& args = iop.first;
@@ -75,7 +78,9 @@ void arangodb::tests::aql::runFetcher(arangodb::aql::MultiDependencySingleRowFet
         auto const& actualState = actual.first;
         auto const& actualRow = actual.second;
         EXPECT_EQ(expectedState, actualState) << "during step " << i;
-        EXPECT_TRUE(expectedRow.equates(actualRow)) << "  expected: " << expectedRow << "\n  actual: " << actualRow << "\n  during step " << i;
+        EXPECT_TRUE(expectedRow.equates(actualRow, options))
+            << "  expected: " << expectedRow << "\n  actual: " << actualRow
+            << "\n  during step " << i;
       }
 
      private:

tests/Aql/SortExecutorTest.cpp

@@ -54,12 +54,6 @@ namespace arangodb {
 namespace tests {
 namespace aql {
 
-int compareAqlValues(irs::sort::prepared const*, arangodb::transaction::Methods* trx,
-                     arangodb::aql::AqlValue const& lhs,
-                     arangodb::aql::AqlValue const& rhs) {
-  return arangodb::aql::AqlValue::Compare(trx, lhs, rhs, true);
-}
-
 class SortExecutorTest : public ::testing::Test {
  protected:
   ExecutionState state;
@@ -67,14 +61,7 @@ class SortExecutorTest : public ::testing::Test {
   AqlItemBlockManager itemBlockManager;
   SharedAqlItemBlockPtr block;
 
-  // Mock of the Transaction
+  velocypack::Options const* vpackOptions{&velocypack::Options::Defaults};
-  // Enough for this test, will only be passed through and accessed
-  // on documents alone.
-  fakeit::Mock<transaction::Methods> mockTrx;
-  transaction::Methods& trx;
-
-  fakeit::Mock<transaction::Context> mockContext;
-  transaction::Context& ctxt;
 
   Variable sortVar;
   SortElement sl;
@@ -84,13 +71,9 @@ class SortExecutorTest : public ::testing::Test {
   SortExecutorTest()
       : itemBlockManager(&monitor, SerializationFormat::SHADOWROWS),
         block(new AqlItemBlock(itemBlockManager, 1000, 1)),
-        trx(mockTrx.get()),
-        ctxt(mockContext.get()),
         sortVar("mySortVar", 0),
         sl(&sortVar, true),
         sortReg(0, sl) {
-    fakeit::When(Method(mockTrx, transactionContextPtr)).AlwaysReturn(&ctxt);
-    fakeit::When(Method(mockContext, getVPackOptions)).AlwaysReturn(&arangodb::velocypack::Options::Defaults);
     sortRegisters.emplace_back(std::move(sortReg));
   }
 };
@@ -98,7 +81,7 @@ class SortExecutorTest : public ::testing::Test {
 TEST_F(SortExecutorTest, no_rows_upstream_producer_doesnt_wait) {
   SortExecutorInfos infos(std::move(sortRegisters),
                           /*limit (ignored for default sort)*/ 0,
-                          itemBlockManager, 1, 1, {}, {0}, &trx, false);
+                          itemBlockManager, 1, 1, {}, {0}, vpackOptions, false);
   VPackBuilder input;
   AllRowsFetcherHelper fetcher(input.steal(), false);
   SortExecutor testee(fetcher, infos);
@@ -117,7 +100,7 @@ TEST_F(SortExecutorTest, no_rows_upstream_producer_doesnt_wait) {
 TEST_F(SortExecutorTest, no_rows_upstream_producer_waits) {
   SortExecutorInfos infos(std::move(sortRegisters),
                           /*limit (ignored for default sort)*/ 0,
-                          itemBlockManager, 1, 1, {}, {0}, &trx, false);
+                          itemBlockManager, 1, 1, {}, {0}, vpackOptions, false);
   VPackBuilder input;
   AllRowsFetcherHelper fetcher(input.steal(), true);
   SortExecutor testee(fetcher, infos);
@@ -141,7 +124,7 @@ TEST_F(SortExecutorTest, no_rows_upstream_producer_waits) {
 TEST_F(SortExecutorTest, rows_upstream_we_are_waiting_for_list_of_numbers) {
   SortExecutorInfos infos(std::move(sortRegisters),
                           /*limit (ignored for default sort)*/ 0,
-                          itemBlockManager, 1, 1, {}, {0}, &trx, false);
+                          itemBlockManager, 1, 1, {}, {0}, vpackOptions, false);
   std::shared_ptr<VPackBuilder> input =
       VPackParser::fromJson("[[5],[3],[1],[2],[4]]");
   AllRowsFetcherHelper fetcher(input->steal(), true);

tests/Geo/GeoConstructorTest.cpp

@@ -34,6 +34,7 @@
 #include "Aql/Functions.h"
 #include "Aql/Query.h"
 #include "Containers/SmallVector.h"
+#include "Transaction/Context.h"
 #include "Transaction/Methods.h"
 
 #include <velocypack/Builder.h>
@@ -57,12 +58,20 @@ class GeoConstructorTest : public ::testing::Test {
 
   fakeit::Mock<transaction::Methods> trxMock;
   transaction::Methods& trx;
+  fakeit::Mock<transaction::Context> contextMock;
+  transaction::Context& context;
 
   SmallVector<AqlValue>::allocator_type::arena_type arena;
   SmallVector<AqlValue> params;
 
   GeoConstructorTest()
-      : expressionContext(expressionContextMock.get()), trx(trxMock.get()), params{arena} {}
+      : expressionContext(expressionContextMock.get()),
+        trx(trxMock.get()),
+        context(contextMock.get()),
+        params{arena} {
+    fakeit::When(Method(trxMock, transactionContextPtr)).AlwaysReturn(&context);
+    fakeit::When(Method(contextMock, getVPackOptions)).AlwaysReturn(&velocypack::Options::Defaults);
+  }
 };
 
 namespace geo_point {

tests/Geo/GeoFunctionsTest.cpp

@@ -34,6 +34,7 @@
 #include "Aql/Functions.h"
 #include "Aql/Query.h"
 #include "Containers/SmallVector.h"
+#include "Transaction/Context.h"
 #include "Transaction/Methods.h"
 
 #include <velocypack/Builder.h>
@@ -64,14 +65,23 @@ protected:
 
   fakeit::Mock<transaction::Methods> trxMock;
   transaction::Methods& trx;
+  fakeit::Mock<transaction::Context> contextMock;
+  transaction::Context& context;
 
     SmallVector<AqlValue>::allocator_type::arena_type arena;
     SmallVector<AqlValue> paramsA;
     SmallVector<AqlValue> paramsB;
     SmallVector<AqlValue> paramsC;
 
-    GeoEqualsTest() : expressionContext(expressionContextMock.get()),
+    GeoEqualsTest()
-    trx(trxMock.get()), paramsA{arena}, paramsB{arena}, paramsC{arena} {
+        : expressionContext(expressionContextMock.get()),
+          trx(trxMock.get()),
+          context(contextMock.get()),
+          paramsA{arena},
+          paramsB{arena},
+          paramsC{arena} {
+      fakeit::When(Method(trxMock, transactionContextPtr)).AlwaysReturn(&context);
+      fakeit::When(Method(contextMock, getVPackOptions)).AlwaysReturn(&velocypack::Options::Defaults);
     }
 
     ~GeoEqualsTest() {

tests/js/server/aql/aql-gather-block-cluster.js

@@ -498,10 +498,39 @@ function gatherBlockTestSuite () {
     testSubqueryValuePropagation : function () {
       c4 = db._create(cn4, {numberOfShards:3});
       c4.insert({Hallo:1});
-      var query = "FOR i IN 1..1 LET s = (FOR j IN 1..i FOR k IN " + cn4 + " RETURN j) RETURN s";
+      const query = `FOR i IN 1..1 LET s = (FOR j IN 1..i FOR k IN ${cn4} RETURN j) RETURN s`;
       // check the return value
-      var expected = [ [ 1 ] ];
+      const expected = [ [ 1 ] ];
-      var actual = AQL_EXECUTE(query).json;
+      const rules = ['-splice-subqueries'];
+      const opts = {optimizer:{rules}};
+      const plan = AQL_EXPLAIN(query, {}, opts).plan;
+      const nodeTypes = plan.nodes.map(function(node) {
+        return node.type;
+      });
+      assertNotEqual(0, nodeTypes.filter(type => type === 'SubqueryNode').length);
+      assertEqual(0, nodeTypes.filter(type => type === 'SubqueryStartNode').length);
+      assertEqual(0, nodeTypes.filter(type => type === 'SubqueryEndNode').length);
+      const actual = AQL_EXECUTE(query, {}, opts).json;
+
+      assertEqual(expected, actual, query);
+    },
+
+    testSplicedSubqueryValuePropagation : function () {
+      c4 = db._create(cn4, {numberOfShards:3});
+      c4.insert({Hallo:1});
+      const query = `FOR i IN 1..1 LET s = (FOR j IN 1..i FOR k IN ${cn4} RETURN j) RETURN s`;
+      // check the return value
+      const expected = [ [ 1 ] ];
+      const rules = ['+splice-subqueries'];
+      const opts = {optimizer:{rules}};
+      const plan = AQL_EXPLAIN(query, {}, opts).plan;
+      const nodeTypes = plan.nodes.map(function(node) {
+        return node.type;
+      });
+      assertEqual(0, nodeTypes.filter(type => type === 'SubqueryNode').length);
+      assertNotEqual(0, nodeTypes.filter(type => type === 'SubqueryStartNode').length);
+      assertNotEqual(0, nodeTypes.filter(type => type === 'SubqueryEndNode').length);
+      const actual = AQL_EXECUTE(query, {}, opts).json;
 
       assertEqual(expected, actual, query);
     },

tests/js/server/aql/aql-optimizer-indexes.js

@@ -904,15 +904,6 @@ function optimizerIndexesTestSuite () {
       const subqueryStartIdx = nodeTypes.indexOf('SubqueryStartNode');
       const subqueryEndIdx = nodeTypes.indexOf('SubqueryEndNode');
       const hasSplicedSubquery = subqueryStartIdx !== -1 && subqueryEndIdx !== -1;
-      { // TODO Remove this block as soon as subquery splicing is enabled in the cluster again.
-        //  It's here so the test will fail as soon as that happens, so the actual test will not be forgotten
-        //  to be re-enabled.
-        const isCluster = require("@arangodb/cluster").isCluster();
-        if (isCluster) {
-          assertFalse(hasSplicedSubquery);
-          return;
-        }
-      }
       assertTrue(hasSplicedSubquery, JSON.stringify({
         subqueryStartIdx,
         subqueryEndIdx,

tests/js/server/aql/aql-optimizer-rule-use-index-for-sort.js

@@ -1073,15 +1073,6 @@ function optimizerRuleTestSuite() {
       const query = `FOR i IN [123] RETURN (FOR v IN ${colName} FILTER v.a == i SORT v.b ASC RETURN v)`;
       const plan = AQL_EXPLAIN(query, {}, {optimizer: {rules: ['+splice-subqueries']}}).plan;
       const rules = plan.rules;
-      { // TODO Remove this block as soon as subquery splicing is enabled in the cluster again.
-        //  It's here so the test will fail as soon as that happens, so the actual test will not be forgotten
-        //  to be re-enabled.
-        const isCluster = require("@arangodb/cluster").isCluster();
-        if (isCluster) {
-          assertEqual(-1, rules.indexOf("splice-subqueries"));
-          return;
-        }
-      }
       assertNotEqual(-1, rules.indexOf(ruleName));
       assertNotEqual(-1, rules.indexOf(secondRuleName));
       assertNotEqual(-1, rules.indexOf("remove-filter-covered-by-index"));
@@ -1123,15 +1114,6 @@ function optimizerRuleTestSuite() {
       const query = `FOR i IN [123] RETURN (FOR v IN ${colName} FILTER v.a == i SORT v.b DESC RETURN v)`;
       const plan = AQL_EXPLAIN(query, {}, {optimizer: {rules: ['+splice-subqueries']}}).plan;
       const rules = plan.rules;
-      { // TODO Remove this block as soon as subquery splicing is enabled in the cluster again.
-        //  It's here so the test will fail as soon as that happens, so the actual test will not be forgotten
-        //  to be re-enabled.
-        const isCluster = require("@arangodb/cluster").isCluster();
-        if (isCluster) {
-          assertEqual(-1, rules.indexOf("splice-subqueries"));
-          return;
-        }
-      }
       assertNotEqual(-1, rules.indexOf(ruleName));
       assertNotEqual(-1, rules.indexOf(secondRuleName));
       assertNotEqual(-1, rules.indexOf("remove-filter-covered-by-index"));

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

@@ -86,32 +86,27 @@ function ahuacatlProfilerTestSuite () {
     fuzzy = fuzzy || db._engine().name === 'mmfiles';
     return _.sum(
       _.values(rowsPerClient)
-        .map(fuzzy ? mmfilesBatches: optimalBatches)
+        .map(fuzzy ? mmfilesBatches : optimalBatches)
     );
   };
+  const dbServerBatch = (rows, fuzzy = false) => {
+    fuzzy = fuzzy || db._engine().name === 'mmfiles';
+    return (fuzzy ? mmfilesBatches : optimalBatches)(rows);
+  };
   const dbServerOptimalBatches = (rowsPerClient) =>
     _.sum(
       _.values(rowsPerClient)
         .map(optimalBatches)
     );
-  const groupedDBServerBatches = (rowsPerShard) => {
+  const groupedBatches = (rowsPerClient, fuzzy) => {
-    const shardIds = Object.keys(rowsPerShard);
+    const callInfo = {calls: 0, overhead: 0};
-    const shardToServerMapping = getResponsibleServers(shardIds);
-    const callsPerServer = {};
 
-    for (const [shard, rows] of Object.entries(rowsPerShard)) {
+    for (const [shard, rows] of Object.entries(rowsPerClient)) {
-      const server = shardToServerMapping[shard];
-      const callInfo = callsPerServer[server] ||  {calls: 0, overhead: 0};
       const testHere = rows + callInfo.overhead;
-      if (db._engine().name === 'mmfiles') {
+      callInfo.calls += dbServerBatch(testHere, fuzzy);
-        callInfo.calls += mmfilesBatches(testHere);
-      } else {
-        callInfo.calls += optimalBatches(testHere);
-      }
       callInfo.overhead = testHere % defaultBatchSize;
-      callsPerServer[server] = callInfo;
     }
-    return _.sum(_.values(callsPerServer).map(c => c.calls)); 
+    return callInfo.calls;
   };
 
   return {
@@ -178,32 +173,34 @@ function ahuacatlProfilerTestSuite () {
 
       // Number of local getSome calls that do not return WAITING.
       // This is at least 1.
-      // Batches are just passed through, but empty ones are skipped.
       // DONE can only be returned when the last shard is asked, so iff the last
       // asked shard is empty, there is one more call (the last call returns
       // DONE without any results).
       // As there is no guaranteed order in which the shards are processed, we
       // have to allow a range.
       const localCalls = (rowsPerShard) => {
-        const batches = _.sum(
+        const batches = optimalNonEmptyBatches(_.sum(_.values(rowsPerShard)));
-          _.values(rowsPerShard)
+        return [
-            .map(optimalNonEmptyBatches)
+          Math.max(1, batches),
-        );
+          Math.max(1, batches+1)
-        return [Math.max(1, batches), Math.max(1, batches+1)];
+        ];
       };
 
 
       // If we figure out that we are done depends on randomness.
       // In some cases we get the full batch on last shard, in this case the DBServer knows it is done.
-      // In other cases we get the full batch on an early shard, but 0 documents later, in chis case the DBServer does not know it is done
+      // In other cases we get the full batch on an early shard, but 0 documents later, in this case the DBServer does
-      // in advance.
+      // not know it is done in advance.
-      const fuzzyDBServerBatches = rowsPerServer => [dbServerBatches(rowsPerServer, false), dbServerBatches(rowsPerServer, true)];
+      const fuzzyDBServerBatches = rowsPerServer => [
+        groupedBatches(rowsPerServer, false),
+        groupedBatches(rowsPerServer, true)
+      ];
 
       const coordinatorBatches = (rowsPerShard) => addIntervals(fuzzyDBServerBatches(rowsPerShard), localCalls(rowsPerShard));
 
       const genNodeList = (rowsPerShard, rowsPerServer) => [
         { type : SingletonBlock, calls : numberOfShards, items : numberOfShards },
-        { type : EnumerateCollectionBlock, calls : groupedDBServerBatches(rowsPerShard), items : totalItems(rowsPerShard) },
+        { type : EnumerateCollectionBlock, calls : groupedBatches(rowsPerShard), items : totalItems(rowsPerShard) },
         // Twice the number due to WAITING, fuzzy, because the Gather does not know
         { type : RemoteBlock, calls : fuzzyDBServerBatches(rowsPerServer).map(i => i * 2), items : totalItems(rowsPerShard) },
         // We get dbServerBatches(rowsPerShard) times WAITING, plus the non-waiting getSome calls.

tests/js/server/aql/aql-subquery.js

@@ -196,18 +196,6 @@ function ahuacatlSubqueryTestSuite () {
     testSpliceSubqueryOutVariableName : function () {
       const explainResult = AQL_EXPLAIN("FOR u IN _users LET theLetVariable = (FOR j IN _users RETURN j) RETURN theLetVariable");
 
-      { // TODO Remove this block as soon as subquery splicing is enabled in the cluster again.
-        //  It's here so the test will fail as soon as that happens, so the actual test will not be forgotten
-        //  to be re-enabled.
-        const isCluster = require("@arangodb/cluster").isCluster();
-        if (isCluster) {
-          const numSubqueryEndNode = findExecutionNodes(explainResult, "SubqueryEndNode").length;
-
-          assertEqual(0, numSubqueryEndNode);
-          return;
-        }
-      }
-
       const subqueryEndNode = findExecutionNodes(explainResult, "SubqueryEndNode")[0];
 
       assertEqual(subqueryEndNode.outVariable.name, "theLetVariable");