Bug fix/non recursive plan handling (#14475)

Co-authored-by: Jan <jsteemann@users.noreply.github.com>

CHANGELOG

@@ -1,5 +1,8 @@
 devel
 -----
+* Adapt various places related to handling of execution plans non-recursive
+  in order to avoid stack overflows. This allows us now to execute much larger
+  queries.
 
 * Lower log level to warning, when take over shard leadership finds
   an agency Current entry is missing the server taking over.

arangod/Aql/ClusterNodes.cpp

@@ -153,18 +153,14 @@ std::unique_ptr<ExecutionBlock> RemoteNode::createBlock(
                                                               queryId());
 }
 
-/// @brief toVelocyPack, for RemoteNode
-void RemoteNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
-                                    std::unordered_set<ExecutionNode const*>& seen) const {
+/// @brief doToVelocyPack, for RemoteNode
+void RemoteNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const {
   // call base class method
-  DistributeConsumerNode::toVelocyPackHelperInternal(nodes, flags, seen);
+  DistributeConsumerNode::doToVelocyPack(nodes, flags);
 
   nodes.add("database", VPackValue(_vocbase->name()));
   nodes.add("server", VPackValue(_server));
   nodes.add("queryId", VPackValue(_queryId));
-
-  // And close it:
-  nodes.close();
 }
 
 /// @brief estimateCost
@@ -202,16 +198,10 @@ std::unique_ptr<ExecutionBlock> ScatterNode::createBlock(
                                                                std::move(executorInfos));
 }
 
