From 0e955682dbe7c50ad571ba0b7fd9ae07e87578ef Mon Sep 17 00:00:00 2001 From: "jie.wang" <38901892+jievince@users.noreply.github.com> Date: Mon, 30 Jan 2023 15:55:51 +0800 Subject: [PATCH] address comments (#5287) --- src/common/function/FunctionManager.cpp | 40 +++++++ src/graph/util/ExpressionUtils.cpp | 14 ++- src/graph/util/ExpressionUtils.h | 2 +- src/graph/validator/MatchValidator.cpp | 4 +- src/graph/visitor/DeduceTypeVisitor.cpp | 3 +- .../PushFilterDownTraverseRule.feature | 102 ++++++++++++++++++ 6 files changed, 157 insertions(+), 8 deletions(-) diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp index 11d41741cc8..0d3bb3475ac 100644 --- a/src/common/function/FunctionManager.cpp +++ b/src/common/function/FunctionManager.cpp @@ -2056,6 +2056,46 @@ FunctionManager::FunctionManager() { } }; } + { + // `none_direct_src` always return the srcId of an edge key + // without considering the direction of the edge type. + // The encoding of the edge key is: + // type(1) + partId(3) + srcId(*) + edgeType(4) + edgeRank(8) + dstId(*) + placeHolder(1) + // More information of encoding could be found in `NebulaKeyUtils.h` + auto &attr = functions_["none_direct_src"]; + attr.minArity_ = 1; + attr.maxArity_ = 1; + attr.isAlwaysPure_ = true; + attr.body_ = [](const auto &args) -> Value { + switch (args[0].get().type()) { + case Value::Type::NULLVALUE: { + return Value::kNullValue; + } + case Value::Type::EDGE: { + const auto &edge = args[0].get().getEdge(); + return edge.src; + } + case Value::Type::VERTEX: { + const auto &v = args[0].get().getVertex(); + return v.vid; + } + case Value::Type::LIST: { + const auto &listVal = args[0].get().getList(); + auto &firstVal = listVal.values.front(); + if (firstVal.isEdge()) { + return firstVal.getEdge().src; + } else if (firstVal.isVertex()) { + return firstVal.getVertex().vid; + } else { + return Value::kNullBadType; + } + } + default: { + return Value::kNullBadType; + } + } + }; + } { auto &attr = functions_["rank"]; attr.minArity_ = 1; diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index 71499459358..72ac2bda431 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -152,8 +152,8 @@ Expression *ExpressionUtils::rewriteAttr2LabelTagProp( return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter)); } -// rewrite rank(e) to e._rank -Expression *ExpressionUtils::rewriteRankFunc2LabelAttribute( +// rewrite rank(e) to e._rank, none_direct_src(e) to e._src, none_direct_dst(e) to e._dst +Expression *ExpressionUtils::rewriteEdgePropFunc2LabelAttribute( const Expression *expr, const std::unordered_map &aliasTypeMap) { ObjectPool *pool = expr->getObjPool(); auto matcher = [&aliasTypeMap](const Expression *e) -> bool { @@ -162,7 +162,9 @@ Expression *ExpressionUtils::rewriteRankFunc2LabelAttribute( auto *funcExpr = static_cast(e); auto funcName = funcExpr->name(); std::transform(funcName.begin(), funcName.end(), funcName.begin(), ::tolower); - if (funcName != "rank") return false; + if (funcName != "rank" && funcName != "none_direct_src" && funcName != "none_direct_dst") { + return false; + } auto args = funcExpr->args()->args(); if (args.size() != 1) return false; if (args[0]->kind() != Expression::Kind::kLabel) return false; @@ -174,11 +176,15 @@ Expression *ExpressionUtils::rewriteRankFunc2LabelAttribute( } return true; }; + static const std::unordered_map func2Attr = { + {"rank", "_rank"}, {"none_direct_src", "_src"}, {"none_direct_dst", "_dst"}}; auto rewriter = [pool](const Expression *e) -> Expression * { auto funcExpr = static_cast(e); + auto &funcName = funcExpr->name(); + auto &attrName = func2Attr.at(funcName); auto args = funcExpr->args()->args(); return LabelAttributeExpression::make( - pool, static_cast(args[0]), ConstantExpression::make(pool, "_rank")); + pool, static_cast(args[0]), ConstantExpression::make(pool, attrName)); }; return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter)); diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h index 87dc45b0090..896f44a80cf 100644 --- a/src/graph/util/ExpressionUtils.h +++ b/src/graph/util/ExpressionUtils.h @@ -65,7 +65,7 @@ class ExpressionUtils { const Expression* expr, const std::unordered_map& aliasTypeMap); // rewrite rank(e) to e._rank - static Expression* rewriteRankFunc2LabelAttribute( + static Expression* rewriteEdgePropFunc2LabelAttribute( const Expression* expr, const std::unordered_map& aliasTypeMap); // rewrite LabelAttr to tagProp diff --git a/src/graph/validator/MatchValidator.cpp b/src/graph/validator/MatchValidator.cpp index 492a36d76b5..3aae47fb019 100644 --- a/src/graph/validator/MatchValidator.cpp +++ b/src/graph/validator/MatchValidator.cpp @@ -340,8 +340,8 @@ Status MatchValidator::validateFilter(const Expression *filter, // rewrite Attribute to LabelTagProperty newFilter = ExpressionUtils::rewriteAttr2LabelTagProp(transformRes.value(), whereClauseCtx.aliasesAvailable); - newFilter = - ExpressionUtils::rewriteRankFunc2LabelAttribute(newFilter, whereClauseCtx.aliasesAvailable); + newFilter = ExpressionUtils::rewriteEdgePropFunc2LabelAttribute(newFilter, + whereClauseCtx.aliasesAvailable); whereClauseCtx.filter = newFilter; diff --git a/src/graph/visitor/DeduceTypeVisitor.cpp b/src/graph/visitor/DeduceTypeVisitor.cpp index f6c3c5dba86..66fbcc11c4e 100644 --- a/src/graph/visitor/DeduceTypeVisitor.cpp +++ b/src/graph/visitor/DeduceTypeVisitor.cpp @@ -475,7 +475,8 @@ void DeduceTypeVisitor::visit(FunctionCallExpression *expr) { } auto funName = expr->name(); - if (funName == "id" || funName == "src" || funName == "dst") { + if (funName == "id" || funName == "src" || funName == "dst" || funName == "none_direct_src" || + funName == "none_direct_dst") { type_ = vidType_; return; } diff --git a/tests/tck/features/optimizer/PushFilterDownTraverseRule.feature b/tests/tck/features/optimizer/PushFilterDownTraverseRule.feature index e0a1f50eff5..e30d7091b3a 100644 --- a/tests/tck/features/optimizer/PushFilterDownTraverseRule.feature +++ b/tests/tck/features/optimizer/PushFilterDownTraverseRule.feature @@ -25,6 +25,86 @@ Feature: Push Filter down Traverse rule | 2 | Dedup | 9 | | | 9 | IndexScan | 3 | | | 3 | Start | | | + When profiling query: + """ + MATCH (v:player)-[e:like]->(v2) WHERE rank(e) == 0 RETURN COUNT(*) + """ + Then the result should be, in any order: + | COUNT(*) | + | 81 | + And the execution plan should be: + | id | name | dependencies | operator info | + | 7 | Aggregate | 6 | | + | 6 | Project | 5 | | + | 5 | AppendVertices | 10 | | + | 10 | Traverse | 2 | {"filter": "(like._rank==0)"} | + | 2 | Dedup | 9 | | + | 9 | IndexScan | 3 | | + | 3 | Start | | | + When profiling query: + """ + MATCH (v:player)-[e:like]->(v2) WHERE none_direct_dst(e) IN ["Tony Parker", "Tim Duncan"] RETURN e.likeness, v2.player.age + """ + Then the result should be, in any order: + | e.likeness | v2.player.age | + | 80 | 36 | + | 99 | 36 | + | 75 | 36 | + | 50 | 36 | + | 95 | 36 | + | 80 | 42 | + | 80 | 42 | + | 70 | 42 | + | 99 | 42 | + | 75 | 42 | + | 90 | 42 | + | 55 | 42 | + | 80 | 42 | + | 80 | 42 | + | 95 | 42 | + And the execution plan should be: + | id | name | dependencies | operator info | + | 6 | Project | 5 | | + | 5 | AppendVertices | 10 | | + | 10 | Traverse | 9 | {"filter": "((like._dst==\"Tony Parker\") OR (like._dst==\"Tim Duncan\"))"} | + | 9 | IndexScan | 3 | | + | 3 | Start | | | + # The following two match statements is equivalent, so the minus of them should be empty. + When executing query: + """ + MATCH (v:player)-[e:like]->(v2) WHERE none_direct_dst(e) IN ["Tony Parker", "Tim Duncan"] RETURN * + MINUS + MATCH (v:player)-[e:like]->(v2) WHERE id(v2) IN ["Tony Parker", "Tim Duncan"] RETURN * + """ + Then the result should be, in any order: + | v | e | v2 | + When profiling query: + """ + MATCH (v:player)-[e:like]->(v2) WHERE none_direct_src(e) IN ["Tony Parker", "Tim Duncan"] RETURN e.likeness, v2.player.age + """ + Then the result should be, in any order: + | e.likeness | v2.player.age | + | 90 | 33 | + | 95 | 41 | + | 95 | 42 | + | 95 | 41 | + | 95 | 36 | + And the execution plan should be: + | id | name | dependencies | operator info | + | 6 | Project | 5 | | + | 5 | AppendVertices | 10 | | + | 10 | Traverse | 9 | {"filter": "((like._src==\"Tony Parker\") OR (like._src==\"Tim Duncan\"))"} | + | 9 | IndexScan | 3 | | + | 3 | Start | | | + # The following two match statements is equivalent, so the minus of them should be empty. + When executing query: + """ + MATCH (v:player)-[e:like]->(v2) WHERE none_direct_src(e) IN ["Tony Parker", "Tim Duncan"] RETURN * + MINUS + MATCH (v:player)-[e:like]->(v2) WHERE id(v) IN ["Tony Parker", "Tim Duncan"] RETURN * + """ + Then the result should be, in any order: + | v | e | v2 | When profiling query: """ MATCH (person:player)-[:like*1..2]-(friend:player)-[served:serve]->(friendTeam:team) @@ -94,3 +174,25 @@ Feature: Push Filter down Traverse rule | 12 | Traverse | 8 | {"filter": "((like.likeness+100)!=199)"} | | 8 | IndexScan | 2 | | | 2 | Start | | | + When profiling query: + """ + MATCH (v:player)-[e:like]->(v2) + WHERE v.player.age != 35 and (e.likeness + 100) != 199 and none_direct_dst(e) in ["Tony Parker", "Tim Duncan", "Yao Ming"] + RETURN e.likeness, v2.player.age as age + ORDER BY age + LIMIT 3 + """ + Then the result should be, in any order: + | e.likeness | age | + | 80 | 36 | + | 75 | 36 | + | 50 | 36 | + 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+100)!=199) AND ((like._dst==\"Tony Parker\") OR (like._dst==\"Tim Duncan\") OR (like._dst==\"Yao Ming\")))"} | + | 8 | IndexScan | 2 | | + | 2 | Start | | |