From 82bfcae6ca4c60507c221ce2dff3ce052c327164 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Tue, 31 Jan 2023 18:36:17 +0800 Subject: [PATCH] add more tck --- src/graph/planner/match/LabelIndexSeek.cpp | 2 +- .../planner/match/MatchClausePlanner.cpp | 6 --- src/graph/planner/match/PropIndexSeek.cpp | 5 +- .../optimizer/CasesUsingTestSpace.feature | 3 +- .../PushFilterDownTraverseRule.feature | 54 ++++++++++++++----- tests/tck/features/yield/parameter.feature | 4 +- 6 files changed, 49 insertions(+), 25 deletions(-) diff --git a/src/graph/planner/match/LabelIndexSeek.cpp b/src/graph/planner/match/LabelIndexSeek.cpp index 9c95008c97d..d2245a34e1a 100644 --- a/src/graph/planner/match/LabelIndexSeek.cpp +++ b/src/graph/planner/match/LabelIndexSeek.cpp @@ -85,7 +85,7 @@ StatusOr LabelIndexSeek::transformNode(NodeContext* nodeCtx) { // refactored auto* pool = nodeCtx->qctx->objPool(); if (nodeCtx->bindWhereClause != nullptr && nodeCtx->bindWhereClause->filter != nullptr) { - auto* filter = nodeCtx->bindWhereClause->filter; + auto* filter = ExpressionUtils::rewriteInnerInExpr(nodeCtx->bindWhereClause->filter); const auto& nodeAlias = nodeCtx->info->alias; const auto& schemaName = nodeCtx->scanInfo.schemaNames.back(); diff --git a/src/graph/planner/match/MatchClausePlanner.cpp b/src/graph/planner/match/MatchClausePlanner.cpp index 6d168bdfa2a..66ece1b3296 100644 --- a/src/graph/planner/match/MatchClausePlanner.cpp +++ b/src/graph/planner/match/MatchClausePlanner.cpp @@ -30,12 +30,6 @@ StatusOr MatchClausePlanner::transform(CypherClauseContextBase* clauseC auto& nodeInfos = iter->nodeInfos; SubPlan pathPlan; if (iter->pathType == Path::PathType::kDefault) { - // This filter is just used for index scan module which embedded in planner, e.g. - // PropIndexSeek, LabelIndexSeek So we need to rewrite the in list expr to or expr. - if (matchClauseCtx->where && matchClauseCtx->where->filter) { - matchClauseCtx->where->filter = - ExpressionUtils::rewriteInnerInExpr(matchClauseCtx->where->filter); - } MatchPathPlanner matchPathPlanner(matchClauseCtx, *iter); auto result = matchPathPlanner.transform(matchClauseCtx->where.get(), nodeAliasesSeen); NG_RETURN_IF_ERROR(result); diff --git a/src/graph/planner/match/PropIndexSeek.cpp b/src/graph/planner/match/PropIndexSeek.cpp index f3b75f0a54e..84e60edd81f 100644 --- a/src/graph/planner/match/PropIndexSeek.cpp +++ b/src/graph/planner/match/PropIndexSeek.cpp @@ -125,8 +125,9 @@ bool PropIndexSeek::matchNode(NodeContext* nodeCtx) { Expression* filterInWhere = nullptr; Expression* filterInPattern = nullptr; if (nodeCtx->bindWhereClause != nullptr && nodeCtx->bindWhereClause->filter != nullptr) { - filterInWhere = MatchSolver::makeIndexFilter( - node.labels.back(), node.alias, nodeCtx->bindWhereClause->filter, nodeCtx->qctx); + auto* newFilter = ExpressionUtils::rewriteInnerInExpr(nodeCtx->bindWhereClause->filter); + filterInWhere = + MatchSolver::makeIndexFilter(node.labels.back(), node.alias, newFilter, nodeCtx->qctx); } if (!node.labelProps.empty()) { auto props = node.labelProps.back(); diff --git a/tests/tck/features/optimizer/CasesUsingTestSpace.feature b/tests/tck/features/optimizer/CasesUsingTestSpace.feature index 2e27df659fd..fa04e9fc879 100644 --- a/tests/tck/features/optimizer/CasesUsingTestSpace.feature +++ b/tests/tck/features/optimizer/CasesUsingTestSpace.feature @@ -25,8 +25,7 @@ Feature: Push Filter Down Cases Using the test Space | 17 | project | 19 | | | 19 | aggregate | 24 | | | 24 | HashInnerJoin | 21,29 | | - | 21 | project | 20 | | - | 20 | filter | 6 | | + | 21 | project | 6 | | | 6 | AppendVertices | 26 | | | 26 | Traverse | 25 | | | 25 | Traverse | 2 | | diff --git a/tests/tck/features/optimizer/PushFilterDownTraverseRule.feature b/tests/tck/features/optimizer/PushFilterDownTraverseRule.feature index e30d7091b3a..5e564a794a2 100644 --- a/tests/tck/features/optimizer/PushFilterDownTraverseRule.feature +++ b/tests/tck/features/optimizer/PushFilterDownTraverseRule.feature @@ -1,6 +1,7 @@ # Copyright (c) 2022 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. +@jie Feature: Push Filter down Traverse rule Background: @@ -63,12 +64,12 @@ Feature: Push Filter down Traverse rule | 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 | | | + | id | name | dependencies | operator info | + | 6 | Project | 5 | | + | 5 | AppendVertices | 10 | | + | 10 | Traverse | 9 | {"filter": "(like._dst IN [\"Tony Parker\",\"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: """ @@ -90,12 +91,12 @@ Feature: Push Filter down Traverse rule | 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 | | | + | id | name | dependencies | operator info | + | 6 | Project | 5 | | + | 5 | AppendVertices | 10 | | + | 10 | Traverse | 9 | {"filter": "(like._src IN [\"Tony Parker\",\"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: """ @@ -149,6 +150,35 @@ Feature: Push Filter down Traverse rule | 14 | Traverse | 12 | | | | 12 | Traverse | 11 | | | | 11 | Argument | | | | + When profiling query: + """ + MATCH (v)-[e1:like]->(v1) WHERE id(v) == "Tony Parker" + WITH DISTINCT v1, e1.degree AS strength + ORDER BY strength DESC LIMIT 20 + MATCH (v1)<-[e2:like]-(v2) + WITH v1, e2, v2 WHERE none_direct_dst(e2) IN ["Yao Ming", "Tim Duncan"] + RETURN id(v2) AS candidate, count(*) AS cnt; + """ + Then the result should be, in any order, with relax comparison: + | candidate | cnt | + | "Tim Duncan" | 1 | + And the execution plan should be: + | id | name | dependencies | profiling data | operator info | + | 18 | Aggregate | 22 | | | + | 22 | Project | 25 | | | + | 25 | HashInnerJoin | 20,27 | | | + | 20 | TopN | 8 | | | + | 8 | Dedup | 19 | | | + | 19 | Project | 5 | | | + | 5 | AppendVertices | 4 | | | + | 4 | Traverse | 2 | | | + | 2 | Dedup | 1 | | | + | 1 | PassThrough | 3 | | | + | 3 | Start | | | | + | 27 | Project | 28 | | | + | 28 | AppendVertices | 30 | | | + | 30 | Traverse | 11 | | {"filter": "(like._dst IN [\"Yao Ming\",\"Tim Duncan\"])"} | + | 11 | Argument | | | | Scenario: push filter down Traverse with complex filter When profiling query: diff --git a/tests/tck/features/yield/parameter.feature b/tests/tck/features/yield/parameter.feature index 83f44a85fa8..12f1b2fc862 100644 --- a/tests/tck/features/yield/parameter.feature +++ b/tests/tck/features/yield/parameter.feature @@ -1,6 +1,7 @@ # Copyright (c) 2021 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License +@jie Feature: Parameter Background: @@ -93,8 +94,7 @@ Feature: Parameter | "Tony Parker" | And the execution plan should be: | id | name | dependencies | operator info | - | 9 | Project | 7 | | - | 7 | Filter | 2 | | + | 9 | Project | 2 | | | 2 | AppendVertices | 6 | | | 6 | Dedup | 6 | | | 6 | PassThrough | 0 | |