Skip to content

Commit

Permalink
Fix label tag expression toString function (#5008)
Browse files Browse the repository at this point in the history 
* Fix label tag property expression toString

* Fix LabelTagPropertyExpression toString

* Fix TCK

* Log query for opt error

* Cleanup and line wrap

* Fix tck tests

Co-authored-by: jie.wang <38901892+jievince@users.noreply.github.com>
  • Loading branch information
@yixinglu @jievince
yixinglu and jievince authored Dec 14, 2022
1 parent 4c6b05b commit 285ee34
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 33 deletions.
16 changes: 14 additions & 2 deletions src/common/expression/PropertyExpression.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,20 @@ void LabelTagPropertyExpression::accept(ExprVisitor* visitor) {
}

std::string LabelTagPropertyExpression::toString() const {
std::string labelStr = label_ != nullptr ? label_->toString().erase(0, 1) : "";
return labelStr + "." + sym_ + "." + prop_;
std::string labelStr;
if (label_ != nullptr) {
labelStr = label_->toString();
// Remove the leading '$' character for variable except '$-/$$/$^'
if (labelStr.find(kInputRef) != 0 && labelStr.find(kSrcRef) != 0 &&
labelStr.find(kDstRef) != 0) {
labelStr.erase(0, 1);
}
labelStr += ".";
}
if (!sym_.empty()) {
labelStr += sym_ + ".";
}
return labelStr + prop_;
}

bool LabelTagPropertyExpression::operator==(const Expression& rhs) const {
Expand Down
3 changes: 2 additions & 1 deletion src/graph/service/QueryInstance.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ Status QueryInstance::validateAndOptimize() {
// Optimize the query, and get the execution plan. We should not pass the optimizer errors to user
// since the message is often not easy to understand. Logging them is enough.
if (auto status = findBestPlan(); !status.ok()) {
LOG(ERROR) << "Error found in optimization stage: " << status.message();
LOG(ERROR) << "Error found in optimization stage for query: " << rctx->query()
<< ", error: " << status.message();
return Status::Error(
"There are some errors found in optimizer, "
"please contact to the admin to learn more details");
Expand Down
25 changes: 22 additions & 3 deletions tests/tck/features/bugfix/AliasTypeDeduce.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,40 @@ Feature: Test extract filter
Scenario: Extract filter bug
When executing query:
"""
match p=allShortestPaths((v1)-[:like*1..3]-(v2)) where id(v1)=="Tim Duncan" and id(v2)=="Tony Parker" with nodes(p) as pathNodes WITH [n IN pathNodes | id(n)] AS personIdsInPath, [idx IN range(1, size(pathNodes)-1) | [prev IN [pathNodes[idx-1]] | [curr IN [pathNodes[idx]] | [prev, curr]]]] AS vertList UNWIND vertList AS c WITH c[0][0][0] AS prev , c[0][0][1] AS curr OPTIONAL MATCH (curr)<-[e:like]-(:player)-[:serve]->(:team)-[:teammate]->(prev) RETURN count(e) AS cnt1, prev, curr
match p=allShortestPaths((v1)-[:like*1..3]-(v2))
where id(v1)=="Tim Duncan" and id(v2)=="Tony Parker"
with nodes(p) as pathNodes
WITH
[n IN pathNodes | id(n)] AS personIdsInPath,
[idx IN range(1, size(pathNodes)-1) | [prev IN [pathNodes[idx-1]] | [curr IN [pathNodes[idx]] | [prev, curr]]]] AS vertList
UNWIND vertList AS c
WITH c[0][0][0] AS prev , c[0][0][1] AS curr
OPTIONAL MATCH (curr)<-[e:like]-(:player)-[:serve]->(:team)-[:teammate]->(prev)
RETURN count(e) AS cnt1, prev, curr
"""
Then the result should be, in any order:
| cnt1 | prev | curr |
| 0 | ("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"}) | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) |
When executing query:
"""
match p=(a:player)-[e:like*1..3]->(b) where b.player.age>42 with relationships(p)[1] AS e1 match (b)-[:serve]->(c) where c.team.name>"S" and (b)-[e1]->() return count(c)
match p=(a:player)-[e:like*1..3]->(b)
where b.player.age>42
with relationships(p)[1] AS e1
match (b)-[:serve]->(c)
where c.team.name>"S" and (b)-[e1]->()
return count(c)
"""
Then the result should be, in any order:
| count(c) |
| 3225 |
When executing query:
"""
match p=(a:player)-[e:like*1..3]->(b) where b.player.age>42 with relationships(p)[1..2][0] AS e1 match (b)-[:serve]->(c) where c.team.name>"S" and (b)-[e1]->() return count(c)
match p=(a:player)-[e:like*1..3]->(b)
where b.player.age>42
with relationships(p)[1..2][0] AS e1
match (b)-[:serve]->(c)
where c.team.name>"S" and (b)-[e1]->()
return count(c)
"""
Then the result should be, in any order:
| count(c) |
Expand Down
23 changes: 12 additions & 11 deletions tests/tck/features/bugfix/PushFilterDownProject.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,21 @@ Feature: Test push filter down project
When profiling query:
"""
MATCH (n0)-[:like]->(n1)
WHERE (id(n0) IN ["Tim Duncan"])
WHERE id(n0) IN ['Tim Duncan']
WITH n1.player.age AS a0
WHERE ((a0 - (a0 + ((a0 % a0) + (a0 + a0)))) <= a0) RETURN count(*)
WHERE (a0 - (a0 + ((a0 % a0) + (a0 + a0)))) <= a0
RETURN count(*)
"""
Then the result should be, in any order:
| count(*) |
| 2 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 9 | Aggregate | 12 | |
| 12 | Project | 11 | |
| 11 | Filter | 5 | {"condition": "((-.n1.player.age-(-.n1.player.age+((-.n1.player.age%-.n1.player.age)+(-.n1.player.age+-.n1.player.age))))<=-.n1.player.age)"} |
| 5 | AppendVertices | 4 | |
| 4 | Traverse | 2 | |
| 2 | Dedup | 1 | |
| 1 | PassThrough | 3 | |
| 3 | Start | | |
| id | name | dependencies | operator info |
| 9 | Aggregate | 12 | |
| 12 | Project | 11 | |
| 11 | Filter | 5 | {"condition": "(($-.n1.player.age-($-.n1.player.age+(($-.n1.player.age%$-.n1.player.age)+($-.n1.player.age+$-.n1.player.age))))<=$-.n1.player.age)"} |
| 5 | AppendVertices | 4 | |
| 4 | Traverse | 2 | |
| 2 | Dedup | 1 | |
| 1 | PassThrough | 3 | |
| 3 | Start | | |
87 changes: 72 additions & 15 deletions tests/tck/features/optimizer/PushFilterDownHashInnerJoinRule.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,21 +149,78 @@ Feature: Push Filter down HashInnerJoin rule
| [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) |
| [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) |
And the execution plan should be:
| id | name | dependencies | operator info |
| 30 | Sort | 14 | |
| 14 | Project | 19 | |
| 19 | HashInnerJoin | 6,22 | |
| 6 | Project | 20 | |
| 20 | AppendVertices | 2 | |
| 2 | Dedup | 1 | |
| 1 | PassThrough | 3 | |
| 3 | Start | | |
| 22 | Project | 21 | |
| 21 | Filter | 10 | { "condition": "(($-.e[0].likeness>0) OR (-.v1.player.age>0))" } |
| 10 | AppendVertices | 9 | |
| 9 | Traverse | 7 | |
| 7 | Argument | 8 | |
| 8 | Start | | |
| id | name | dependencies | operator info |
| 30 | Sort | 14 | |
| 14 | Project | 19 | |
| 19 | HashInnerJoin | 6,22 | |
| 6 | Project | 20 | |
| 20 | AppendVertices | 2 | |
| 2 | Dedup | 1 | |
| 1 | PassThrough | 3 | |
| 3 | Start | | |
| 22 | Project | 21 | |
| 21 | Filter | 10 | { "condition": "(($-.e[0].likeness>0) OR ($-.v1.player.age>0))" } |
| 10 | AppendVertices | 9 | |
| 9 | Traverse | 7 | |
| 7 | Argument | 8 | |
| 8 | Start | | |
When profiling query:
"""
match (a:player)-[:like]->(b)
where b.player.age>30
match (b)-[:serve]->(c)
where c.team.name>'A' and b.player.age+a.player.age>40 and a.player.age<45
return a,b,c
order by a,b,c
limit 1
"""
Then the result should be, in any order:
| a | b | c |
| ("Amar'e Stoudemire" :player{age: 36, name: "Amar'e Stoudemire"}) | ("Steve Nash" :player{age: 45, name: "Steve Nash"}) | ("Lakers" :team{name: "Lakers"}) |
And the execution plan should be:
| id | name | dependencies | operator info |
| 16 | TopN | 25 | |
| 25 | HashInnerJoin | 27,29 | |
| 27 | Project | 30 | |
| 30 | Filter | 4 | {"condition": "((($-.b.player.age+$-.a.player.age)>40) AND ($-.a.player.age<45) AND ($-.b.player.age>30))"} |
| 4 | AppendVertices | 24 | |
| 24 | Traverse | 1 | |
| 1 | IndexScan | 2 | |
| 2 | Start | | |
| 29 | Project | 28 | |
| 28 | Filter | 9 | {"condition": "($-.c.team.name>\"A\")"} |
| 9 | AppendVertices | 8 | |
| 8 | Traverse | 7 | |
| 7 | Argument | | |
When profiling query:
"""
match (a:player)-[:like]->(b)
where b.player.age>30 or b.player.age>45
match (b)-[:serve]->(c)
where c.team.name>'A' or b.player.age+a.player.age>40 and a.player.age<45
return a,b,c
order by a,b,c
limit 1
"""
Then the result should be, in any order:
| a | b | c |
| ("Amar'e Stoudemire" :player{age: 36, name: "Amar'e Stoudemire"}) | ("Steve Nash" :player{age: 45, name: "Steve Nash"}) | ("Lakers" :team{name: "Lakers"}) |
And the execution plan should be:
| id | name | dependencies | operator info |
| 16 | TopN | 13 | |
| 13 | Project | 12 | |
| 12 | Filter | 11 | {"condition": "((c.team.name>\"A\") OR (((b.player.age+a.player.age)>40) AND (a.player.age<45)))" } |
| 11 | HashInnerJoin | 18,10 | |
| 18 | Project | 17 | |
| 17 | Filter | 4 | {"condition": "(($-.b.player.age>30) OR ($-.b.player.age>45))"} |
| 4 | AppendVertices | 19 | |
| 19 | Traverse | 1 | |
| 1 | IndexScan | 2 | |
| 2 | Start | | |
| 10 | Project | 9 | |
| 9 | AppendVertices | 8 | |
| 8 | Traverse | 7 | |
| 7 | Argument | | |

Scenario: NOT push filter down HashInnerJoin
When profiling query:
Expand Down
2 changes: 1 addition & 1 deletion tests/tck/features/optimizer/PushFilterDownTraverseRule.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ Feature: Push Filter down Traverse rule
| id | name | dependencies | operator info |
| 11 | TopN | 10 | |
| 10 | Project | 9 | |
| 9 | Filter | 4 | {"condition": "(-.v.player.age!=35)" } |
| 9 | Filter | 4 | {"condition": "($-.v.player.age!=35)" } |
| 4 | AppendVertices | 12 | |
| 12 | Traverse | 8 | {"filter": "((like.likeness+100)!=199)"} |
| 8 | IndexScan | 2 | |
Expand Down

0 comments on commit 285ee34

Please sign in to comment.