Skip to content

Commit

Permalink
Fix argument
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 committed Nov 29, 2022
1 parent 5425036 commit 718f153
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 718f153

Please sign in to comment.