diff --git a/src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp b/src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp index 0ad74b1dd01..6c9b507763c 100644 --- a/src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp +++ b/src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp @@ -67,26 +67,34 @@ StatusOr 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(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(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(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(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 index 54b06b3d959..71499459358 100644 --- 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 &aliasTypeMap) { + ObjectPool *pool = expr->getObjPool(); + auto matcher = [&aliasTypeMap](const Expression *e) -> bool { + if (e->kind() != Expression::Kind::kFunctionCall) return false; + + auto *funcExpr = static_cast(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(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(e); + auto args = funcExpr->args()->args(); + return LabelAttributeExpression::make( + pool, static_cast(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 index ac10f1570d7..87dc45b0090 100644 --- 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& aliasTypeMap); + // rewrite rank(e) to e._rank + static Expression* rewriteRankFunc2LabelAttribute( + const Expression* expr, const std::unordered_map& 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 index 0abb29431ca..e5936943179 100644 --- 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 index a86efc4e0f7..7c1330dd813 100644 --- 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 index b2b779fd3b3..7df710e9cbc 100644 --- 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; - 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 foundExprs_; }; diff --git a/tests/tck/features/match/Base.feature b/tests/tck/features/match/Base.feature index 6b0c1793c03..0fd7c426979 100644 --- a/tests/tck/features/match/Base.feature +++ b/tests/tck/features/match/Base.feature @@ -1,6 +1,7 @@ # Copyright (c) 2020 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. +@jie Feature: Basic match Background: @@ -964,6 +965,107 @@ Feature: Basic match Then the result should be, in any order, with relax comparison: | id(v) | + Scenario: match with rank + When executing query: + """ + match (v)-[e:like]->() + where id(v) == "Tim Duncan" + and rank(e) == 0 + return * + """ + Then the result should be, in any order: + | v | e | + | ("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"}) | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | + | ("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"}) | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | + When executing query: + """ + match (v)-[e:like]->() + where id(v) == "Tim Duncan" + and rank(e) != 0 + return * + """ + Then the result should be, in any order: + | v | e | + + Scenario: match with rank + When executing query: + """ + match (v)-[e:like]->() + where id(v) == "Tim Duncan" + and rank(e) == 0 + return * + """ + Then the result should be, in any order, with relax comparison: + | v | e | + | ("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"}) | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | + | ("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"}) | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | + When executing query: + """ + match ()-[e]->() + where rank(e) == 0 + return rank(e) + limit 3 + """ + Then the result should be, in any order, with relax comparison: + | rank(e) | + | 0 | + | 0 | + | 0 | + When executing query: + """ + match ()-[e]->() + where rank(e) != 0 + return rank(e) + limit 1000 + """ + Then the result should be, in any order, with relax comparison: + | rank(e) | + | 1 | + | 1 | + | 1 | + | 1 | + | 1 | + | 1 | + When executing query: + """ + match ()-[e]->() + where abs(rank(e)) != 0 and e.start_year > 2010 + return rank(e) + limit 1000 + """ + Then the result should be, in any order, with relax comparison: + | rank(e) | + | 1 | + | 1 | + | 1 | + | 1 | + When executing query: + """ + match ()-[e]->() + where abs(rank(e)) == 1 and e.start_year == 2016 + return e + limit 1000 + """ + Then the result should be, in any order, with relax comparison: + | e | + | [:serve "Marco Belinelli"->"Hornets" @1 {end_year: 2017, start_year: 2016}] | + When executing query: + """ + match ()-[e]->() + where src(e) != "jack" + return rank(e) + limit 10000 + """ + Then an ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down. + When executing query: + """ + match ()-[e]->() + where src(e) != 0 or abs(rank(e)) != 0 + return rank(e) + limit 10000 + """ + Then an ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down. + Scenario: match_with_wrong_syntax When executing query: """