arangodb · mchacki · Mar 15, 2020 · Jan 6, 2020 · Jan 6, 2020 · Jan 27, 2020
diff --git a/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp b/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp
@@ -194,7 +194,7 @@ EngineInfoContainerDBServerServerBased::EngineInfoContainerDBServerServerBased(Q
   // NOTE: We need to start with _lastSnippetID > 0. 0 is reserved for GraphNodes
 }
 
-void EngineInfoContainerDBServerServerBased::injectVertexColletions(GraphNode* graphNode) {
+void EngineInfoContainerDBServerServerBased::injectVertexCollections(GraphNode* graphNode) {
   auto const& vCols = graphNode->vertexColls();
   if (vCols.empty()) {
     std::map<std::string, Collection*> const* allCollections =
@@ -226,7 +226,9 @@ void EngineInfoContainerDBServerServerBased::addNode(ExecutionNode* node) {
     case ExecutionNode::TRAVERSAL:
     case ExecutionNode::SHORTEST_PATH:
     case ExecutionNode::K_SHORTEST_PATHS: {
-      injectVertexColletions(static_cast<GraphNode*>(node));
+      auto* const graphNode = ExecutionNode::castTo<GraphNode*>(node);
+      graphNode->prepareOptions();
+      injectVertexCollections(graphNode);
       break;
     }
     default:
@@ -516,7 +518,7 @@ void EngineInfoContainerDBServerServerBased::cleanupEngines(
 // the DBServers. The GraphNode itself will retain on the coordinator.
 void EngineInfoContainerDBServerServerBased::addGraphNode(GraphNode* node) {
   node->prepareOptions();
-  injectVertexColletions(node);
+  injectVertexCollections(node);
   // SnippetID does not matter on GraphNodes
   _shardLocking.addNode(node, 0);
   _graphNodes.emplace_back(node);

diff --git a/arangod/Aql/EngineInfoContainerDBServerServerBased.h b/arangod/Aql/EngineInfoContainerDBServerServerBased.h
@@ -173,7 +173,7 @@ class EngineInfoContainerDBServerServerBased {
                        ServerID const& server, std::string const& serverDest,
                        std::vector<bool> const& didCreateEngine) const;
 
-  void injectVertexColletions(GraphNode* node);
+  void injectVertexCollections(GraphNode* node);
  private:
   std::stack<std::shared_ptr<QuerySnippet>, std::vector<std::shared_ptr<QuerySnippet>>> _snippetStack;
 

diff --git a/arangod/Aql/ExecutionNode.cpp b/arangod/Aql/ExecutionNode.cpp
@@ -463,6 +463,12 @@ ExecutionNode::ExecutionNode(ExecutionPlan* plan, VPackSlice const& slice)
       VelocyPackHelper::getBooleanValue(slice, "isInSplicedSubquery", false);
 }
 
+ExecutionNode::ExecutionNode(ExecutionPlan& plan, ExecutionNode const& other)
+    : ExecutionNode(&plan, other.id()) {
+  TRI_ASSERT(&plan == other.plan());
+  other.cloneWithoutRegisteringAndDependencies(plan, *this, false);
+}
+
 /// @brief toVelocyPack, export an ExecutionNode to VelocyPack
 void ExecutionNode::toVelocyPack(VPackBuilder& builder, unsigned flags,
                                  bool keepTopLevelOpen) const {
@@ -485,59 +491,64 @@ ExecutionNode* ExecutionNode::cloneHelper(std::unique_ptr<ExecutionNode> other,
                                           bool withDependencies, bool withProperties) const {
   ExecutionPlan* plan = other->plan();
 
-  if (plan == _plan) {
+  cloneWithoutRegisteringAndDependencies(*plan, *other, withProperties);
+
+  auto* registeredNode = plan->registerNode(std::move(other));
+
+  if (withDependencies) {
+    cloneDependencies(plan, registeredNode, withProperties);
+  }
+
+  return registeredNode;
+}
+
+void ExecutionNode::cloneWithoutRegisteringAndDependencies(ExecutionPlan& plan, ExecutionNode& other, bool withProperties) const {
+
+  if (&plan == _plan) {
     // same execution plan for source and target
     // now assign a new id to the cloned node, otherwise it will fail
     // upon node registration and/or its meaning is ambiguous
-    other->setId(plan->nextId());
+    other.setId(plan.nextId());
 
     // cloning with properties will only work if we clone a node into
     // a different plan
     TRI_ASSERT(!withProperties);
   }
 
-  other->_regsToClear = _regsToClear;
-  other->_depth = _depth;
-  other->_varUsageValid = _varUsageValid;
-  other->_isInSplicedSubquery = _isInSplicedSubquery;
+  other._regsToClear = _regsToClear;
+  other._depth = _depth;
+  other._varUsageValid = _varUsageValid;
+  other._isInSplicedSubquery = _isInSplicedSubquery;
 
   if (withProperties) {
-    auto allVars = plan->getAst()->variables();
+    auto allVars = plan.getAst()->variables();
     // Create new structures on the new AST...
-    other->_varsUsedLater.reserve(_varsUsedLater.size());
+    other._varsUsedLater.reserve(_varsUsedLater.size());
     for (auto const& orgVar : _varsUsedLater) {
       auto var = allVars->getVariable(orgVar->id);
       TRI_ASSERT(var != nullptr);
-      other->_varsUsedLater.insert(var);
+      other._varsUsedLater.insert(var);
     }
 
-    other->_varsValid.reserve(_varsValid.size());
+    other._varsValid.reserve(_varsValid.size());
 
     for (auto const& orgVar : _varsValid) {
       auto var = allVars->getVariable(orgVar->id);
       TRI_ASSERT(var != nullptr);
-      other->_varsValid.insert(var);
+      other._varsValid.insert(var);
     }
 
     if (_registerPlan.get() != nullptr) {
       auto otherRegisterPlan =
-          std::shared_ptr<RegisterPlan>(_registerPlan->clone(plan, _plan));
-      other->_registerPlan = otherRegisterPlan;
+          std::shared_ptr<RegisterPlan>(_registerPlan->clone(&plan, _plan));
+      other._registerPlan = otherRegisterPlan;
     }
   } else {
     // point to current AST -> don't do deep copies.
-    other->_varsUsedLater = _varsUsedLater;
-    other->_varsValid = _varsValid;
-    other->_registerPlan = _registerPlan;
+    other._varsUsedLater = _varsUsedLater;
+    other._varsValid = _varsValid;
+    other._registerPlan = _registerPlan;
   }
-
-  auto* registeredNode = plan->registerNode(std::move(other));
-
-  if (withDependencies) {
-    cloneDependencies(plan, registeredNode, withProperties);
-  }
-
-  return registeredNode;
 }
 
 /// @brief helper for cloning, use virtual clone methods for dependencies
@@ -1067,8 +1078,6 @@ RegisterId ExecutionNode::getNrOutputRegisters() const {
 ExecutionNode::ExecutionNode(ExecutionPlan* plan, size_t id)
     : _id(id), _depth(0), _varUsageValid(false), _plan(plan), _isInSplicedSubquery(false) {}
 
-ExecutionNode::~ExecutionNode() = default;
-
 size_t ExecutionNode::id() const { return _id; }
 
 void ExecutionN
10000
ode::swapFirstDependency(ExecutionNode* node) {

diff --git a/arangod/Aql/ExecutionNode.h b/arangod/Aql/ExecutionNode.h
@@ -68,6 +68,7 @@
 #include "Aql/WalkerWorker.h"
 #include "Aql/types.h"
 #include "Basics/Common.h"
+#include "Basics/TypeTraits.h"
 #include "Containers/HashSet.h"
 
 namespace arangodb {
@@ -162,6 +163,14 @@ class ExecutionNode {
   ExecutionNode() = delete;
   ExecutionNode(ExecutionNode const&) = delete;
   ExecutionNode& operator=(ExecutionNode const&) = delete;
+ protected:
+  /// @brief Clone constructor, used for constructors of derived classes.
+  /// Does not clone recursively, does not clone properties (`other.plan()` is
+  /// expected to be the same as `plan)`, and does not register this node in the
+  /// plan.
+  ExecutionNode(ExecutionPlan& plan, ExecutionNode const& other);
+
+ public:
 
   /// @brief constructor using an id
   ExecutionNode(ExecutionPlan* plan, size_t id);
@@ -170,7 +179,7 @@ class ExecutionNode {
   ExecutionNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& slice);
 
   /// @brief destructor, free dependencies
-  virtual ~ExecutionNode();
+  virtual ~ExecutionNode() = default;
 
  public:
   /// @brief factory from JSON
@@ -195,7 +204,13 @@ class ExecutionNode {
     TRI_ASSERT(result != nullptr);
     return result;
 #else
-    return static_cast<T>(node);
+    // At least GraphNode is virtually inherited by its subclasses. We have to
+    // use dynamic_cast for these types.
+    if constexpr (can_static_cast_v<FromType, T>) {
+      return static_cast<T>(node);
+    } else {
+      return dynamic_cast<T>(node);
+    }
 #endif
   }
 
@@ -303,6 +318,9 @@ class ExecutionNode {
   ExecutionNode* cloneHelper(std::unique_ptr<ExecutionNode> Other,
                              bool withDependencies, bool withProperties) const;
 
+  void cloneWithoutRegisteringAndDependencies(ExecutionPlan& plan, ExecutionNode& other,
+                                      bool withProperties) const;
+
   /// @brief helper for cloning, use virtual clone methods for dependencies
   void cloneDependencies(ExecutionPlan* plan, ExecutionNode* theClone, bool withProperties) const;
 

diff --git a/arangod/Aql/ExecutionPlan.cpp b/arangod/Aql/ExecutionPlan.cpp
@@ -2443,6 +2443,29 @@ bool ExecutionPlan::fullCount() const noexcept {
   return lastLimitNode != nullptr && lastLimitNode->fullCount();
 }
 
+void ExecutionPlan::prepareTraversalOptions() {
+  ::arangodb::containers::SmallVector<ExecutionNode*>::allocator_type::arena_type a;
+  ::arangodb::containers::SmallVector<ExecutionNode*> nodes{a};
+  findNodesOfType(nodes,
+      {arangodb::aql::ExecutionNode::TRAVERSAL,
+       arangodb::aql::ExecutionNode::SHORTEST_PATH,
+       arangodb::aql::ExecutionNode::K_SHORTEST_PATHS},
+      true);
+  for (auto& node : nodes) {
+    switch (node->getType()) {
+      case ExecutionNode::TRAVERSAL:
+      case ExecutionNode::SHORTEST_PATH:
+      case ExecutionNode::K_SHORTEST_PATHS: {
+        auto* graphNode = ExecutionNode::castTo<GraphNode*>(node);
+        graphNode->prepareOptions();
+      } break;
+      default:
+        TRI_ASSERT(false);
+        THROW_ARANGO_EXCEPTION(TRI_ERROR_INTERNAL_AQL);
+    }
+  }
+}
+
 #ifdef ARANGODB_ENABLE_MAINTAINER_MODE
 #include <iostream>
 

diff --git a/arangod/Aql/ExecutionPlan.h b/arangod/Aql/ExecutionPlan.h
@@ -196,6 +196,8 @@ class ExecutionPlan {
   /// @brief static analysis
   void planRegisters() { _root->planRegisters(); }
 
+  void prepareTraversalOptions();
+
   /// @brief unlinkNodes, note that this does not delete the removed
   /// nodes and that one cannot remove the root node of the plan.
   void unlinkNodes(std::unordered_set<ExecutionNode*> const& toUnlink);