Skip to content

Commit

Permalink
Fix bug
Browse files Browse the repository at this point in the history
  • Loading branch information
@yixinglu
yixinglu committed Dec 3, 2022
1 parent c95055f commit f876e4f
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 43 deletions.
13 changes: 3 additions & 10 deletions src/graph/optimizer/Optimizer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,12 @@ Status rewriteArgumentInputVarInternal(PlanNode *root,

if (!root) return Status::OK();

path.push_back(root);
switch (root->numDeps()) {
case 0: {
if (root->kind() == PlanNode::Kind::kArgument) {
hasArgument = true;
path.push_back(root);
for (size_t i = path.size() - 1; i >= 0; i--) {
for (int i = path.size() - 1; i >= 0; i--) {
const auto *pn = path[i];
if (pn->isBiInput()) {
DCHECK_LT(i, path.size() - 1);
Expand Down Expand Up @@ -173,35 +173,28 @@ Status rewriteArgumentInputVarInternal(PlanNode *root,
if (root->inputVar().empty()) {
return Status::Error("Could not find the right input variable for argument plan node");
}
path.pop_back();
}
break;
}
case 1: {
path.push_back(root);
auto *dep = const_cast<PlanNode *>(root->dep());
NG_RETURN_IF_ERROR(rewriteArgumentInputVarInternal(dep, stackDepth, hasArgument, path));
path.pop_back();
break;
}
case 2: {
auto *bpn = static_cast<BinaryInputNode *>(root);
auto *left = const_cast<PlanNode *>(bpn->left());
path.push_back(left);
NG_RETURN_IF_ERROR(rewriteArgumentInputVarInternal(left, stackDepth, hasArgument, path));
path.pop_back();

auto *right = const_cast<PlanNode *>(bpn->right());
path.push_back(right);
NG_RETURN_IF_ERROR(rewriteArgumentInputVarInternal(right, stackDepth, hasArgument, path));
path.pop_back();
break;
}
default: {
return Status::Error(
"Invalid dependencies of plan node `%s': %lu", root->toString().c_str(), root->numDeps());
}
}
path.pop_back();

// Ensure that there's no argument plan node if loop/select plan nodes exist in execution plan
if (root->kind() == PlanNode::Kind::kLoop || root->kind() == PlanNode::Kind::kSelect) {
Expand Down
5 changes: 0 additions & 5 deletions src/graph/planner/match/MatchPlanner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,6 @@ Status MatchPlanner::connectMatchPlan(SubPlan& queryPlan, MatchClauseContext* ma
}
}
if (!interAliases.empty()) {
if (matchPlan.tail->kind() == PlanNode::Kind::kArgument) {
// The input of the argument operator is always the output of the plan on the other side of
// the join
matchPlan.tail->setInputVar(queryPlan.root->outputVar());
}
if (matchCtx->isOptional) {
// connect LeftJoin match filter
auto& whereCtx = matchCtx->where;
Expand Down
26 changes: 13 additions & 13 deletions tests/tck/features/match/AllShortestPaths.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -476,16 +476,16 @@ Feature: allShortestPaths
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.

# Scenario: allShortestPaths for argument issue
# When executing query:
# """
# MATCH (a:player), (b:player)
# MATCH p= allShortestPaths((a:player {name:"Tim Duncan"})-[*..15]-(b:player {age:33}))
# WITH nodes(p) AS pathNodes
# UNWIND pathNodes AS node
# RETURN DISTINCT node
# """
# Then the result should be, in any order, with relax comparison:
# | node |
# | ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) |
# | ("Dejounte Murray" :player{age: 29, name: "Dejounte Murray"}) |
# Scenario: allShortestPaths for argument issue
# When executing query:
# """
# MATCH (a:player), (b:player)
# MATCH p= allShortestPaths((a:player {name:"Tim Duncan"})-[*..15]-(b:player {age:33}))
# WITH nodes(p) AS pathNodes
# UNWIND pathNodes AS node
# RETURN DISTINCT node
# """
# Then the result should be, in any order, with relax comparison:
# | node |
# | ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) |
# | ("Dejounte Murray" :player{age: 29, name: "Dejounte Murray"}) |
30 changes: 15 additions & 15 deletions tests/tck/features/optimizer/PushFilterDownHashInnerJoinRule.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,21 +153,21 @@ 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 | BiInnerJoin | 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 | BiInnerJoin | 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 | | |

Scenario: NOT push filter down BiInnerJoin
When profiling query:
Expand Down

0 comments on commit f876e4f

Please sign in to comment.