From b7e8397ab59cdd6a06d92b1e11a41545c40442ae Mon Sep 17 00:00:00 2001 From: "kyle.cao" Date: Wed, 4 Jan 2023 11:51:47 +0800 Subject: [PATCH 1/2] Fix pattern expression with same edge variable add tck fmt --- src/common/function/FunctionManager.cpp | 9 ++++++ src/graph/planner/match/SegmentsConnector.cpp | 4 +-- src/graph/validator/MatchValidator.cpp | 19 ++++++++++--- .../features/bugfix/AliasTypeDeduce.feature | 4 +-- tests/tck/features/match/PathExpr.feature | 28 ++++++++++++++++--- .../match/PathExprRefLocalVariable.feature | 21 ++++++++++++++ 6 files changed, 73 insertions(+), 12 deletions(-) diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp index 74a762a93d9..11d41741cc8 100644 --- a/src/common/function/FunctionManager.cpp +++ b/src/common/function/FunctionManager.cpp @@ -1867,6 +1867,15 @@ FunctionManager::FunctionManager() { case Value::Type::EDGE: { return value.getEdge().id(); } + // The root cause is the edge-type data format of Traverse executor + case Value::Type::LIST: { + auto &edges = value.getList().values; + if (edges.size() == 1 && edges[0].isEdge()) { + return edges[0].getEdge().id(); + } else { + return args[0]; + } + } default: { // Join on the origin type return args[0]; diff --git a/src/graph/planner/match/SegmentsConnector.cpp b/src/graph/planner/match/SegmentsConnector.cpp index cc29fb85cb1..813a4208eaf 100644 --- a/src/graph/planner/match/SegmentsConnector.cpp +++ b/src/graph/planner/match/SegmentsConnector.cpp @@ -79,7 +79,7 @@ SubPlan SegmentsConnector::rollUpApply(CypherClauseContextBase* ctx, std::vector compareProps; for (const auto& col : path.compareVariables) { compareProps.emplace_back(FunctionCallExpression::make( - qctx->objPool(), "id", {InputPropertyExpression::make(qctx->objPool(), col)})); + qctx->objPool(), "_joinkey", {InputPropertyExpression::make(qctx->objPool(), col)})); } InputPropertyExpression* collectProp = InputPropertyExpression::make(qctx->objPool(), collectCol); auto* rollUpApply = RollUpApply::make( @@ -104,7 +104,7 @@ SubPlan SegmentsConnector::rollUpApply(CypherClauseContextBase* ctx, std::vector keyProps; for (const auto& col : path.compareVariables) { keyProps.emplace_back(FunctionCallExpression::make( - qctx->objPool(), "id", {InputPropertyExpression::make(qctx->objPool(), col)})); + qctx->objPool(), "_joinkey", {InputPropertyExpression::make(qctx->objPool(), col)})); } auto* patternApply = PatternApply::make( qctx, left.root, DCHECK_NOTNULL(right.root), std::move(keyProps), path.isAntiPred); diff --git a/src/graph/validator/MatchValidator.cpp b/src/graph/validator/MatchValidator.cpp index 56c2505deba..e47de251cf7 100644 --- a/src/graph/validator/MatchValidator.cpp +++ b/src/graph/validator/MatchValidator.cpp @@ -1240,7 +1240,7 @@ Status MatchValidator::validatePathInWhere( matchPath.alias()->c_str()); } if (find->second != AliasType::kPath) { - return Status::SemanticError("Alias `%s' should be Path, but got type '%s", + return Status::SemanticError("`%s' is defined with type %s, but referenced with type Path", matchPath.alias()->c_str(), AliasTypeName::get(find->second).c_str()); } @@ -1258,7 +1258,7 @@ Status MatchValidator::validatePathInWhere( node->alias().c_str()); } if (find->second != AliasType::kNode) { - return Status::SemanticError("Alias `%s' should be Node, but got type '%s", + return Status::SemanticError("`%s' is defined with type %s, but referenced with type Node", node->alias().c_str(), AliasTypeName::get(find->second).c_str()); } @@ -1272,11 +1272,17 @@ Status MatchValidator::validatePathInWhere( "PatternExpression are not allowed to introduce new variables: `%s'.", edge->alias().c_str()); } - if (find->second != AliasType::kEdge) { - return Status::SemanticError("Alias `%s' should be Edge, but got type '%s'", + if (!edge->range() && find->second != AliasType::kEdge) { + return Status::SemanticError("`%s' is defined with type %s, but referenced with type Edge", edge->alias().c_str(), AliasTypeName::get(find->second).c_str()); } + if (edge->range() && find->second != AliasType::kEdgeList) { + return Status::SemanticError( + "`%s' is defined with type %s, but referenced with type EdgeList", + edge->alias().c_str(), + AliasTypeName::get(find->second).c_str()); + } } } return Status::OK(); @@ -1290,6 +1296,11 @@ Status MatchValidator::validatePathInWhere( pathInfo.compareVariables.emplace_back(node->alias()); } } + for (const auto &edge : path->edges()) { + if (edge->alias()[0] != '_') { + pathInfo.compareVariables.emplace_back(edge->alias()); + } + } pathInfo.collectVariable = *path->alias(); pathInfo.rollUpApply = true; return Status::OK(); diff --git a/tests/tck/features/bugfix/AliasTypeDeduce.feature b/tests/tck/features/bugfix/AliasTypeDeduce.feature index 958c2183788..4c9caaa6b1d 100644 --- a/tests/tck/features/bugfix/AliasTypeDeduce.feature +++ b/tests/tck/features/bugfix/AliasTypeDeduce.feature @@ -34,7 +34,7 @@ Feature: Test extract filter """ Then the result should be, in any order: | count(c) | - | 3225 | + | 49 | When executing query: """ match p=(a:player)-[e:like*1..3]->(b) @@ -46,4 +46,4 @@ Feature: Test extract filter """ Then the result should be, in any order: | count(c) | - | 3225 | + | 49 | diff --git a/tests/tck/features/match/PathExpr.feature b/tests/tck/features/match/PathExpr.feature index d4366d179c6..d115285b67d 100644 --- a/tests/tck/features/match/PathExpr.feature +++ b/tests/tck/features/match/PathExpr.feature @@ -50,22 +50,42 @@ Feature: Basic match """ MATCH (v:player) WITH (v)-[v]-() AS p RETURN p """ - Then a SemanticError should be raised at runtime: Alias `v' should be Edge, but got type 'Node' + Then a SemanticError should be raised at runtime: `v' is defined with type Node, but referenced with type Edge When executing query: """ MATCH (v:player) UNWIND (v)-[v]-() AS p RETURN p """ - Then a SemanticError should be raised at runtime: Alias `v' should be Edge, but got type 'Node' + Then a SemanticError should be raised at runtime: `v' is defined with type Node, but referenced with type Edge When executing query: """ MATCH (v:player) WHERE (v)-[v]-() RETURN v """ - Then a SemanticError should be raised at runtime: Alias `v' should be Edge, but got type 'Node' + Then a SemanticError should be raised at runtime: `v' is defined with type Node, but referenced with type Edge When executing query: """ MATCH (v:player) RETURN (v)-[v]-() """ - Then a SemanticError should be raised at runtime: Alias `v' should be Edge, but got type 'Node' + Then a SemanticError should be raised at runtime: `v' is defined with type Node, but referenced with type Edge + When executing query: + """ + MATCH (v:player)-[e*3]->() RETURN (v)-[e]-() + """ + Then a SemanticError should be raised at runtime: `e' is defined with type EdgeList, but referenced with type Edge + When executing query: + """ + MATCH (v:player)-[e]->() RETURN (v)-[e*1..3]-() + """ + Then a SemanticError should be raised at runtime: `e' is defined with type Edge, but referenced with type EdgeList + When executing query: + """ + MATCH (v:player)-[e]->() RETURN (e)-[*1..3]-() + """ + Then a SemanticError should be raised at runtime: `e' is defined with type Edge, but referenced with type Node + When executing query: + """ + MATCH (v:player)-[e*3]->() RETURN (e)-[*1..3]-() + """ + Then a SemanticError should be raised at runtime: `e' is defined with type EdgeList, but referenced with type Node Scenario: In Where When executing query: diff --git a/tests/tck/features/match/PathExprRefLocalVariable.feature b/tests/tck/features/match/PathExprRefLocalVariable.feature index 3c3e4b07cbe..5bdefdb96ae 100644 --- a/tests/tck/features/match/PathExprRefLocalVariable.feature +++ b/tests/tck/features/match/PathExprRefLocalVariable.feature @@ -100,6 +100,27 @@ Feature: Path expression reference local defined variables | name | | "Tim Duncan" | | "Tim Duncan" | + When executing query: + """ + MATCH (v:player)-[e:like]->(n) WHERE (n)-[e]->(:player) return v + """ + Then the result should be, in any order: + | v | + + @skip + Scenario: Fix after issue #2045 + When executing query: + """ + MATCH (v:player)-[e:like*1..3]->(n) WHERE (n)-[e*1..4]->(:player) return v + """ + Then the result should be, in any order: + | v | + When executing query: + """ + MATCH (v:player)-[e:like*3]->(n) WHERE id(v)=="Tim Duncan" and (n)-[e*3]->(:player) return v + """ + Then the result should be, in any order: + | v | Scenario: In With When executing query: From 1730f97fb83b067c571c092ac58b800a0e0ecc71 Mon Sep 17 00:00:00 2001 From: "kyle.cao" Date: Thu, 5 Jan 2023 11:11:45 +0800 Subject: [PATCH 2/2] add tck --- .../match/PathExprRefLocalVariable.feature | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/tests/tck/features/match/PathExprRefLocalVariable.feature b/tests/tck/features/match/PathExprRefLocalVariable.feature index 5bdefdb96ae..02355c3d134 100644 --- a/tests/tck/features/match/PathExprRefLocalVariable.feature +++ b/tests/tck/features/match/PathExprRefLocalVariable.feature @@ -102,10 +102,26 @@ Feature: Path expression reference local defined variables | "Tim Duncan" | When executing query: """ - MATCH (v:player)-[e:like]->(n) WHERE (n)-[e]->(:player) return v + MATCH (v:player)-[e:like]->(n) WHERE (n)-[e]->(:player) RETURN v """ Then the result should be, in any order: | v | + When executing query: + """ + MATCH (v:player)-[e]->(n) WHERE ()-[e]->(:player) and e.likeness<80 RETURN distinct v.player.name AS vname + """ + Then the result should be, in any order: + | vname | + | "Kyrie Irving" | + | "LaMarcus Aldridge" | + | "Dirk Nowitzki" | + | "Rudy Gay" | + | "Danny Green" | + | "Blake Griffin" | + | "Marco Belinelli" | + | "Vince Carter" | + | "Rajon Rondo" | + | "Ray Allen" | @skip Scenario: Fix after issue #2045 @@ -203,6 +219,26 @@ Feature: Path expression reference local defined variables | p | | [[<("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"})-[:serve@0 {end_year: 2016, start_year: 1997}]->("Spurs" :team{name: "Spurs"})>]] | | [[<("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"})-[:serve@0 {end_year: 2016, start_year: 1997}]->("Spurs" :team{name: "Spurs"})>]] | + When executing query: + """ + MATCH (v:player)-[e]->(n) WITH ()-[e{likeness:80}]->(:player) AS p1, ()-[e]-(:team) AS p2, v.player.name AS vname WHERE size(p1)>0 RETURN distinct * ORDER BY vname + """ + Then the result should be, in any order, with relax comparison: + | p1 | p2 | vname | + | [<("Aron Baynes" :player{age: 32, name: "Aron Baynes"})-[:like@0 {likeness: 80}]->("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})>] | [] | "Aron Baynes" | + | [<("Ben Simmons" :player{age: 22, name: "Ben Simmons"})-[:like@0 {likeness: 80}]->("Joel Embiid" :player{age: 25, name: "Joel Embiid"})>] | [] | "Ben Simmons" | + | [<("Boris Diaw" :player{age: 36, name: "Boris Diaw"})-[:like@0 {likeness: 80}]->("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})>] | [] | "Boris Diaw" | + | [<("Boris Diaw" :player{age: 36, name: "Boris Diaw"})-[:like@0 {likeness: 80}]->("Tony Parker" :player{age: 36, name: "Tony Parker"})>] | [] | "Boris Diaw" | + | [<("Damian Lillard" :player{age: 28, name: "Damian Lillard"})-[:like@0 {likeness: 80}]->("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"})>] | [] | "Damian Lillard" | + | [<("Danny Green" :player{age: 31, name: "Danny Green"})-[:like@0 {likeness: 80}]->("LeBron James" :player{age: 34, name: "LeBron James"})>] | [] | "Danny Green" | + | [<("Dirk Nowitzki" :player{age: 40, name: "Dirk Nowitzki"})-[:like@0 {likeness: 80}]->("Steve Nash" :player{age: 45, name: "Steve Nash"})>] | [] | "Dirk Nowitzki" | + | [<("Dirk Nowitzki" :player{age: 40, name: "Dirk Nowitzki"})-[:like@0 {likeness: 80}]->("Jason Kidd" :player{age: 45, name: "Jason Kidd"})>] | [] | "Dirk Nowitzki" | + | [<("James Harden" :player{age: 29, name: "James Harden"})-[:like@0 {likeness: 80}]->("Russell Westbrook" :player{age: 30, name: "Russell Westbrook"})>] | [] | "James Harden" | + | [<("Jason Kidd" :player{age: 45, name: "Jason Kidd"})-[:like@0 {likeness: 80}]->("Vince Carter" :player{age: 42, name: "Vince Carter"})>] | [] | "Jason Kidd" | + | [<("Joel Embiid" :player{age: 25, name: "Joel Embiid"})-[:like@0 {likeness: 80}]->("Ben Simmons" :player{age: 22, name: "Ben Simmons"})>] | [] | "Joel Embiid" | + | [<("Luka Doncic" :player{age: 20, name: "Luka Doncic"})-[:like@0 {likeness: 80}]->("James Harden" :player{age: 29, name: "James Harden"})>] | [] | "Luka Doncic" | + | [<("Shaquille O'Neal" :player{age: 47, name: "Shaquille O'Neal"})-[:like@0 {likeness: 80}]->("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})>] | [] | "Shaquille O'Neal" | + | [<("Tiago Splitter" :player{age: 34, name: "Tiago Splitter"})-[:like@0 {likeness: 80}]->("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})>] | [] | "Tiago Splitter" | Scenario: In Unwind When executing query: @@ -213,3 +249,10 @@ Feature: Path expression reference local defined variables | p | | [<("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"})-[:serve@0 {end_year: 2016, start_year: 1997}]->("Spurs" :team{name: "Spurs"})>] | | [<("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"})-[:serve@0 {end_year: 2016, start_year: 1997}]->("Spurs" :team{name: "Spurs"})>] | + When executing query: + """ + MATCH (v:player{name: 'Tim Duncan'})-[e:like*1..3]->(n), (t:team {name: "Spurs"}) WITH v, e, collect(distinct n) AS ns UNWIND [n in ns | ()-[e*2..4]->(n:player)] AS p RETURN count(p) AS count + """ + Then the result should be, in any order: + | count | + | 11 |