Skip to content

Commit

Permalink
Fix argument (#4950)
Browse files Browse the repository at this point in the history 
fix shortest path

fix argument input var

fix

fix tck

add tck

fix tck

fmt
  • Loading branch information
@czpmango
czpmango authored Nov 29, 2022
1 parent 5425036 commit 6c5cbdc
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 27 deletions.
5 changes: 4 additions & 1 deletion src/graph/planner/match/MatchPathPlanner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "graph/planner/match/WhereClausePlanner.h"
#include "graph/planner/plan/Algo.h"
#include "graph/planner/plan/Logic.h"
#include "graph/planner/plan/PlanNode.h"
#include "graph/planner/plan/Query.h"
#include "graph/util/ExpressionUtils.h"
#include "graph/util/SchemaUtil.h"
Expand Down Expand Up @@ -165,7 +166,9 @@ Status MatchPathPlanner::findStarts(
return Status::SemanticError("Can't solve the start vids from the sentence.");
}

if (matchClausePlan.tail->isSingleInput()) {
// Both StartNode and Argument are leaf plannodes
if (matchClausePlan.tail->isSingleInput() &&
matchClausePlan.tail->kind() != PlanNode::Kind::kArgument) {
auto start = StartNode::make(qctx);
matchClausePlan.tail->setDep(0, start);
matchClausePlan.tail = start;
Expand Down
5 changes: 5 additions & 0 deletions src/graph/planner/match/MatchPlanner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ Status MatchPlanner::connectMatchPlan(SubPlan& queryPlan, MatchClauseContext* ma
}
}
if (!intersectedAliases.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
9 changes: 6 additions & 3 deletions src/graph/planner/match/ShortestPathPlanner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,12 @@ StatusOr<SubPlan> ShortestPathPlanner::transform(
auto status = nodeFinder->transform(&nodeCtx);
NG_RETURN_IF_ERROR(status);
auto plan = status.value();
auto start = StartNode::make(qctx);
plan.tail->setDep(0, start);
plan.tail = start;
if (plan.tail->kind() != PlanNode::Kind::kStart &&
plan.tail->kind() != PlanNode::Kind::kArgument) {
auto start = StartNode::make(qctx);
plan.tail->setDep(0, start);
plan.tail = start;
}

auto initExpr = nodeCtx.initialExpr->clone();
auto columns = qctx->objPool()->makeAndAdd<YieldColumns>();
Expand Down
5 changes: 2 additions & 3 deletions src/graph/planner/plan/Logic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ std::unique_ptr<PlanNodeDescription> Select::explain() const {
return desc;
}

Argument::Argument(QueryContext* qctx, std::string alias, const PlanNode* dep)
: SingleInputNode(qctx, Kind::kArgument, dep) {
Argument::Argument(QueryContext* qctx, std::string alias) : PlanNode(qctx, Kind::kArgument) {
alias_ = alias;
// An argument is a kind of leaf node, it has no dependencies but read a variable.
inputVars_.emplace_back(nullptr);
Expand All @@ -87,7 +86,7 @@ void Argument::cloneMembers(const Argument& arg) {
}

std::unique_ptr<PlanNodeDescription> Argument::explain() const {
auto desc = SingleInputNode::explain();
auto desc = PlanNode::explain();
addDescription("inputVar", inputVar(), desc.get());
return desc;
}
Expand Down
8 changes: 4 additions & 4 deletions src/graph/planner/plan/Logic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,10 @@ class PassThroughNode final : public SingleInputNode {
};

// This operator is used for getting a named alias from another executed operator.
class Argument final : public SingleInputNode {
class Argument final : public PlanNode {
public:
static Argument* make(QueryContext* qctx, std::string alias, const PlanNode* dep = nullptr) {
return qctx->objPool()->makeAndAdd<Argument>(qctx, alias, dep);
static Argument* make(QueryContext* qctx, std::string alias) {
return qctx->objPool()->makeAndAdd<Argument>(qctx, alias);
}

PlanNode* clone() const override;
Expand All @@ -163,7 +163,7 @@ class Argument final : public SingleInputNode {

private:
friend ObjectPool;
Argument(QueryContext* qctx, std::string alias, const PlanNode* dep = nullptr);
Argument(QueryContext* qctx, std::string alias);

void cloneMembers(const Argument&);

Expand Down
6 changes: 2 additions & 4 deletions tests/tck/features/bugfix/ArgumentPlanNodeDep.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@ Feature: Fix Argument plan node dependency
And the execution plan should be:
| id | name | dependencies | operator info |
| 11 | Aggregate | 10 | |
| 10 | BiInnerJoin | 9,4 | |
| 9 | Project | 8 | |
| 10 | BiInnerJoin | 8,4 | |
| 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 | | |
| 1 | Argument | | |
12 changes: 12 additions & 0 deletions tests/tck/features/match/MultiLineMultiQueryParts.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,18 @@ Feature: Multi Line Multi Query Parts
| "Tim Duncan" | "Boris Diaw" | "Spurs" |
| "Tim Duncan" | "Boris Diaw" | "Suns" |
| "Tim Duncan" | "Boris Diaw" | "Tim Duncan" |
When executing query:
"""
MATCH (v:player{name:"Tim Duncan"})-[:like*1..3]->(n)
WITH DISTINCT v, n
MATCH (v)-[:serve]->(nn) return id(v) AS vid, id(nn) AS nnid
"""
Then the result should be, in any order:
| vid | nnid |
| "Tim Duncan" | "Spurs" |
| "Tim Duncan" | "Spurs" |
| "Tim Duncan" | "Spurs" |
| "Tim Duncan" | "Spurs" |
When executing query:
"""
MATCH (m)-[]-(n),(n) WHERE id(m)=="Tim Duncan" and id(n)=="Tony Parker"
Expand Down
18 changes: 6 additions & 12 deletions tests/tck/features/optimizer/PrunePropertiesRule.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,7 @@ Feature: Prune Properties rule
| 29 | Project | 8 | |
| 8 | AppendVertices | 7 | { "props": "[{\"tagId\":10,\"props\":[\"name\"]}]" } |
| 7 | Traverse | 6 | { "vertexProps": "[{\"tagId\":9,\"props\":[\"name\"]}]", "edgeProps": "[{\"type\": -5, \"props\": [\"_dst\", \"_rank\", \"_type\"]}, {\"type\": 5, \"props\": [\"_dst\", \"_rank\", \"_type\"]}, {\"props\": [\"_dst\", \"_rank\", \"_type\"], \"type\": -3}, {\"props\": [\"_dst\", \"_rank\", \"_type\"], \"type\": 3}, {\"type\": -4, \"props\": [\"_dst\", \"_rank\", \"_type\"]}, {\"props\": [\"_dst\", \"_rank\", \"_type\"], \"type\": 4}]" } |
| 6 | Argument | 31 | |
| 31 | Start | | |
| 6 | Argument | | |
| 30 | Project | 12 | |
| 12 | AppendVertices | 11 | { "props": "[{\"props\":[\"name\"],\"tagId\":9}]" } |
| 11 | Traverse | 10 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":10}]", "edgeProps": "[{\"type\": -5, \"props\": [\"_dst\", \"_rank\", \"_type\"]}, {\"type\": 5, \"props\": [\"_dst\", \"_rank\", \"_type\"]}, {\"props\": [\"_dst\", \"_rank\", \"_type\"], \"type\": -3}, {\"props\": [\"_dst\", \"_rank\", \"_type\"], \"type\": 3}, {\"type\": -4, \"props\": [\"_dst\", \"_rank\", \"_type\"]}, {\"props\": [\"_dst\", \"_rank\", \"_type\"], \"type\": 4}]" } |
Expand Down Expand Up @@ -264,8 +263,7 @@ Feature: Prune Properties rule
| 11 | Project | 10 | |
| 10 | AppendVertices | 9 | { "props": "[{\"tagId\":9,\"props\":[\"name\"]}, {\"tagId\":10,\"props\":[\"name\"]}]" } |
| 9 | Traverse | 8 | { "vertexProps": "[{\"tagId\":9,\"props\":[\"name\"]}]" } |
| 8 | Argument | 33 | |
| 33 | Start | | |
| 8 | Argument | | |
When profiling query:
"""
MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan"
Expand Down Expand Up @@ -300,13 +298,11 @@ Feature: Prune Properties rule
| 33 | Project | 10 | |
| 10 | AppendVertices | 9 | { "props": "[{\"tagId\":10,\"props\":[\"name\"]}]" } |
| 9 | Traverse | 8 | { "vertexProps": "[{\"tagId\":9,\"props\":[\"name\"]}]" } |
| 8 | Argument | 35 | |
| 35 | Start | | |
| 8 | Argument | | |
| 34 | Project | 13 | |
| 13 | AppendVertices | 12 | { "props": "[{\"tagId\":9,\"props\":[\"name\"]}]" } |
| 12 | Traverse | 11 | { "vertexProps": "[{\"tagId\":10,\"props\":[\"name\"]}]", "edgeProps": "[{\"props\": [\"_dst\", \"_rank\", \"_type\"], \"type\": -5}, {\"props\": [\"_dst\", \"_rank\", \"_type\"], \"type\": 5}, {\"type\": -3, \"props\": [\"_dst\", \"_rank\", \"_type\"]}, {\"props\": [\"_dst\", \"_rank\", \"_type\"], \"type\": 3}, {\"props\": [\"_dst\", \"_rank\", \"_type\"], \"type\": -4}, {\"props\": [\"_dst\", \"_rank\", \"_type\"], \"type\": 4}]" } |
| 11 | Argument | 36 | |
| 36 | Start | | |
| 11 | Argument | | |
When profiling query:
"""
MATCH (v:player{name:"Tony Parker"})
Expand Down Expand Up @@ -483,8 +479,7 @@ Feature: Prune Properties rule
| 12 | Project | 18 | |
| 18 | AppendVertices | 10 | { "props": "[{\"props\":[\"_tag\"],\"tagId\":10}]" } |
| 10 | Traverse | 8 | {"vertexProps": "[{\"props\":[\"name\", \"age\", \"_tag\"],\"tagId\":9}, {\"props\":[\"name\", \"speciality\", \"_tag\"],\"tagId\":8}, {\"props\":[\"name\", \"_tag\"],\"tagId\":10}]", "edgeProps": "[{\"type\": 4, \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" } |
| 8 | Argument | 9 | |
| 9 | Start | | |
| 8 | Argument | | |

@distonly
Scenario: union match
Expand Down Expand Up @@ -558,8 +553,7 @@ Feature: Prune Properties rule
| 15 | Traverse | 14 | {"vertexProps": "[{\"props\":[\"age\"],\"tagId\":9}]", "edgeProps": "[{\"type\": 4, \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" } |
| 14 | Traverse | 13 | {"vertexProps": "", "edgeProps": "[{\"type\": -3, \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" } |
| 13 | Traverse | 11 | {"vertexProps": "", "edgeProps": "[{\"type\": -3, \"props\": [\"_type\", \"_rank\", \"_dst\"]}, {\"type\": 3, \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" } |
| 11 | Argument | 12 | |
| 12 | Start | | |
| 11 | Argument | | |

@distonly
Scenario: test properties:
Expand Down

0 comments on commit 6c5cbdc

Please sign in to comment.