Split optimizer rules (#5470)

Fix compile small rename Fix tck Fix tck fmt Fix tck Fix tck
vesoft-inc · Apr 7, 2023 · 7c01128 · 7c01128
1 parent db4c4ee
commit 7c01128
Show file tree

Hide file tree

Showing 15 changed files with 259 additions and 71 deletions.
diff --git a/src/common/expression/Expression.h b/src/common/expression/Expression.h
@@ -167,6 +167,10 @@ class Expression {
     return false;
   }
 
+  virtual bool isPropertyExpr() const {
+    return false;
+  }
+
   virtual bool isContainerExpr() const {
     return false;
   }

diff --git a/src/common/expression/PropertyExpression.h b/src/common/expression/PropertyExpression.h
@@ -32,6 +32,10 @@ class PropertyExpression : public Expression {
  public:
   bool operator==(const Expression& rhs) const override;
 
+  bool isPropertyExpr() const override {
+    return true;
+  }
+
   const Value& eval(ExpressionContext& ctx) override;
 
   const std::string& ref() const {

diff --git a/src/graph/optimizer/CMakeLists.txt b/src/graph/optimizer/CMakeLists.txt
@@ -59,6 +59,7 @@ nebula_add_library(
     rule/PushLimitDownScanEdgesRule.cpp
     rule/PushFilterDownTraverseRule.cpp
     rule/RemoveAppendVerticesBelowJoinRule.cpp
+    rule/PushFilterThroughAppendVerticesRule.cpp
 )
 
 nebula_add_subdirectory(test)
diff --git a/src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp b/src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp
@@ -32,38 +32,31 @@ PushFilterDownTraverseRule::PushFilterDownTraverseRule() {
 
 const Pattern& PushFilterDownTraverseRule::pattern() const {
   static Pattern pattern =
-      Pattern::create(PlanNode::Kind::kFilter,
-                      {Pattern::create(PlanNode::Kind::kAppendVertices,
-                                       {Pattern::create(PlanNode::Kind::kTraverse)})});
+      Pattern::create(PlanNode::Kind::kFilter, {Pattern::create(PlanNode::Kind::kTraverse)});
   return pattern;
 }
 
 bool PushFilterDownTraverseRule::match(OptContext* ctx, const MatchedResult& matched) const {
   if (!OptRule::match(ctx, matched)) {
     return false;
   }
-  DCHECK_EQ(matched.dependencies[0].dependencies[0].node->node()->kind(),
-            PlanNode::Kind::kTraverse);
-  auto traverse =
-      static_cast<const Traverse*>(matched.dependencies[0].dependencies[0].node->node());
+  DCHECK_EQ(matched.dependencies[0].node->node()->kind(), PlanNode::Kind::kTraverse);
+  auto traverse = static_cast<const Traverse*>(matched.dependencies[0].node->node());
   return traverse->isOneStep();
 }
 
 StatusOr<OptRule::TransformResult> PushFilterDownTraverseRule::transform(
-    OptContext* ctx, const MatchedResult& matched) const {
-  auto* filterGroupNode = matched.node;
-  auto* filterGroup = filterGroupNode->group();
-  auto* filter = static_cast<graph::Filter*>(filterGroupNode->node());
+    OptContext* octx, const MatchedResult& matched) const {
+  auto* filterGNode = matched.node;
+  auto* filterGroup = filterGNode->group();
+  auto* filter = static_cast<graph::Filter*>(filterGNode->node());
   auto* condition = filter->condition();
 
-  auto* avGroupNode = matched.dependencies[0].node;
-  auto* av = static_cast<graph::AppendVertices*>(avGroupNode->node());
+  auto* tvGNode = matched.dependencies[0].node;
+  auto* tvNode = static_cast<graph::Traverse*>(tvGNode->node());
+  auto& edgeAlias = tvNode->edgeAlias();
 
-  auto* tvGroupNode = matched.dependencies[0].dependencies[0].node;
-  auto* tv = static_cast<graph::Traverse*>(tvGroupNode->node());
-  auto& edgeAlias = tv->edgeAlias();
-
-  auto qctx = ctx->qctx();
+  auto qctx = octx->qctx();
   auto pool = qctx->objPool();
 
   // Pick the expr looks like `$-.e[0].likeness
@@ -105,49 +98,39 @@ StatusOr<OptRule::TransformResult> PushFilterDownTraverseRule::transform(
   }
   auto* newFilterPicked =
       graph::ExpressionUtils::rewriteEdgePropertyFilter(pool, edgeAlias, filterPicked->clone());
-
-  Filter* newFilter = nullptr;
-  OptGroupNode* newFilterGroupNode = nullptr;
-  if (filterUnpicked) {
-    newFilter = Filter::make(qctx, nullptr, filterUnpicked);
-    newFilter->setOutputVar(filter->outputVar());
-    newFilter->setColNames(filter->colNames());
-    newFilterGroupNode = OptGroupNode::create(ctx, newFilter, filterGroup);
-  }
-
-  auto* newAv = static_cast<graph::AppendVertices*>(av->clone());
-
-  OptGroupNode* newAvGroupNode = nullptr;
-  if (newFilterGroupNode) {
-    auto* newAvGroup = OptGroup::create(ctx);
-    newAvGroupNode = newAvGroup->makeGroupNode(newAv);
-    newFilterGroupNode->dependsOn(newAvGroup);
-    newFilter->setInputVar(newAv->outputVar());
-  } else {
-    newAvGroupNode = OptGroupNode::create(ctx, newAv, filterGroup);
-    newAv->setOutputVar(filter->outputVar());
-  }
-
-  auto* eFilter = tv->eFilter();
+  auto* eFilter = tvNode->eFilter();
   Expression* newEFilter = eFilter
                                ? LogicalExpression::makeAnd(pool, newFilterPicked, eFilter->clone())
                                : newFilterPicked;
 
-  auto* newTv = static_cast<graph::Traverse*>(tv->clone());
-  newAv->setInputVar(newTv->outputVar());
-  newTv->setEdgeFilter(newEFilter);
-
-  auto* newTvGroup = OptGroup::create(ctx);
-  newAvGroupNode->dependsOn(newTvGroup);
-  auto* newTvGroupNode = newTvGroup->makeGroupNode(newTv);
-
-  for (auto dep : tvGroupNode->dependencies()) {
-    newTvGroupNode->dependsOn(dep);
-  }
+  // produce new Traverse node
+  auto* newTvNode = static_cast<graph::Traverse*>(tvNode->clone());
+  newTvNode->setEdgeFilter(newEFilter);
+  newTvNode->setInputVar(tvNode->inputVar());
+  newTvNode->setColNames(tvNode->outputVarPtr()->colNames);
 
+  // connect the optimized plan
   TransformResult result;
-  result.eraseCurr = true;
-  result.newGroupNodes.emplace_back(newFilterGroupNode ? newFilterGroupNode : newAvGroupNode);
+  result.eraseAll = true;
+  if (filterUnpicked) {
+    auto* newFilterNode = graph::Filter::make(qctx, newTvNode, filterUnpicked);
+    newFilterNode->setOutputVar(filter->outputVar());
+    newFilterNode->setColNames(filter->colNames());
+    auto newFilterGNode = OptGroupNode::create(octx, newFilterNode, filterGroup);
+    // assemble the new Traverse group below Filter
+    auto newTvGroup = OptGroup::create(octx);
+    auto newTvGNode = newTvGroup->makeGroupNode(newTvNode);
+    newTvGNode->setDeps(tvGNode->dependencies());
+    newFilterGNode->setDeps({newTvGroup});
+    newFilterNode->setInputVar(newTvNode->outputVar());
+    result.newGroupNodes.emplace_back(newFilterGNode);
+  } else {
+    // replace the new Traverse node with the old Filter group
+    auto newTvGNode = OptGroupNode::create(octx, newTvNode, filterGroup);
+    newTvNode->setOutputVar(filter->outputVar());
+    newTvGNode->setDeps(tvGNode->dependencies());
+    result.newGroupNodes.emplace_back(newTvGNode);
+  }
 
   return result;
 }

diff --git a/src/graph/optimizer/rule/PushFilterThroughAppendVerticesRule.cpp b/src/graph/optimizer/rule/PushFilterThroughAppendVerticesRule.cpp
@@ -0,0 +1,123 @@
+/* Copyright (c) 2023 vesoft inc. All rights reserved.
+ *
+ * This source code is licensed under Apache 2.0 License.
+ */
+
+#include "graph/optimizer/rule/PushFilterThroughAppendVerticesRule.h"
+
+#include "common/expression/Expression.h"
+#include "graph/optimizer/OptContext.h"
+#include "graph/optimizer/OptGroup.h"
+#include "graph/planner/plan/PlanNode.h"
+#include "graph/planner/plan/Query.h"
+#include "graph/util/ExpressionUtils.h"
+#include "graph/visitor/ExtractFilterExprVisitor.h"
+
+using nebula::Expression;
+using nebula::graph::AppendVertices;
+using nebula::graph::Filter;
+using nebula::graph::PlanNode;
+using nebula::graph::QueryContext;
+
+namespace nebula {
+namespace opt {
+
+std::unique_ptr<OptRule> PushFilterThroughAppendVerticesRule::kInstance =
+    std::unique_ptr<PushFilterThroughAppendVerticesRule>(new PushFilterThroughAppendVerticesRule());
+
+PushFilterThroughAppendVerticesRule::PushFilterThroughAppendVerticesRule() {
+  RuleSet::QueryRules().addRule(this);
+}
+
+const Pattern& PushFilterThroughAppendVerticesRule::pattern() const {
+  static Pattern pattern =
+      Pattern::create(PlanNode::Kind::kFilter, {Pattern::create(PlanNode::Kind::kAppendVertices)});
+  return pattern;
+}
+
+StatusOr<OptRule::TransformResult> PushFilterThroughAppendVerticesRule::transform(
+    OptContext* octx, const MatchedResult& matched) const {
+  auto* oldFilterGroupNode = matched.node;
+  auto* oldFilterGroup = oldFilterGroupNode->group();
+  auto* oldFilterNode = static_cast<graph::Filter*>(oldFilterGroupNode->node());
+  auto* condition = oldFilterNode->condition();
+  auto* oldAvGroupNode = matched.dependencies[0].node;
+  auto* oldAvNode = static_cast<graph::AppendVertices*>(oldAvGroupNode->node());
+  auto& dstNode = oldAvNode->nodeAlias();
+
+  auto inputColNames = oldAvNode->inputVars().front()->colNames;
+  auto qctx = octx->qctx();
+
+  auto picker = [&inputColNames, &dstNode](const Expression* expr) -> bool {
+    auto finder = [&inputColNames, &dstNode](const Expression* e) -> bool {
+      if (e->isPropertyExpr() &&
+          std::find(inputColNames.begin(),
+                    inputColNames.end(),
+                    (static_cast<const PropertyExpression*>(e)->prop())) == inputColNames.end()) {
+        return true;
+      }
+      if (e->kind() == Expression::Kind::kVar &&
+          static_cast<const VariableExpression*>(e)->var() == dstNode) {
+        return true;
+      }
+      return false;
+    };
+    graph::FindVisitor visitor(finder);
+    const_cast<Expression*>(expr)->accept(&visitor);
+    return visitor.results().empty();
+  };
+  Expression* filterPicked = nullptr;
+  Expression* filterUnpicked = nullptr;
+  graph::ExpressionUtils::splitFilter(condition, picker, &filterPicked, &filterUnpicked);
+
+  if (!filterPicked) {
+    return TransformResult::noTransform();
+  }
+
+  // produce new Filter node below
+  auto* newBelowFilterNode =
+      graph::Filter::make(qctx, const_cast<graph::PlanNode*>(oldAvNode->dep()), filterPicked);
+  newBelowFilterNode->setInputVar(oldAvNode->inputVar());
+  newBelowFilterNode->setColNames(oldAvNode->inputVars().front()->colNames);
+  auto newBelowFilterGroup = OptGroup::create(octx);
+  auto newFilterGroupNode = newBelowFilterGroup->makeGroupNode(newBelowFilterNode);
+  newFilterGroupNode->setDeps(oldAvGroupNode->dependencies());
+
+  // produce new AppendVertices node
+  auto* newAvNode = static_cast<graph::AppendVertices*>(oldAvNode->clone());
+  newAvNode->setInputVar(newBelowFilterNode->outputVar());
+
+  TransformResult result;
+  result.eraseAll = true;
+  if (filterUnpicked) {
+    // produce new Filter node above
+    auto* newAboveFilterNode = graph::Filter::make(octx->qctx(), newAvNode, filterUnpicked);
+    newAboveFilterNode->setOutputVar(oldFilterNode->outputVar());
+    auto newAboveFilterGroupNode = OptGroupNode::create(octx, newAboveFilterNode, oldFilterGroup);
+
+    auto newAvGroup = OptGroup::create(octx);
+    auto newAvGroupNode = newAvGroup->makeGroupNode(newAvNode);
+    newAvGroupNode->setDeps({newBelowFilterGroup});
+    newAvNode->setInputVar(newBelowFilterNode->outputVar());
+    newAboveFilterGroupNode->setDeps({newAvGroup});
+    newAboveFilterNode->setInputVar(newAvNode->outputVar());
+    result.newGroupNodes.emplace_back(newAboveFilterGroupNode);
+  } else {
+    newAvNode->setOutputVar(oldFilterNode->outputVar());
+    // newAvNode's col names, on the hand, should inherit those of the oldAvNode
+    // since they are the same project.
+    newAvNode->setColNames(oldAvNode->outputVarPtr()->colNames);
+    auto newAvGroupNode = OptGroupNode::create(octx, newAvNode, oldFilterGroup);
+    newAvGroupNode->setDeps({newBelowFilterGroup});
+    newAvNode->setInputVar(newBelowFilterNode->outputVar());
+    result.newGroupNodes.emplace_back(newAvGroupNode);
+  }
+  return result;
+}
+
+std::string PushFilterThroughAppendVerticesRule::toString() const {
+  return "PushFilterThroughAppendVerticesRule";
+}
+
+}  // namespace opt
+}  // namespace nebula
diff --git a/src/graph/optimizer/rule/PushFilterThroughAppendVerticesRule.h b/src/graph/optimizer/rule/PushFilterThroughAppendVerticesRule.h
@@ -0,0 +1,46 @@
+/* Copyright (c) 2023 vesoft inc. All rights reserved.
+ *
+ * This source code is licensed under Apache 2.0 License.
+ */
+
+#ifndef GRAPH_OPTIMIZER_RULE_PUSHFILTERTHROUGHAPPENDVERTICES_H_
+#define GRAPH_OPTIMIZER_RULE_PUSHFILTERTHROUGHAPPENDVERTICES_H_
+
+#include "graph/optimizer/OptRule.h"
+
+namespace nebula {
+namespace opt {
+
+/*
+ * Before:
+ *    Filter(e.likeness > 78 and v.prop > 40)
+ *        |
+ *   AppendVertices(v)
+ *
+ * After :
+ *   Filter(v.prop > 40)
+ *        |
+ *   AppendVertices(v)
+ *        |
+ *   Filter(e.likeness > 78)
+ *
+ */
+
+class PushFilterThroughAppendVerticesRule final : public OptRule {
+ public:
+  const Pattern &pattern() const override;
+
+  StatusOr<TransformResult> transform(OptContext *ctx, const MatchedResult &matched) const override;
+
+  std::string toString() const override;
+
+ private:
+  PushFilterThroughAppendVerticesRule();
+
+  static std::unique_ptr<OptRule> kInstance;
+};
+
+}  // namespace opt
+}  // namespace nebula
+
+#endif  // GRAPH_OPTIMIZER_RULE_PUSHFILTERTHROUGHAPPENDVERTICES_H_
diff --git a/tests/tck/features/bugfix/LackFilterGetEdges.feature b/tests/tck/features/bugfix/LackFilterGetEdges.feature
@@ -8,6 +8,25 @@ Feature: Test lack filter of get edges transform
 
   # #5145
   Scenario: Lack filter of get edges transform
+    When profiling query:
+      """
+      match ()-[e*1]->()
+      where e[0].likeness > 78  or uuid() > 100
+      return rank(e[0]) AS re limit 3
+      """
+    Then the result should be, in any order:
+      | re |
+      | 0  |
+      | 0  |
+      | 0  |
+    And the execution plan should be:
+      | id | name           | dependencies | operator info                                         |
+      | 24 | Project        | 20           |                                                       |
+      | 20 | Limit          | 21           |                                                       |
+      | 21 | AppendVertices | 23           |                                                       |
+      | 23 | Traverse       | 22           | { "edge filter": "((*.likeness>78) OR (uuid()>100))"} |
+      | 22 | ScanVertices   | 3            |                                                       |
+      | 3  | Start          |              |                                                       |
     When profiling query:
       """
       match ()-[e]->()

diff --git a/tests/tck/features/match/SeekById.feature b/tests/tck/features/match/SeekById.feature
@@ -860,6 +860,7 @@ Feature: Match seek by id
       | 11 | Project        | 8            |               |
       | 8  | Filter         | 4            |               |
       | 4  | AppendVertices | 10           |               |
+      | 10 | Filter         | 10           |               |
       | 10 | Traverse       | 2            |               |
       | 2  | Dedup          | 4            |               |
       | 1  | PassThrough    | 3            |               |

diff --git a/tests/tck/features/optimizer/CollapseProjectRule.feature b/tests/tck/features/optimizer/CollapseProjectRule.feature
@@ -6,6 +6,7 @@ Feature: Collapse Project Rule
   Background:
     Given a graph with space named "nba"
 
+  @czp
   Scenario: Collapse Project
     When profiling query:
       """
@@ -99,6 +100,7 @@ Feature: Collapse Project Rule
       | 14 | Project        | 12           |               |
       | 12 | Filter         | 6            |               |
       | 6  | AppendVertices | 5            |               |
+      | 5  | Filter         | 5            |               |
       | 5  | Traverse       | 4            |               |
       | 4  | Traverse       | 2            |               |
       | 2  | Dedup          | 1            |               |

diff --git a/tests/tck/features/optimizer/PrunePropertiesRule.feature b/tests/tck/features/optimizer/PrunePropertiesRule.feature
@@ -579,6 +579,7 @@ Feature: Prune Properties rule
       | 21 | Project        | 20           |                                                                                                                                          |
       | 20 | Filter         | 25           |                                                                                                                                          |
       | 25 | AppendVertices | 24           | {  "props": "[{ \"props\":[\"name\",\"_tag\"]}]" }                                                                                       |
+      | 24 | Filter         | 24           |                                                                                                                                          |
       | 24 | Traverse       | 23           | {"vertexProps": "[{ \"props\":[\"age\"]}]", "edgeProps": "[{  \"props\": [\"_type\", \"_rank\", \"_dst\"]}]"  }                          |
       | 23 | Traverse       | 22           | {"vertexProps": "", "edgeProps": "[{  \"props\": [\"_type\", \"_rank\", \"_dst\"]}]"  }                                                  |
       | 22 | Traverse       | 2            | {"vertexProps": "", "edgeProps": "[{  \"props\": [\"_type\", \"_rank\", \"_dst\"]}, {  \"props\": [\"_type\", \"_rank\", \"_dst\"]}]"  } |