Decouple visiting of distributed operators from regular

Summary:
Visitor pattern's main issue is cyclical dependency between classes that
are visited and the visitor instance itself. We need to decouple this
dependency if we want to open source part of the code, namely
non-distributed part. This decoupling is achieved through the use of
`dynamic_cast` in distributed operators. Hopefully the solution is good
enough and doesn't cause performance issues. An alternative solution is
to build our own custom double dispatch solution, but that will
basically boil down to our implementation of runtime type information
and casts.

Note, this only decouples the distributed operators. If and when we
decide that other operators shouldn't be open sourced, the same
`dynamic_cast` pattern should be applied in them also.

Depends on D1563

Reviewers: mtomic, msantl, buda

Reviewed By: mtomic

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D1566

src/query/distributed_interpreter.cpp

@@ -2,6 +2,7 @@
 
 #include "database/distributed_graph_db.hpp"
 #include "distributed/plan_dispatcher.hpp"
+#include "query/plan/distributed.hpp"
 #include "query/plan/planner.hpp"
 #include "query/plan/rule_based_planner.hpp"
 #include "query/plan/vertex_count_cache.hpp"

src/query/interpreter.hpp

@@ -9,7 +9,6 @@
 #include "query/frontend/ast/ast.hpp"
 #include "query/frontend/stripped.hpp"
 #include "query/interpret/frame.hpp"
-#include "query/plan/distributed.hpp"
 #include "query/plan/operator.hpp"
 #include "utils/thread/sync.hpp"
 #include "utils/timer.hpp"
@@ -17,10 +16,6 @@
 DECLARE_bool(query_cost_planner);
 DECLARE_int32(query_plan_cache_ttl);
 
-namespace distributed {
-class PlanDispatcher;
-}
-
 namespace auth {
 class Auth;
 }  // namespace auth
