Skip to content

Commit

Permalink
Enhancement
Browse files Browse the repository at this point in the history
  • Loading branch information
@yixinglu
yixinglu committed Dec 3, 2022
1 parent f876e4f commit b829d79
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 51 deletions.
57 changes: 29 additions & 28 deletions src/graph/optimizer/Optimizer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,32 @@ void Optimizer::addBodyToGroupNode(OptContext *ctx,
gnode->addBody(body);
}

namespace {

bool findArgumentRefPlanNodeInPath(const std::vector<const PlanNode *> &path, PlanNode *argument) {
DCHECK_EQ(argument->kind(), PlanNode::Kind::kArgument);
for (int i = path.size() - 1; i >= 0; i--) {
const auto *pn = path[i];
if (pn->isBiInput()) {
DCHECK_LT(i, path.size() - 1);
const auto *bpn = static_cast<const BinaryInputNode *>(pn);
if (bpn->right() == path[i + 1]) {
// Argument is in the right side dependency of binary plan node, check the left child
// output columns
if (argument->isColumnsIncludedIn(bpn->left())) {
argument->setInputVar(bpn->left()->outputVar());
return true;
}
} else {
// Argument is in the left side dependency of binary plan node, continue to find
// next parent plan node
DCHECK_EQ(bpn->left(), path[i + 1]);
}
}
}
return false;
}

Status rewriteArgumentInputVarInternal(PlanNode *root,
uint16_t stackDepth,
bool &hasArgument,
Expand All @@ -143,34 +169,7 @@ Status rewriteArgumentInputVarInternal(PlanNode *root,
case 0: {
if (root->kind() == PlanNode::Kind::kArgument) {
hasArgument = true;
for (int i = path.size() - 1; i >= 0; i--) {
const auto *pn = path[i];
if (pn->isBiInput()) {
DCHECK_LT(i, path.size() - 1);
const auto *bpn = static_cast<const BinaryInputNode *>(pn);
if (bpn->right() == path[i + 1]) {
// Argument is in the right side dependency of binary plan node, check the left child
// output columns
const auto &colNames = bpn->left()->colNames();
bool found = true;
for (const auto &col : root->colNames()) {
if (std::find(colNames.begin(), colNames.end(), col) == colNames.end()) {
found = false;
break;
}
}
if (found) {
root->setInputVar(bpn->left()->outputVar());
break;
}
} else {
// Argument is in the left side dependency of binary plan node, continue to find
// next parent plan node
DCHECK_EQ(bpn->left(), path[i + 1]);
}
}
}
if (root->inputVar().empty()) {
if (!findArgumentRefPlanNodeInPath(path, root) || root->inputVar().empty()) {
return Status::Error("Could not find the right input variable for argument plan node");
}
}
Expand Down Expand Up @@ -207,6 +206,8 @@ Status rewriteArgumentInputVarInternal(PlanNode *root,
return Status::OK();
}

} // namespace

// static
Status Optimizer::rewriteArgumentInputVar(PlanNode *root) {
bool hasArgument = false;
Expand Down
15 changes: 6 additions & 9 deletions src/graph/planner/plan/PlanNode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,18 +354,15 @@ void PlanNode::setInputVar(const std::string& varname, size_t idx) {
}
}

Status PlanNode::isColumnsIncluded(const std::string& varname) const {
auto* refVar = qctx()->symTable()->getVar(varname);
// The referenced variable should have all columns used in current node
bool PlanNode::isColumnsIncludedIn(const PlanNode* other) const {
const auto& otherColNames = other->colNames();
for (auto& colName : colNames()) {
auto iter = std::find(refVar->colNames.begin(), refVar->colNames.end(), colName);
if (iter == refVar->colNames.end()) {
return Status::Error("the column referenced by Argument is not included in variable `%s': %s",
varname.c_str(),
colName.c_str());
auto iter = std::find(otherColNames.begin(), otherColNames.end(), colName);
if (iter == otherColNames.end()) {
return false;
}
}
return Status::OK();
return true;
}

std::unique_ptr<PlanNodeDescription> PlanNode::explain() const {
Expand Down
2 changes: 1 addition & 1 deletion src/graph/planner/plan/PlanNode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ class PlanNode {
return static_cast<const T*>(this);
}

Status isColumnsIncluded(const std::string& varname) const;
bool isColumnsIncludedIn(const PlanNode* other) const;

protected:
PlanNode(QueryContext* qctx, Kind kind);
Expand Down
74 changes: 61 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,64 @@ 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 profiling query:
"""
MATCH (a:player{name:'Tim Duncan'}), (b:player{name:'Tony Parker'})
WITH a AS b, b AS a
MATCH allShortestPaths((a)-[:like*1..3]-(b))
RETURN a, b
"""
Then the result should be, in any order, with relax comparison:
| a | b |
| ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | ("Tim Duncan" :player{name: "Tim Duncan"}) |
| ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | ("Tim Duncan" :player{name: "Tim Duncan"}) |
And the execution plan should be:
| id | name | dependencies | operator info |
| 19 | Project | 18 | |
| 18 | BiInnerJoin | 10,17 | |
| 10 | Project | 9 | |
| 9 | BiCartesianProduct | 24,25 | |
| 24 | AppendVertices | 20 | |
| 20 | IndexScan | 2 | |
| 2 | Start | | |
| 25 | AppendVertices | 21 | |
| 21 | IndexScan | 6 | |
| 6 | Start | | |
| 17 | Project | 16 | |
| 16 | ShortestPath | 15 | |
| 15 | BiCartesianProduct | 12,14 | |
| 12 | Project | 11 | |
| 11 | Argument | | |
| 14 | Project | 13 | |
| 13 | Argument | | |
When profiling query:
"""
MATCH (a:player{name:'Tim Duncan'}), (b:player{name:'Tony Parker'})
WITH a AS b, b AS a
OPTIONAL MATCH allShortestPaths((a)-[:like*1..3]-(b))
RETURN a, b
"""
Then the result should be, in any order, with relax comparison:
| a | b |
| ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | ("Tim Duncan" :player{name: "Tim Duncan"}) |
| ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | ("Tim Duncan" :player{name: "Tim Duncan"}) |
And the execution plan should be:
| id | name | dependencies | operator info |
| 19 | Project | 18 | |
| 18 | BiLeftJoin | 10,17 | |
| 10 | Project | 9 | |
| 9 | BiCartesianProduct | 24,25 | |
| 24 | AppendVertices | 20 | |
| 20 | IndexScan | 2 | |
| 2 | Start | | |
| 25 | AppendVertices | 21 | |
| 21 | IndexScan | 6 | |
| 6 | Start | | |
| 17 | Project | 16 | |
| 16 | ShortestPath | 15 | |
| 15 | BiCartesianProduct | 12,14 | |
| 12 | Project | 11 | |
| 11 | Argument | | |
| 14 | Project | 13 | |
| 13 | Argument | | |

0 comments on commit b829d79

Please sign in to comment.