Fix pattern expression with same edge variable (#5192)

* Fix pattern expression with same edge variable

add tck

fmt

* add tck

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];

src/graph/planner/match/SegmentsConnector.cpp

@@ -79,7 +79,7 @@ SubPlan SegmentsConnector::rollUpApply(CypherClauseContextBase* ctx,
  std::vector<Expression*> 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<Expression*> 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);

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();

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  |

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:

tests/tck/features/match/PathExprRefLocalVariable.feature

@@ -100,6 +100,43 @@ 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 |
+ 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
+ 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:
@@ -182,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:
@@ -192,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 |