Skip to content

Commit

Permalink
Fix the dependency of argument plan node (vesoft-inc#4939)
Browse files Browse the repository at this point in the history
* Fix the dependency of argument plan node

* Add test features

* make ps

* Rename feature and scenario

* Fix tck tests

Co-authored-by: Sophie <[email protected]>
  • Loading branch information
yixinglu and Sophie-Xie authored Nov 28, 2022
1 parent 611c670 commit 852512b
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 20 deletions.
4 changes: 2 additions & 2 deletions src/graph/executor/logic/ArgumentExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ folly::Future<Status> ArgumentExecutor::execute() {
ds.rows.reserve(iter->size());
std::unordered_set<Value> unique;
for (; iter->valid(); iter->next()) {
auto val = iter->getColumn(alias);
auto &val = iter->getColumn(alias);
if (!val.isVertex()) {
return Status::Error("Argument only support vertex, but got %s, which is type %s, ",
val.toString().c_str(),
val.typeName().c_str());
}
if (unique.emplace(val.getVertex().vid).second) {
Row row;
row.values.emplace_back(std::move(val));
row.values.emplace_back(val);
ds.rows.emplace_back(std::move(row));
}
}
Expand Down
8 changes: 2 additions & 6 deletions src/graph/planner/match/MatchPathPlanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,7 @@ Status MatchPathPlanner::findStarts(
auto nodeFinder = finder();
if (nodeFinder->match(&nodeCtx)) {
auto plan = nodeFinder->transform(&nodeCtx);
if (!plan.ok()) {
return plan.status();
}
NG_RETURN_IF_ERROR(plan);
matchClausePlan = std::move(plan).value();
startIndex = i;
foundStart = true;
Expand All @@ -149,9 +147,7 @@ Status MatchPathPlanner::findStarts(
auto edgeFinder = finder();
if (edgeFinder->match(&edgeCtx)) {
auto plan = edgeFinder->transform(&edgeCtx);
if (!plan.ok()) {
return plan.status();
}
NG_RETURN_IF_ERROR(plan);
matchClausePlan = std::move(plan).value();
startFromEdge = true;
startIndex = i;
Expand Down
2 changes: 1 addition & 1 deletion src/graph/planner/plan/Logic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ void Argument::cloneMembers(const Argument& arg) {
}

std::unique_ptr<PlanNodeDescription> Argument::explain() const {
auto desc = PlanNode::explain();
auto desc = SingleInputNode::explain();
addDescription("inputVar", inputVar(), desc.get());
return desc;
}
Expand Down
3 changes: 3 additions & 0 deletions tests/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,6 @@ clean:

kill:
ps -ef | grep -P '\sbin/nebula-' | grep "$$(whoami)" | sed 's/\s\s*/ /g' | cut -f2 -d' ' | xargs kill -9

ps:
@ps -ef | grep -P '\sbin/nebula-' | grep "$$(whoami)"
30 changes: 30 additions & 0 deletions tests/tck/features/bugfix/ArgumentPlanNodeDep.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
Feature: Fix Argument plan node dependency

Background:
Given a graph with space named "nba"

Scenario: fix argument plan node dependency in issue 4938
When profiling query:
"""
MATCH (a:player)
WHERE id(a)=='Tim Duncan'
MATCH (a)-[:like]-(b)
RETURN count(*) AS cnt
"""
Then the result should be, in any order:
| cnt |
| 12 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 11 | Aggregate | 10 | |
| 10 | BiInnerJoin | 9,4 | |
| 9 | Project | 8 | |
| 8 | AppendVertices | 7 | |
| 7 | Dedup | 6 | |
| 6 | PassThrough | 5 | |
| 5 | Start | | |
| 4 | Project | 3 | |
| 3 | AppendVertices | 2 | |
| 2 | Traverse | 1 | |
| 1 | Argument | 0 | |
| 0 | Start | | |
Loading

0 comments on commit 852512b

Please sign in to comment.