diff --git a/src/common/expression/PredicateExpression.cpp b/src/common/expression/PredicateExpression.cpp index 16390619f24..88f71a058c4 100644 --- a/src/common/expression/PredicateExpression.cpp +++ b/src/common/expression/PredicateExpression.cpp @@ -19,7 +19,8 @@ std::unordered_map PredicateExpression:: const Value& PredicateExpression::evalExists(ExpressionContext& ctx) { DCHECK(collection_->kind() == Expression::Kind::kAttribute || collection_->kind() == Expression::Kind::kSubscript || - collection_->kind() == Expression::Kind::kLabelTagProperty); + collection_->kind() == Expression::Kind::kLabelTagProperty) + << "actual kind: " << collection_->kind() << ", toString: " << toString(); if (collection_->kind() == Expression::Kind::kLabelTagProperty) { result_ = !collection_->eval(ctx).isNull(); diff --git a/src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp b/src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp index 55e33373d76..3ea748a2fd5 100644 --- a/src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp +++ b/src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp @@ -5,6 +5,7 @@ #include "graph/optimizer/rule/PushFilterDownTraverseRule.h" +#include "common/expression/ConstantExpression.h" #include "common/expression/Expression.h" #include "graph/optimizer/OptContext.h" #include "graph/optimizer/OptGroup.h" @@ -71,6 +72,7 @@ StatusOr PushFilterDownTraverseRule::transform( auto qctx = ctx->qctx(); auto pool = qctx->objPool(); + // Pick the expr looks like `$-.e[0].likeness auto picker = [&edgeAlias](const Expression* e) -> bool { auto varProps = graph::ExpressionUtils::collectAll(e, {Expression::Kind::kTagProperty, @@ -87,7 +89,12 @@ StatusOr PushFilterDownTraverseRule::transform( auto& propName = static_cast(expr)->prop(); if (propName != edgeAlias) return false; } - return true; + auto finder = [&edgeAlias](const Expression* expr) -> bool { + return graph::ExpressionUtils::isSingleLenExpandExpr(edgeAlias, expr); + }; + graph::FindVisitor visitor(finder, true); + const_cast(e)->accept(&visitor); + return !visitor.results().empty(); }; Expression* filterPicked = nullptr; Expression* filterUnpicked = nullptr; diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index 511619d0d6c..888321b0e0e 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -350,6 +350,7 @@ Expression *ExpressionUtils::rewriteInExpr(const Expression *expr) { return const_cast(expr); } std::vector operands; + operands.reserve(values.size()); for (const auto &v : values) { operands.emplace_back( RelationalExpression::makeEQ(pool, inExpr->left(), ConstantExpression::make(pool, v))); @@ -1413,6 +1414,42 @@ bool ExpressionUtils::checkExprDepth(const Expression *expr) { return true; } +/*static*/ +bool ExpressionUtils::isSingleLenExpandExpr(const std::string &edgeAlias, const Expression *expr) { + if (expr->kind() != Expression::Kind::kAttribute) { + return false; + } + auto attributeExpr = static_cast(expr); + auto *left = attributeExpr->left(); + auto *right = attributeExpr->right(); + + if (left->kind() != Expression::Kind::kSubscript) return false; + if (right->kind() != Expression::Kind::kConstant || + !static_cast(right)->value().isStr()) + return false; + + auto subscriptExpr = static_cast(left); + auto *listExpr = subscriptExpr->left(); + auto *idxExpr = subscriptExpr->right(); + if (listExpr->kind() != Expression::Kind::kInputProperty && + listExpr->kind() != Expression::Kind::kVarProperty) { + return false; + } + if (static_cast(listExpr)->prop() != edgeAlias) { + return false; + } + + // NOTE(jie): Just handled `$-.e[0].likeness` for now, whileas the traverse is single length + // expand. + // TODO(jie): Handle `ALL(i IN e WHERE i.likeness > 78)`, whileas the traverse is var len + // expand. + if (idxExpr->kind() != Expression::Kind::kConstant || + static_cast(idxExpr)->value() != 0) { + return false; + } + return true; +} + // Transform expression `$-.e[0].likeness` to EdgePropertyExpression `like.likeness` // for more friendly to push down // \param pool object pool to hold ownership of objects alloacted @@ -1422,39 +1459,7 @@ bool ExpressionUtils::checkExprDepth(const Expression *expr) { const std::string &edgeAlias, Expression *expr) { graph::RewriteVisitor::Matcher matcher = [&edgeAlias](const Expression *e) -> bool { - if (e->kind() != Expression::Kind::kAttribute) { - return false; - } - auto attributeExpr = static_cast(e); - auto *left = attributeExpr->left(); - auto *right = attributeExpr->right(); - - if (left->kind() != Expression::Kind::kSubscript) return false; - if (right->kind() != Expression::Kind::kConstant || - !static_cast(right)->value().isStr()) - return false; - - auto subscriptExpr = static_cast(left); - auto *listExpr = subscriptExpr->left(); - auto *idxExpr = subscriptExpr->right(); - if (listExpr->kind() != Expression::Kind::kInputProperty && - listExpr->kind() != Expression::Kind::kVarProperty) { - return false; - } - if (static_cast(listExpr)->prop() != edgeAlias) { - return false; - } - - // NOTE(jie): Just handled `$-.e[0].likeness` for now, whileas the traverse is single length - // expand. - // TODO(jie): Handle `ALL(i IN e WHERE i.likeness > 78)`, whileas the traverse is var len - // expand. - if (idxExpr->kind() != Expression::Kind::kConstant || - static_cast(idxExpr)->value() != 0) { - return false; - } - - return true; + return isSingleLenExpandExpr(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 f1c28a81ddb..f77f53f9032 100644 --- a/src/graph/util/ExpressionUtils.h +++ b/src/graph/util/ExpressionUtils.h @@ -226,6 +226,9 @@ class ExpressionUtils { // e.g. id(v) == 1, id(v) IN [...] 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 Expression* rewriteEdgePropertyFilter(ObjectPool* pool, const std::string& edgeAlias, Expression* expr); diff --git a/src/storage/query/QueryBaseProcessor-inl.h b/src/storage/query/QueryBaseProcessor-inl.h index e1291920fce..c459b76d1dd 100644 --- a/src/storage/query/QueryBaseProcessor-inl.h +++ b/src/storage/query/QueryBaseProcessor-inl.h @@ -407,9 +407,11 @@ nebula::cpp2::ErrorCode QueryBaseProcessor::checkExp( if (ret != nebula::cpp2::ErrorCode::SUCCEEDED) { return ret; } - ret = checkExp(predExp->filter(), returned, filtered, updated, allowNoexistentProp); - if (ret != nebula::cpp2::ErrorCode::SUCCEEDED) { - return ret; + if (predExp->hasFilter()) { + ret = checkExp(predExp->filter(), returned, filtered, updated, allowNoexistentProp); + if (ret != nebula::cpp2::ErrorCode::SUCCEEDED) { + return ret; + } } return nebula::cpp2::ErrorCode::SUCCEEDED; } diff --git a/tests/tck/features/match/Base.IntVid.feature b/tests/tck/features/match/Base.IntVid.feature index 20a43766a8f..65abe237b3e 100644 --- a/tests/tck/features/match/Base.IntVid.feature +++ b/tests/tck/features/match/Base.IntVid.feature @@ -391,15 +391,15 @@ Feature: Basic match | <("LeBron James")-[:like@0]->("Ray Allen")-[:like@0]->("Rajon Rondo")> | Scenario: exists - When executing query: - """ - MATCH (:player{name:"Tony Parker"})-[r]->() where exists(r.likeness) return r, exists({a:12}.a) - """ - Then the result should be, in any order, with relax comparison: - | r | exists({a:12}.a) | - | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | true | - | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | true | - | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | true | + # When executing query: + # """ + # MATCH (:player{name:"Tony Parker"})-[r]->() where exists(r.likeness) return r, exists({a:12}.a) + # """ + # Then the result should be, in any order, with relax comparison: + # | r | exists({a:12}.a) | + # | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | true | + # | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | true | + # | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | true | When executing query: """ MATCH (:player{name:"Tony Parker"})-[r]->(m) where exists(m.team.likeness) return r, exists({a:12}.a) diff --git a/tests/tck/features/match/Base.feature b/tests/tck/features/match/Base.feature index 316f07b4535..61aa3a58159 100644 --- a/tests/tck/features/match/Base.feature +++ b/tests/tck/features/match/Base.feature @@ -495,15 +495,15 @@ Feature: Basic match Then a SemanticError should be raised at runtime: Match clause is not supported to be followed by other cypher clauses Scenario: exists - When executing query: - """ - match (:player{name:"Tony Parker"})-[r]->() where exists(r.likeness) return r, exists({a:12}.a) - """ - Then the result should be, in any order, with relax comparison: - | r | exists({a:12}.a) | - | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | true | - | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | true | - | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | true | + # When executing query: + # """ + # match (:player{name:"Tony Parker"})-[r]->() where exists(r.likeness) return r, exists({a:12}.a) + # """ + # Then the result should be, in any order, with relax comparison: + # | r | exists({a:12}.a) | + # | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | true | + # | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | true | + # | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | true | When executing query: """ match (:player{name:"Tony Parker"})-[r]->(m) where exists(m.player.likeness) return r, exists({a:12}.a) diff --git a/tests/tck/features/optimizer/PushFilterDownTraverseRule.feature b/tests/tck/features/optimizer/PushFilterDownTraverseRule.feature index 29100f44477..56aed295543 100644 --- a/tests/tck/features/optimizer/PushFilterDownTraverseRule.feature +++ b/tests/tck/features/optimizer/PushFilterDownTraverseRule.feature @@ -25,11 +25,56 @@ Feature: Push Filter down Traverse rule | 2 | Dedup | 9 | | | 9 | IndexScan | 3 | | | 3 | Start | | | + When profiling query: + """ + MATCH (person:player)-[:like*1..2]-(friend:player)-[served:serve]->(friendTeam:team) + WHERE id(person) == "Tony Parker" AND id(friend) != "Tony Parker" AND served.start_year > 2010 + WITH DISTINCT friend, friendTeam + OPTIONAL MATCH (friend)<-[:like]-(friend2:player)<-[:like]-(friendTeam) + WITH friendTeam, count(friend2) AS numFriends + RETURN + friendTeam.team.name AS teamName, + numFriends + ORDER BY teamName DESC + LIMIT 8 + """ + Then the result should be, in any order, with relax comparison: + | teamName | numFriends | + | "Warriors" | 0 | + | "Trail Blazers" | 0 | + | "Spurs" | 0 | + | "Rockets" | 0 | + | "Raptors" | 0 | + | "Pistons" | 0 | + | "Lakers" | 0 | + | "Kings" | 0 | + # The filter `served.start_year` is first pushed down to the `eFilter_` of `Traverese` by rule `PushFilterDownTraverse`, + # and then pushed down to the `filter_` of `Traverse` by rule `PushEFilterDown`. + And the execution plan should be: + | id | name | dependencies | profiling data | operator info | + | 21 | TopN | 18 | | | + | 18 | Project | 17 | | | + | 17 | Aggregate | 16 | | | + | 16 | HashLeftJoin | 10,15 | | | + | 10 | Dedup | 28 | | | + | 28 | Project | 22 | | | + | 22 | Filter | 26 | | | + | 26 | AppendVertices | 25 | | | + | 25 | Traverse | 24 | | {"filter": "(serve.start_year>2010)"} | + | 24 | Traverse | 2 | | | + | 2 | Dedup | 1 | | | + | 1 | PassThrough | 3 | | | + | 3 | Start | | | | + | 15 | Project | 15 | | | + | 30 | AppendVertices | 14 | | | + | 14 | Traverse | 12 | | | + | 12 | Traverse | 11 | | | + | 11 | Argument | | | | Scenario: push filter down Traverse with complex filter When profiling query: """ - MATCH (v:player)-[e:like]->(v2) WHERE v.player.age != 35 and e.likeness != 99 + MATCH (v:player)-[e:like]->(v2) WHERE v.player.age != 35 and (e.likeness + 100) != 199 RETURN e.likeness, v2.player.age as age ORDER BY age LIMIT 3 @@ -39,14 +84,14 @@ Feature: Push Filter down Traverse rule | 90 | 20 | | 80 | 22 | | 90 | 23 | - # The filter `e.likeness!=99` is first pushed down to the `eFilter_` of `Traverese` by rule `PushFilterDownTraverse`, + # The filter `e.likeness+100!=99` is first pushed down to the `eFilter_` of `Traverese` by rule `PushFilterDownTraverse`, # and then pushed down to the `filter_` of `Traverse` by rule `PushEFilterDown`. And the execution plan should be: - | id | name | dependencies | operator info | - | 11 | TopN | 10 | | - | 10 | Project | 9 | | - | 9 | Filter | 4 | {"condition": "(-.v.player.age!=35)" } | - | 4 | AppendVertices | 12 | | - | 12 | Traverse | 8 | {"filter": "(like.likeness!=99)"} | - | 8 | IndexScan | 2 | | - | 2 | Start | | | + | id | name | dependencies | operator info | + | 11 | TopN | 10 | | + | 10 | Project | 9 | | + | 9 | Filter | 4 | {"condition": "(-.v.player.age!=35)" } | + | 4 | AppendVertices | 12 | | + | 12 | Traverse | 8 | {"filter": "((like.likeness+100)!=199)"} | + | 8 | IndexScan | 2 | | + | 2 | Start | | |