@@ -44,8 +39,6 @@ class LogicalPlan {
 
 class Interpreter {
  private:
-  /// Encapsulates a plan for caching. Takes care of remote (worker) cache
-  /// updating in distributed memgraph.
   class CachedPlan {
    public:
     CachedPlan(std::unique_ptr<LogicalPlan> plan);

src/query/plan/distributed.cpp

@@ -72,7 +72,7 @@ Branch FindIndependentSubtree(
 // indexed lookup is split to regular lookup + filtering.
 // TODO: After indexed lookup is moved to another stage, then this class should
 // never visit such lookups or modify the tree.
-class IndependentSubtreeFinder : public HierarchicalLogicalOperatorVisitor {
+class IndependentSubtreeFinder : public DistributedOperatorVisitor {
  public:
   IndependentSubtreeFinder(
       const std::vector<std::vector<Symbol>> &forbidden_symbols,

src/query/plan/distributed_ops.cpp

@@ -18,19 +18,25 @@ DEFINE_HIDDEN_int32(remote_pull_sleep_micros, 10,
 // that accepts the visitor and visits it's input_ operator
 #define ACCEPT_WITH_INPUT(class_name)                                    \
   bool class_name::Accept(HierarchicalLogicalOperatorVisitor &visitor) { \
-    if (visitor.PreVisit(*this)) {                                       \
+    auto *distributed_visitor =                                          \
+        dynamic_cast<DistributedOperatorVisitor *>(&visitor);            \
+    CHECK(distributed_visitor);                                          \
+    if (distributed_visitor->PreVisit(*this)) {                          \
       input_->Accept(visitor);                                           \
     }                                                                    \
-    return visitor.PostVisit(*this);                                     \
+    return distributed_visitor->PostVisit(*this);                        \
   }
 
 namespace query::plan {
 
 bool PullRemote::Accept(HierarchicalLogicalOperatorVisitor &visitor) {
-  if (visitor.PreVisit(*this)) {
+  auto *distributed_visitor =
+      dynamic_cast<DistributedOperatorVisitor *>(&visitor);
+  CHECK(distributed_visitor);
+  if (distributed_visitor->PreVisit(*this)) {
     if (input_) input_->Accept(visitor);
   }
-  return visitor.PostVisit(*this);
+  return distributed_visitor->PostVisit(*this);
 }
 
 std::vector<Symbol> PullRemote::OutputSymbols(const SymbolTable &table) const {
@@ -58,12 +64,15 @@ std::vector<Symbol> Synchronize::ModifiedSymbols(
 }
 
 bool Synchronize::Accept(HierarchicalLogicalOperatorVisitor &visitor) {
-  if (visitor.PreVisit(*this)) {
+  auto *distributed_visitor =
+      dynamic_cast<DistributedOperatorVisitor *>(&visitor);
+  CHECK(distributed_visitor);
+  if (distributed_visitor->PreVisit(*this)) {
     // pull_remote_ is optional here, so visit it only if we continue visiting
     // and pull_remote_ does exist.
     input_->Accept(visitor) && pull_remote_ && pull_remote_->Accept(visitor);
   }
-  return visitor.PostVisit(*this);
+  return distributed_visitor->PostVisit(*this);
 }
 
 PullRemoteOrderBy::PullRemoteOrderBy(

src/query/plan/distributed_ops.lcp

@@ -11,6 +11,23 @@ cpp<#
 (lcp:namespace query)
 (lcp:namespace plan)
 
+#>cpp
+class PullRemote;
+class Synchronize;
+class PullRemoteOrderBy;
+
+using DistributedOperatorCompositeVisitor =
+    ::utils::CompositeVisitor<PullRemote, Synchronize, PullRemoteOrderBy>;
+
+/// Base class for visiting regular and distributed LogicalOperator instances.
+class DistributedOperatorVisitor : public HierarchicalLogicalOperatorVisitor,
+                                   public DistributedOperatorCompositeVisitor {
+ public:
+  using DistributedOperatorCompositeVisitor::PostVisit;
+  using DistributedOperatorCompositeVisitor::PreVisit;
+};
+cpp<#
+
 (lcp:define-class pull-remote (logical-operator)
   ((input "std::shared_ptr<LogicalOperator>"
           :capnp-save #'save-operator-pointer

src/query/plan/operator.lcp

@@ -103,10 +103,7 @@ class Unwind;
 class Distinct;
 class CreateIndex;
 class Union;
-class PullRemote;
-class Synchronize;
 class Cartesian;
-class PullRemoteOrderBy;
 class AuthHandler;
 class CreateStream;
 class DropStream;
@@ -123,8 +120,7 @@ using LogicalOperatorCompositeVisitor = ::utils::CompositeVisitor<
     SetProperties, SetLabels, RemoveProperty, RemoveLabels,
     ExpandUniquenessFilter<VertexAccessor>,
     ExpandUniquenessFilter<EdgeAccessor>, Accumulate, Aggregate, Skip, Limit,
-    OrderBy, Merge, Optional, Unwind, Distinct, Union, PullRemote, Synchronize,
-    Cartesian, PullRemoteOrderBy, Explain>;
+    OrderBy, Merge, Optional, Unwind, Distinct, Union, Cartesian, Explain>;
 
 using LogicalOperatorLeafVisitor =
     ::utils::LeafVisitor<Once, CreateIndex, AuthHandler, CreateStream,

src/query/plan/pretty_print.cpp

@@ -8,7 +8,7 @@ namespace query::plan {
 
 namespace {
 
-class PlanPrinter final : public HierarchicalLogicalOperatorVisitor {
+class PlanPrinter final : public DistributedOperatorVisitor {
  public:
   using HierarchicalLogicalOperatorVisitor::PostVisit;
   using HierarchicalLogicalOperatorVisitor::PreVisit;

tests/unit/query_planner.cpp

@@ -45,7 +45,7 @@ class BaseOpChecker {
   virtual void CheckOp(LogicalOperator &, const SymbolTable &) = 0;
 };
 
-class PlanChecker : public HierarchicalLogicalOperatorVisitor {
+class PlanChecker : public DistributedOperatorVisitor {
  public:
   using HierarchicalLogicalOperatorVisitor::PostVisit;
   using HierarchicalLogicalOperatorVisitor::PreVisit;