-/// @brief toVelocyPack, for ScatterNode
-void ScatterNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
-                                     std::unordered_set<ExecutionNode const*>& seen) const {
-  // call base class method
-  ExecutionNode::toVelocyPackHelperGeneric(nodes, flags, seen);
-
+/// @brief doToVelocyPack, for ScatterNode
+void ScatterNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const {
   // serialize clients
   writeClientsToVelocyPack(nodes);
-  // And close it:
-  nodes.close();
 }
 
 bool ScatterNode::readClientsFromVelocyPack(VPackSlice base) {
@@ -312,23 +302,15 @@ std::unique_ptr<ExecutionBlock> DistributeNode::createBlock(
                                                                   std::move(infos));
 }
 
-/// @brief toVelocyPack, for DistributedNode
-void DistributeNode::toVelocyPackHelper(VPackBuilder& builder, unsigned flags,
-                                        std::unordered_set<ExecutionNode const*>& seen) const {
+/// @brief doToVelocyPack, for DistributeNode
+void DistributeNode::doToVelocyPack(VPackBuilder& builder, unsigned flags) const {
   // call base class method
-  ExecutionNode::toVelocyPackHelperGeneric(builder, flags, seen);
-
+  ScatterNode::doToVelocyPack(builder, flags);
   // add collection information
   CollectionAccessingNode::toVelocyPack(builder, flags);
 
-  // serialize clients
-  writeClientsToVelocyPack(builder);
-
   builder.add(VPackValue("variable"));
-  _variable->toVelocyPack(builder);;
-
-  // And close it:
-  builder.close();
+  _variable->toVelocyPack(builder);
 }
 
 void DistributeNode::replaceVariables(std::unordered_map<VariableId, Variable const*> const& replacements) {
@@ -435,12 +417,8 @@ GatherNode::GatherNode(ExecutionPlan* plan, ExecutionNodeId id,
       _parallelism(parallelism),
       _limit(0) {}
 
-/// @brief toVelocyPack, for GatherNode
-void GatherNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
-                                    std::unordered_set<ExecutionNode const*>& seen) const {
-  // call base class method
-  ExecutionNode::toVelocyPackHelperGeneric(nodes, flags, seen);
-
+/// @brief doToVelocyPack, for GatherNode
+void GatherNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const {
   nodes.add("parallelism", VPackValue(toString(_parallelism)));
 
   if (_elements.empty()) {
@@ -467,9 +445,6 @@ void GatherNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
       }
     }
   }
-
-  // And close it:
-  nodes.close();
 }
 
 /// @brief creates corresponding ExecutionBlock
@@ -656,11 +631,8 @@ std::unique_ptr<ExecutionBlock> SingleRemoteOperationNode::createBlock(
   }
 }
 
-/// @brief toVelocyPack, for SingleRemoteOperationNode
-void SingleRemoteOperationNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
-                                                   std::unordered_set<ExecutionNode const*>& seen) const {
-  // call base class method
-  ExecutionNode::toVelocyPackHelperGeneric(nodes, flags, seen);
+/// @brief doToVelocyPack, for SingleRemoteOperationNode
+void SingleRemoteOperationNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const {
   CollectionAccessingNode::toVelocyPackHelperPrimaryIndex(nodes);
 
   // add collection information
@@ -703,9 +675,6 @@ void SingleRemoteOperationNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned
   nodes.add("projections", VPackValue(VPackValueType::Array));
   // TODO: support projections?
   nodes.close();
-
-  // And close it:
-  nodes.close();
 }
 
 /// @brief estimateCost

arangod/Aql/ClusterNodes.h

@@ -72,10 +72,6 @@ class RemoteNode final : public DistributeConsumerNode {
   /// @brief return the type of the node
   NodeType getType() const override final { return REMOTE; }
 
-  /// @brief export to VelocyPack
-  void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
-                          std::unordered_set<ExecutionNode const*>& seen) const override final;
-
   /// @brief creates corresponding ExecutionBlock
   std::unique_ptr<ExecutionBlock> createBlock(
       ExecutionEngine& engine,
@@ -112,6 +108,10 @@ class RemoteNode final : public DistributeConsumerNode {
     _queryId = arangodb::basics::StringUtils::itoa(queryId);
   }
 
+ protected:
+  /// @brief export to VelocyPack
+  void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override final;
+
  private:
   /// @brief the underlying database
   TRI_vocbase_t* _vocbase;
@@ -137,10 +137,6 @@ class ScatterNode : public ExecutionNode {
   /// @brief return the type of the node
   NodeType getType() const override { return SCATTER; }
 
-  /// @brief export to VelocyPack
-  void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
-                          std::unordered_set<ExecutionNode const*>& seen) const override;
-
   /// @brief creates corresponding ExecutionBlock
   std::unique_ptr<ExecutionBlock> createBlock(
       ExecutionEngine& engine,
@@ -176,6 +172,9 @@ class ScatterNode : public ExecutionNode {
   void setScatterType(ScatterType targetType) { _type = targetType; }
 
  protected:
+  /// @brief export to VelocyPack
+  void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override;
+
   void writeClientsToVelocyPack(velocypack::Builder& builder) const;
   bool readClientsFromVelocyPack(velocypack::Slice base);
 
@@ -205,10 +204,6 @@ class DistributeNode final : public ScatterNode, public CollectionAccessingNode
   /// @brief return the type of the node
   NodeType getType() const override final { return DISTRIBUTE; }
 
-  /// @brief export to VelocyPack
-  void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
-                          std::unordered_set<ExecutionNode const*>& seen) const override final;
-
   /// @brief creates corresponding ExecutionBlock
   std::unique_ptr<ExecutionBlock> createBlock(
       ExecutionEngine& engine,
@@ -232,6 +227,10 @@ class DistributeNode final : public ScatterNode, public CollectionAccessingNode
 
   ExecutionNodeId getTargetNodeId() const noexcept { return _targetNodeId; }
 
+ protected:
+  /// @brief export to VelocyPack
+  void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override final;
+
  private:
   /// @brief the variable we must inspect to know where to distribute
   Variable const* _variable;
@@ -274,10 +273,6 @@ class GatherNode final : public ExecutionNode {
   /// @brief return the type of the node
   NodeType getType() const override final { return GATHER; }
 
-  /// @brief export to VelocyPack
-  void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
-                          std::unordered_set<ExecutionNode const*>& seen) const override final;
-
   /// @brief clone ExecutionNode recursively
   ExecutionNode* clone(ExecutionPlan* plan, bool withDependencies,
                        bool withProperties) const override final {
@@ -320,6 +315,10 @@ class GatherNode final : public ExecutionNode {
     return _parallelism;
   }
 
+ protected: 
+  /// @brief export to VelocyPack
+  void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override final;
+
  private:
   /// @brief the underlying database
   TRI_vocbase_t* _vocbase;
@@ -364,10 +363,6 @@ class SingleRemoteOperationNode final : public ExecutionNode, public CollectionA
   /// @brief return the type of the node
   NodeType getType() const override final { return REMOTESINGLE; }
 
-  /// @brief export to VelocyPack
-  void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
-                          std::unordered_set<ExecutionNode const*>& seen) const override final;
-
   /// @brief creates corresponding ExecutionBlock
   std::unique_ptr<ExecutionBlock> createBlock(
       ExecutionEngine& engine,
@@ -396,6 +391,10 @@ class SingleRemoteOperationNode final : public ExecutionNode, public CollectionA
 
   std::string const& key() const { return _key; }
 
+ protected: 
+  /// @brief export to VelocyPack
+  void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override final;
+
  private:
   // whether we replaced an index node
   bool _replaceIndexNode;

arangod/Aql/CollectNode.cpp

@@ -85,12 +85,8 @@ CollectNode::CollectNode(
 
 CollectNode::~CollectNode() = default;
 
-/// @brief toVelocyPack, for CollectNode
-void CollectNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
-                                     std::unordered_set<ExecutionNode const*>& seen) const {
-  // call base class method
-  ExecutionNode::toVelocyPackHelperGeneric(nodes, flags, seen);
-
+/// @brief doToVelocyPack, for CollectNode
+void CollectNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const {
   // group variables
   nodes.add(VPackValue("groups"));
   {
@@ -148,9 +144,6 @@ void CollectNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
   nodes.add("specialized", VPackValue(_specialized));
   nodes.add(VPackValue("collectOptions"));
   _options.toVelocyPack(nodes);
-
-  // And close it:
-  nodes.close();
 }
 
 void CollectNode::calcExpressionRegister(arangodb::aql::RegisterId& expressionRegister,

arangod/Aql/CollectNode.h

@@ -87,10 +87,6 @@ class CollectNode : public ExecutionNode {
   /// @brief getOptions
   CollectOptions& getOptions();
 
-  /// @brief export to VelocyPack
-  void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
-                          std::unordered_set<ExecutionNode const*>& seen) const override final;
-
   /// @brief calculate the expression register
   void calcExpressionRegister(RegisterId& expressionRegister,
                               RegIdSet& writeableOutputRegisters) const;
@@ -187,6 +183,10 @@ class CollectNode : public ExecutionNode {
   static void calculateAccessibleUserVariables(ExecutionNode const& node,
                                                std::vector<Variable const*>& userVariables);
 
+ protected:
+  /// @brief export to VelocyPack
+  void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override final;
+
  private:
   /// @brief options for the aggregation
   CollectOptions _options;

arangod/Aql/DistributeConsumerNode.cpp

@@ -41,18 +41,7 @@ DistributeConsumerNode::DistributeConsumerNode(ExecutionPlan* plan,
       _isResponsibleForInitializeCursor(
           base.get("isResponsibleForInitializeCursor").getBoolean()) {}
 
-void DistributeConsumerNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
-                                                std::unordered_set<ExecutionNode const*>& seen) const {
-  // Local variant
-  toVelocyPackHelperInternal(nodes, flags, seen);
-  // And close it:
-  nodes.close();
-}
-
-void DistributeConsumerNode::toVelocyPackHelperInternal(
-    VPackBuilder& nodes, unsigned flags, std::unordered_set<ExecutionNode const*>& seen) const {
-  // call base class method
-  ExecutionNode::toVelocyPackHelperGeneric(nodes, flags, seen);
+void DistributeConsumerNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const {
   nodes.add("distributeId", VPackValue(_distributeId));
   nodes.add("isResponsibleForInitializeCursor", VPackValue(_isResponsibleForInitializeCursor));
 }

arangod/Aql/DistributeConsumerNode.h

@@ -48,10 +48,6 @@ class DistributeConsumerNode : public ExecutionNode {
   /// @brief return the type of the node
   NodeType getType() const override { return DISTRIBUTE_CONSUMER; }
 
-  /// @brief export to VelocyPack
-  void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
-                          std::unordered_set<ExecutionNode const*>& seen) const override;
-
   /// @brief creates corresponding ExecutionBlock
   std::unique_ptr<ExecutionBlock> createBlock(
       ExecutionEngine& engine,
@@ -92,8 +88,8 @@ class DistributeConsumerNode : public ExecutionNode {
   }
 
  protected:
-  void toVelocyPackHelperInternal(arangodb::velocypack::Builder& nodes, unsigned flags,
-                                  std::unordered_set<ExecutionNode const*>& seen) const;
+  /// @brief export to VelocyPack
+  void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override;
 
  private:
   /// @brief our own distributeId. If it is set, we will fetch only ata for