fix push down rank(e) (#5135)

vesoft-inc · Dec 29, 2022 · 373a847 · 373a847
1 parent 967a8c9
commit 373a847
Show file tree

Hide file tree

Showing 7 changed files with 200 additions and 43 deletions.
diff --git a/src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp b/src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp
@@ -67,26 +67,34 @@ StatusOr<OptRule::TransformResult> PushFilterDownTraverseRule::transform(
   auto pool = qctx->objPool();
 
   // Pick the expr looks like `$-.e[0].likeness
-  auto picker = [&edgeAlias](const Expression* e) -> bool {
-    // TODO(jie): Handle the strange exists expr. e.g. exists(e.likeness)
-    auto exprs = graph::ExpressionUtils::collectAll(e, {Expression::Kind::kPredicate});
-    for (auto* expr : exprs) {
-      if (static_cast<const PredicateExpression*>(expr)->name() == "exists") {
+  auto picker = [&edgeAlias](const Expression* expr) -> bool {
+    bool shouldNotPick = false;
+    auto finder = [&shouldNotPick, &edgeAlias](const Expression* e) -> bool {
+      // When visiting the expression tree and find an expession node is a one step edge property
+      // expression, stop visiting its children and return true.
+      if (graph::ExpressionUtils::isOneStepEdgeProp(edgeAlias, e)) return true;
+      // Otherwise, continue visiting its children. And if the following two conditions are met,
+      // mark the expression as shouldNotPick and return false.
+      if (e->kind() == Expression::Kind::kInputProperty ||
+          e->kind() == Expression::Kind::kVarProperty) {
+        shouldNotPick = true;
+        return false;
+      }
+      // TODO(jie): Handle the strange exists expr. e.g. exists(e.likeness)
+      if (e->kind() == Expression::Kind::kPredicate &&
+          static_cast<const PredicateExpression*>(e)->name() == "exists") {
+        shouldNotPick = true;
         return false;
       }
-    }
-
-    auto varProps = graph::ExpressionUtils::collectAll(
-        e, {Expression::Kind::kInputProperty, Expression::Kind::kVarProperty});
-    if (varProps.empty()) {
       return false;
+    };
+    graph::FindVisitor visitor(finder, true, true);
+    const_cast<Expression*>(expr)->accept(&visitor);
+    if (shouldNotPick) return false;
+    if (!visitor.results().empty()) {
+      return true;
     }
-    for (auto* expr : varProps) {
-      DCHECK(graph::ExpressionUtils::isPropertyExpr(expr));
-      auto& propName = static_cast<const PropertyExpression*>(expr)->prop();
-      if (propName != edgeAlias) return false;
-    }
-    return true;
+    return false;
   };
   Expression* filterPicked = nullptr;
   Expression* filterUnpicked = nullptr;

diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp
@@ -152,6 +152,38 @@ Expression *ExpressionUtils::rewriteAttr2LabelTagProp(
   return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter));
 }
 
+// rewrite rank(e) to e._rank
+Expression *ExpressionUtils::rewriteRankFunc2LabelAttribute(
+    const Expression *expr, const std::unordered_map<std::string, AliasType> &aliasTypeMap) {
+  ObjectPool *pool = expr->getObjPool();
+  auto matcher = [&aliasTypeMap](const Expression *e) -> bool {
+    if (e->kind() != Expression::Kind::kFunctionCall) return false;
+
+    auto *funcExpr = static_cast<const FunctionCallExpression *>(e);
+    auto funcName = funcExpr->name();
+    std::transform(funcName.begin(), funcName.end(), funcName.begin(), ::tolower);
+    if (funcName != "rank") return false;
+    auto args = funcExpr->args()->args();
+    if (args.size() != 1) return false;
+    if (args[0]->kind() != Expression::Kind::kLabel) return false;
+
+    auto &label = static_cast<const LabelExpression *>(args[0])->name();
+    auto iter = aliasTypeMap.find(label);
+    if (iter == aliasTypeMap.end() || iter->second != AliasType::kEdge) {
+      return false;
+    }
+    return true;
+  };
+  auto rewriter = [pool](const Expression *e) -> Expression * {
+    auto funcExpr = static_cast<const FunctionCallExpression *>(e);
+    auto args = funcExpr->args()->args();
+    return LabelAttributeExpression::make(
+        pool, static_cast<LabelExpression *>(args[0]), ConstantExpression::make(pool, "_rank"));
+  };
+
+  return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter));
+}
+
 Expression *ExpressionUtils::rewriteLabelAttr2TagProp(const Expression *expr) {
   ObjectPool *pool = expr->getObjPool();
   auto matcher = [](const Expression *e) -> bool {
@@ -1518,7 +1550,7 @@ bool ExpressionUtils::checkExprDepth(const Expression *expr) {
 }
 
 /*static*/
-bool ExpressionUtils::isSingleLenExpandExpr(const std::string &edgeAlias, const Expression *expr) {
+bool ExpressionUtils::isOneStepEdgeProp(const std::string &edgeAlias, const Expression *expr) {
   if (expr->kind() != Expression::Kind::kAttribute) {
     return false;
   }
@@ -1562,7 +1594,7 @@ bool ExpressionUtils::isSingleLenExpandExpr(const std::string &edgeAlias, const
                                                                   const std::string &edgeAlias,
                                                                   Expression *expr) {
   graph::RewriteVisitor::Matcher matcher = [&edgeAlias](const Expression *e) -> bool {
-    return isSingleLenExpandExpr(edgeAlias, e);
+    return isOneStepEdgeProp(edgeAlias, e);
   };
   graph::RewriteVisitor::Rewriter rewriter = [pool](const Expression *e) -> Expression * {
     DCHECK_EQ(e->kind(), Expression::Kind::kAttribute);

diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h
@@ -64,6 +64,10 @@ class ExpressionUtils {
   static Expression* rewriteAttr2LabelTagProp(
       const Expression* expr, const std::unordered_map<std::string, AliasType>& aliasTypeMap);
 
+  // rewrite rank(e) to e._rank
+  static Expression* rewriteRankFunc2LabelAttribute(
+      const Expression* expr, const std::unordered_map<std::string, AliasType>& aliasTypeMap);
+
   // rewrite LabelAttr to tagProp
   static Expression* rewriteLabelAttr2TagProp(const Expression* expr);
 
@@ -239,7 +243,7 @@ class ExpressionUtils {
   static bool isVidPredication(const Expression* expr);
 
   // Check if the expr looks like `$-.e[0].likeness`
-  static bool isSingleLenExpandExpr(const std::string& edgeAlias, const Expression* expr);
+  static bool isOneStepEdgeProp(const std::string& edgeAlias, const Expression* expr);
 
   static Expression* rewriteEdgePropertyFilter(ObjectPool* pool,
                                                const std::string& edgeAlias,

diff --git a/src/graph/validator/MatchValidator.cpp b/src/graph/validator/MatchValidator.cpp
@@ -338,8 +338,12 @@ Status MatchValidator::validateFilter(const Expression *filter,
   auto transformRes = ExpressionUtils::filterTransform(newFilter);
   NG_RETURN_IF_ERROR(transformRes);
   // rewrite Attribute to LabelTagProperty
-  whereClauseCtx.filter = ExpressionUtils::rewriteAttr2LabelTagProp(
-      transformRes.value(), whereClauseCtx.aliasesAvailable);
+  newFilter = ExpressionUtils::rewriteAttr2LabelTagProp(transformRes.value(),
+                                                        whereClauseCtx.aliasesAvailable);
+  newFilter =
+      ExpressionUtils::rewriteRankFunc2LabelAttribute(newFilter, whereClauseCtx.aliasesAvailable);
+
+  whereClauseCtx.filter = newFilter;
 
   auto typeStatus = deduceExprType(whereClauseCtx.filter);
   NG_RETURN_IF_ERROR(typeStatus);

diff --git a/src/graph/visitor/FindVisitor.cpp b/src/graph/visitor/FindVisitor.cpp
@@ -7,19 +7,19 @@ namespace nebula {
 namespace graph {
 
 void FindVisitor::visit(TypeCastingExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   expr->operand()->accept(this);
 }
 
 void FindVisitor::visit(UnaryExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   expr->operand()->accept(this);
 }
 
 void FindVisitor::visit(FunctionCallExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   for (const auto& arg : expr->args()->args()) {
     arg->accept(this);
@@ -28,13 +28,13 @@ void FindVisitor::visit(FunctionCallExpression* expr) {
 }
 
 void FindVisitor::visit(AggregateExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   expr->arg()->accept(this);
 }
 
 void FindVisitor::visit(ListExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   for (const auto& item : expr->items()) {
     item->accept(this);
@@ -43,7 +43,7 @@ void FindVisitor::visit(ListExpression* expr) {
 }
 
 void FindVisitor::visit(SetExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   for (const auto& item : expr->items()) {
     item->accept(this);
@@ -52,7 +52,7 @@ void FindVisitor::visit(SetExpression* expr) {
 }
 
 void FindVisitor::visit(MapExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   for (const auto& pair : expr->items()) {
     pair.second->accept(this);
@@ -61,7 +61,7 @@ void FindVisitor::visit(MapExpression* expr) {
 }
 
 void FindVisitor::visit(CaseExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
 
   if (expr->hasCondition()) {
@@ -81,7 +81,7 @@ void FindVisitor::visit(CaseExpression* expr) {
 }
 
 void FindVisitor::visit(PredicateExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
 
   expr->collection()->accept(this);
@@ -92,7 +92,7 @@ void FindVisitor::visit(PredicateExpression* expr) {
 }
 
 void FindVisitor::visit(ReduceExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
 
   expr->initial()->accept(this);
@@ -104,7 +104,7 @@ void FindVisitor::visit(ReduceExpression* expr) {
 }
 
 void FindVisitor::visit(ListComprehensionExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
 
   expr->collection()->accept(this);
@@ -121,7 +121,7 @@ void FindVisitor::visit(ListComprehensionExpression* expr) {
 }
 
 void FindVisitor::visit(LogicalExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   for (const auto& operand : expr->operands()) {
     operand->accept(this);
@@ -190,7 +190,7 @@ void FindVisitor::visit(LabelExpression* expr) {
 }
 
 void FindVisitor::visit(LabelAttributeExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   expr->left()->accept(this);
   if (!needFindAll_ && !foundExprs_.empty()) return;
@@ -202,7 +202,7 @@ void FindVisitor::visit(VertexExpression* expr) {
 }
 
 void FindVisitor::visit(LabelTagPropertyExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   expr->label()->accept(this);
 }
@@ -216,7 +216,7 @@ void FindVisitor::visit(ColumnExpression* expr) {
 }
 
 void FindVisitor::visit(PathBuildExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   for (const auto& item : expr->items()) {
     item->accept(this);
@@ -225,7 +225,7 @@ void FindVisitor::visit(PathBuildExpression* expr) {
 }
 
 void FindVisitor::visit(SubscriptRangeExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
 
   expr->list()->accept(this);
@@ -243,7 +243,7 @@ void FindVisitor::visit(SubscriptRangeExpression* expr) {
 }
 
 void FindVisitor::visit(MatchPathPatternExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   if (expr->genList() != nullptr) {
     expr->genList()->accept(this);
@@ -252,17 +252,19 @@ void FindVisitor::visit(MatchPathPatternExpression* expr) {
 }
 
 void FindVisitor::visitBinaryExpr(BinaryExpression* expr) {
-  findInCurrentExpr(expr);
+  if (findInCurrentExpr(expr) && stopVisitChildrenAfterFind_) return;
   if (!needFindAll_ && !foundExprs_.empty()) return;
   expr->left()->accept(this);
   if (!needFindAll_ && !foundExprs_.empty()) return;
   expr->right()->accept(this);
 }
 
-void FindVisitor::findInCurrentExpr(Expression* expr) {
+bool FindVisitor::findInCurrentExpr(Expression* expr) {
   if (finder_(expr)) {
     foundExprs_.emplace_back(expr);
+    return true;
   }
+  return false;
 }
 
 }  // namespace graph

diff --git a/src/graph/visitor/FindVisitor.h b/src/graph/visitor/FindVisitor.h
@@ -15,8 +15,12 @@ class FindVisitor final : public ExprVisitorImpl {
  public:
   using Finder = std::function<bool(Expression*)>;
 
-  explicit FindVisitor(Finder finder, bool needFindAll = false)
-      : finder_(finder), needFindAll_(needFindAll) {}
+  explicit FindVisitor(Finder finder,
+                       bool needFindAll = false,
+                       bool stopVisitChildrenAfterFind = false)
+      : finder_(finder),
+        needFindAll_(needFindAll),
+        stopVisitChildrenAfterFind_(stopVisitChildrenAfterFind) {}
 
   bool ok() const override {
     // TODO: delete this interface
@@ -80,11 +84,12 @@ class FindVisitor final : public ExprVisitorImpl {
   void visit(MatchPathPatternExpression* expr) override;
 
   void visitBinaryExpr(BinaryExpression* expr) override;
-  void findInCurrentExpr(Expression* expr);
+  bool findInCurrentExpr(Expression* expr);
 
  private:
   Finder finder_;
   bool needFindAll_;
+  bool stopVisitChildrenAfterFind_{false};
   std::vector<const Expression*> foundExprs_;
 };