Skip to content

Commit

Permalink
Fix variable types collected and graph crash (#4724)
Browse files Browse the repository at this point in the history
* Fix variable types collected and graph crash

add test cases

small fix

* unskip some test cases related to multiple query parts

* small delete

* fmt

* Fix ldbc BI R5 implementation

Co-authored-by: Harris.Chu <1726587+HarrisChu@users.noreply.github.com>
Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>
  • Loading branch information
3 people committed Oct 20, 2022
1 parent ab726c4 commit 5593627
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 17 deletions.
16 changes: 9 additions & 7 deletions src/graph/planner/match/MatchPlanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,19 @@ Status MatchPlanner::connectMatchPlan(SubPlan& queryPlan, MatchClauseContext* ma
if (matchCtx->isOptional) {
// connect LeftJoin match filter
auto& whereCtx = matchCtx->where;
if (whereCtx.get() != nullptr) {
auto exprs =
ExpressionUtils::collectAll(whereCtx->filter, {Expression::Kind::kLabelTagProperty});
if (whereCtx.get() != nullptr && whereCtx->filter != nullptr) {
auto exprs = ExpressionUtils::collectAll(
whereCtx->filter, {Expression::Kind::kVarProperty, Expression::Kind::kLabel});

// Check if all aliases in where clause are generated by the current match statement pattern
std::vector<std::string> aliases;
for (const auto* expr : exprs) {
DCHECK_EQ(expr->kind(), Expression::Kind::kLabelTagProperty);
auto* labelExpr = static_cast<const LabelTagPropertyExpression*>(expr)->label();
DCHECK_EQ(labelExpr->kind(), Expression::Kind::kVarProperty);
aliases.emplace_back(static_cast<const PropertyExpression*>(labelExpr)->prop());
if (expr->kind() == Expression::Kind::kVarProperty) {
aliases.emplace_back(static_cast<const PropertyExpression*>(expr)->prop());
} else {
DCHECK_EQ(expr->kind(), Expression::Kind::kLabel);
aliases.emplace_back(static_cast<const LabelExpression*>(expr)->name());
}
}
auto aliasesGenerated = matchCtx->aliasesGenerated;
if (!std::all_of(aliases.begin(), aliases.end(), [&aliasesGenerated](std::string& alias) {
Expand Down
47 changes: 37 additions & 10 deletions tests/tck/features/match/MultiLineMultiQueryParts.feature
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,37 @@ Feature: Multi Line Multi Query Parts
| "Tim Duncan" | "Manu Ginobili" | NULL |
| "Tim Duncan" | "Manu Ginobili" | NULL |
| "Tim Duncan" | "Manu Ginobili" | NULL |
When executing query:
"""
MATCH (m)-[]->(n) WHERE id(m)=="Tim Duncan"
OPTIONAL MATCH (n)-[]->(l) WHERE id(n)=="Tony Parker"
RETURN id(m) AS m, id(n) AS n, id(l) AS l;
"""
Then the result should be, in any order:
| m | n | l |
| "Tim Duncan" | "Spurs" | NULL |
| "Tim Duncan" | "LaMarcus Aldridge" | NULL |
| "Tim Duncan" | "Tony Parker" | "Spurs" |
| "Tim Duncan" | "Tony Parker" | "LaMarcus Aldridge" |
| "Tim Duncan" | "Tony Parker" | "LaMarcus Aldridge" |
| "Tim Duncan" | "Tony Parker" | "Kyle Anderson" |
| "Tim Duncan" | "Tony Parker" | "Tim Duncan" |
| "Tim Duncan" | "Tony Parker" | "Tim Duncan" |
| "Tim Duncan" | "Tony Parker" | "Manu Ginobili" |
| "Tim Duncan" | "Tony Parker" | "Manu Ginobili" |
| "Tim Duncan" | "Tony Parker" | "Hornets" |
| "Tim Duncan" | "Tony Parker" | "Spurs" |
| "Tim Duncan" | "Tony Parker" | "LaMarcus Aldridge" |
| "Tim Duncan" | "Tony Parker" | "LaMarcus Aldridge" |
| "Tim Duncan" | "Tony Parker" | "Kyle Anderson" |
| "Tim Duncan" | "Tony Parker" | "Tim Duncan" |
| "Tim Duncan" | "Tony Parker" | "Tim Duncan" |
| "Tim Duncan" | "Tony Parker" | "Manu Ginobili" |
| "Tim Duncan" | "Tony Parker" | "Manu Ginobili" |
| "Tim Duncan" | "Tony Parker" | "Hornets" |
| "Tim Duncan" | "Danny Green" | NULL |
| "Tim Duncan" | "Manu Ginobili" | NULL |
| "Tim Duncan" | "Manu Ginobili" | NULL |
When executing query:
"""
OPTIONAL match (v:player) WHERE v.player.age > 41
Expand Down Expand Up @@ -245,7 +276,6 @@ Feature: Multi Line Multi Query Parts
| count |
| 4 |

@skip
Scenario: Multi Line Multi Query Parts
When executing query:
"""
Expand Down Expand Up @@ -288,15 +318,6 @@ Feature: Multi Line Multi Query Parts
Then the result should be, in order:
| scount |
| 270 |
When executing query:
"""
MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan"
OPTIONAL MATCH (n)-->(v) WHERE v.player.age < m.player.age
RETURN count(*) AS count
"""
Then the result should be, in order:
| count |
| 45 |
When executing query:
"""
MATCH (a:player{age:42}) WITH a
Expand Down Expand Up @@ -361,3 +382,9 @@ Feature: Multi Line Multi Query Parts
RETURN n,v
"""
Then a SemanticError should be raised at runtime: The where clause of optional match statement that reference variables defined by other statements is not supported yet.
When executing query:
"""
MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan"
OPTIONAL MATCH (n)-->(v) WHERE id(v) < id(m) RETURN count(*) AS count
"""
Then a SemanticError should be raised at runtime: The where clause of optional match statement that reference variables defined by other statements is not supported yet.
1 change: 1 addition & 0 deletions tests/tck/ldbc/business_intelligence_workload/Read.feature
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ Feature: LDBC Business Intelligence Workload - Read
(forum)-[:HAS_MEMBER]->(person:Person)
OPTIONAL MATCH
(person)<-[:HAS_CREATOR]-(post:Post)<-[:CONTAINER_OF]-(popularForum:Forum)
WITH popularForums, popularForum, person, post
WHERE popularForum IN popularForums
RETURN
person.Person.id AS personId,
Expand Down

0 comments on commit 5593627

Please sign in to comment.