Skip to content

Commit

Permalink
remove useless dc (#4533)
Browse files Browse the repository at this point in the history 
Co-authored-by: Sophie <[email protected]>
  • Loading branch information
@jievince @Sophie-Xie
jievince and Sophie-Xie authored Aug 18, 2022
1 parent 81080d3 commit 6317917
Show file tree
Hide file tree
Showing 15 changed files with 26 additions and 132 deletions.
26 changes: 0 additions & 26 deletions src/graph/planner/SequentialPlanner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ bool SequentialPlanner::match(AstContext* astCtx) {
StatusOr<SubPlan> SequentialPlanner::transform(AstContext* astCtx) {
SubPlan subPlan;
auto* seqCtx = static_cast<SequentialAstContext*>(astCtx);
auto* qctx = seqCtx->qctx;
const auto& validators = seqCtx->validators;
subPlan.root = validators.back()->root();
ifBuildDataCollect(subPlan, qctx);
for (auto iter = validators.begin(); iter < validators.end() - 1; ++iter) {
// Remove left tail kStart plannode before append plan.
// It allows that kUse sentence to append kMatch Sentence.
Expand All @@ -40,30 +38,6 @@ StatusOr<SubPlan> SequentialPlanner::transform(AstContext* astCtx) {
return subPlan;
}

void SequentialPlanner::ifBuildDataCollect(SubPlan& subPlan, QueryContext* qctx) {
switch (subPlan.root->kind()) {
case PlanNode::Kind::kSort:
case PlanNode::Kind::kLimit:
case PlanNode::Kind::kSample:
case PlanNode::Kind::kDedup:
case PlanNode::Kind::kUnion:
case PlanNode::Kind::kUnionAllVersionVar:
case PlanNode::Kind::kIntersect:
case PlanNode::Kind::kCartesianProduct:
case PlanNode::Kind::kMinus:
case PlanNode::Kind::kFilter: {
auto* dc = DataCollect::make(qctx, DataCollect::DCKind::kRowBasedMove);
dc->addDep(subPlan.root);
dc->setInputVars({subPlan.root->outputVar()});
dc->setColNames(subPlan.root->colNames());
subPlan.root = dc;
break;
}
default:
break;
}
}

// When appending plans, it need to remove left tail plannode.
// Because the left tail plannode is StartNode which needs to be removed,
// and remain one size for add dependency
Expand Down
2 changes: 0 additions & 2 deletions src/graph/planner/SequentialPlanner.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ class SequentialPlanner final : public Planner {
*/
StatusOr<SubPlan> transform(AstContext* astCtx) override;

void ifBuildDataCollect(SubPlan& subPlan, QueryContext* qctx);

void rmLeftTailStartNode(Validator* validator, Sentence::Kind appendPlanKind);

private:
Expand Down
8 changes: 1 addition & 7 deletions src/graph/validator/test/FetchEdgesTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,7 @@ TEST_F(FetchEdgesValidatorTest, FetchEdgesProp) {
auto *project = Project::make(qctx, filter, yieldColumns.get());
auto *dedup = Dedup::make(qctx, project);

// data collect
auto *dataCollect = DataCollect::make(qctx, DataCollect::DCKind::kRowBasedMove);
dataCollect->addDep(dedup);
dataCollect->setInputVars({dedup->outputVar()});
dataCollect->setColNames({"like.start", "like.end"});

auto result = Eq(qctx->plan()->root(), dataCollect);
auto result = Eq(qctx->plan()->root(), dedup);
ASSERT_TRUE(result.ok()) << result;
}
}
Expand Down
8 changes: 1 addition & 7 deletions src/graph/validator/test/FetchVerticesTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,13 +364,7 @@ TEST_F(FetchVerticesValidatorTest, FetchVerticesProp) {

auto *dedup = Dedup::make(qctx, project);

// data collect
auto *dataCollect = DataCollect::make(qctx, DataCollect::DCKind::kRowBasedMove);
dataCollect->addDep(dedup);
dataCollect->setInputVars({dedup->outputVar()});
dataCollect->setColNames({"person.name", "person.age"});

auto result = Eq(qctx->plan()->root(), dataCollect);
auto result = Eq(qctx->plan()->root(), dedup);
ASSERT_TRUE(result.ok()) << result;
}
// ON *
Expand Down
24 changes: 8 additions & 16 deletions src/graph/validator/test/MatchValidatorTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@ TEST_F(MatchValidatorTest, groupby) {
"avg(distinct n.person.age)+1 AS age,"
"labels(n) AS lb "
"ORDER BY id;";
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kDataCollect,
PlanNode::Kind::kSort,
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kSort,
PlanNode::Kind::kProject,
PlanNode::Kind::kAggregate,
PlanNode::Kind::kProject,
Expand All @@ -140,8 +139,7 @@ TEST_F(MatchValidatorTest, groupby) {
"labels(n) AS lb "
"ORDER BY id "
"SKIP 10 LIMIT 20;";
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kDataCollect,
PlanNode::Kind::kLimit,
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kLimit,
PlanNode::Kind::kSort,
PlanNode::Kind::kProject,
PlanNode::Kind::kAggregate,
Expand Down Expand Up @@ -220,8 +218,7 @@ TEST_F(MatchValidatorTest, groupby) {
"avg(distinct n.person.age) AS age,"
"labels(m) AS lb "
"SKIP 10 LIMIT 20;";
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kDataCollect,
PlanNode::Kind::kLimit,
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kLimit,
PlanNode::Kind::kAggregate,
PlanNode::Kind::kFilter,
PlanNode::Kind::kProject,
Expand All @@ -242,8 +239,7 @@ TEST_F(MatchValidatorTest, groupby) {
"min(n.person.age) AS min,"
"avg(distinct n.person.age) AS age,"
"labels(m) AS lb;";
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kDataCollect,
PlanNode::Kind::kDedup,
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kDedup,
PlanNode::Kind::kAggregate,
PlanNode::Kind::kFilter,
PlanNode::Kind::kProject,
Expand All @@ -265,8 +261,7 @@ TEST_F(MatchValidatorTest, groupby) {
"avg(distinct n.person.age)+1 AS age,"
"labels(m) AS lb "
"SKIP 10 LIMIT 20;";
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kDataCollect,
PlanNode::Kind::kLimit,
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kLimit,
PlanNode::Kind::kProject,
PlanNode::Kind::kAggregate,
PlanNode::Kind::kFilter,
Expand All @@ -290,8 +285,7 @@ TEST_F(MatchValidatorTest, groupby) {
"labels(m) AS lb "
"ORDER BY id "
"SKIP 10 LIMIT 20;";
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kDataCollect,
PlanNode::Kind::kLimit,
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kLimit,
PlanNode::Kind::kSort,
PlanNode::Kind::kDedup,
PlanNode::Kind::kProject,
Expand All @@ -317,8 +311,7 @@ TEST_F(MatchValidatorTest, groupby) {
"labels(m) AS lb "
"ORDER BY id "
"SKIP 10 LIMIT 20;";
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kDataCollect,
PlanNode::Kind::kLimit,
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kLimit,
PlanNode::Kind::kSort,
PlanNode::Kind::kDedup,
PlanNode::Kind::kAggregate,
Expand All @@ -343,8 +336,7 @@ TEST_F(MatchValidatorTest, groupby) {
"labels(m) AS lb "
"ORDER BY id "
"SKIP 10 LIMIT 20;";
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kDataCollect,
PlanNode::Kind::kLimit,
std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kLimit,
PlanNode::Kind::kSort,
PlanNode::Kind::kDedup,
PlanNode::Kind::kProject,
Expand Down
46 changes: 16 additions & 30 deletions src/graph/validator/test/QueryValidatorTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ TEST_F(QueryValidatorTest, GoNSteps) {
"GO 3 steps FROM \"1\",\"2\",\"3\" OVER like WHERE $^.person.age > 20"
"YIELD distinct $^.person.name";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect,
PK::kDedup,
PK::kProject,
PK::kFilter,
Expand All @@ -125,8 +124,7 @@ TEST_F(QueryValidatorTest, GoNSteps) {
std::string query =
"GO 2 STEPS FROM \"1\",\"2\",\"3\" OVER like WHERE $^.person.age > 20"
"YIELD distinct $^.person.name ";
std::vector<PlanNode::Kind> expected = {PK::kDataCollect,
PK::kDedup,
std::vector<PlanNode::Kind> expected = {PK::kDedup,
PK::kProject,
PK::kFilter,
PK::kGetNeighbors,
Expand Down Expand Up @@ -239,8 +237,7 @@ TEST_F(QueryValidatorTest, GoWithPipe) {
"GO 1 STEPS FROM \"1\" OVER like YIELD like._dst AS "
"id | GO 1 STEPS FROM $-.id OVER like "
"WHERE $-.id == \"2\" YIELD DISTINCT $-.id, like._dst";
std::vector<PlanNode::Kind> expected = {PK::kDataCollect,
PK::kDedup,
std::vector<PlanNode::Kind> expected = {PK::kDedup,
PK::kProject,
PK::kFilter,
PK::kInnerJoin,
Expand Down Expand Up @@ -286,11 +283,11 @@ TEST_F(QueryValidatorTest, GoWithPipe) {
"id | GO 2 STEPS FROM $-.id OVER like "
"WHERE $-.id == \"2\" YIELD DISTINCT $-.id, like._dst";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect, PK::kDedup, PK::kProject, PK::kFilter, PK::kInnerJoin,
PK::kInnerJoin, PK::kProject, PK::kGetNeighbors, PK::kLoop, PK::kDedup,
PK::kDedup, PK::kProject, PK::kProject, PK::kDedup, PK::kInnerJoin,
PK::kProject, PK::kDedup, PK::kProject, PK::kProject, PK::kGetNeighbors,
PK::kDedup, PK::kStart, PK::kProject, PK::kGetNeighbors, PK::kStart,
PK::kDedup, PK::kProject, PK::kFilter, PK::kInnerJoin, PK::kInnerJoin,
PK::kProject, PK::kGetNeighbors, PK::kLoop, PK::kDedup, PK::kDedup,
PK::kProject, PK::kProject, PK::kDedup, PK::kInnerJoin, PK::kProject,
PK::kDedup, PK::kProject, PK::kProject, PK::kGetNeighbors, PK::kDedup,
PK::kStart, PK::kProject, PK::kGetNeighbors, PK::kStart,
};
EXPECT_TRUE(checkResult(query, expected));
}
Expand Down Expand Up @@ -327,7 +324,6 @@ TEST_F(QueryValidatorTest, GoWithPipe) {
"YIELD DISTINCT $-.name, like.likeness + 1, $-.id, like._dst, "
"$$.person.name";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect,
PK::kDedup,
PK::kProject,
PK::kInnerJoin,
Expand Down Expand Up @@ -393,13 +389,12 @@ TEST_F(QueryValidatorTest, GoWithPipe) {
"YIELD DISTINCT $-.name, like.likeness + 1, $-.id, like._dst, "
"$$.person.name";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect, PK::kDedup, PK::kProject, PK::kInnerJoin, PK::kInnerJoin,
PK::kLeftJoin, PK::kProject, PK::kGetVertices, PK::kProject, PK::kGetNeighbors,
PK::kLoop, PK::kDedup, PK::kDedup, PK::kProject, PK::kProject,
PK::kDedup, PK::kInnerJoin, PK::kProject, PK::kDedup, PK::kProject,
PK::kProject, PK::kLeftJoin, PK::kDedup, PK::kProject, PK::kProject,
PK::kGetVertices, PK::kGetNeighbors, PK::kProject, PK::kStart, PK::kGetNeighbors,
PK::kStart,
PK::kDedup, PK::kProject, PK::kInnerJoin, PK::kInnerJoin, PK::kLeftJoin,
PK::kProject, PK::kGetVertices, PK::kProject, PK::kGetNeighbors, PK::kLoop,
PK::kDedup, PK::kDedup, PK::kProject, PK::kProject, PK::kDedup,
PK::kInnerJoin, PK::kProject, PK::kDedup, PK::kProject, PK::kProject,
PK::kLeftJoin, PK::kDedup, PK::kProject, PK::kProject, PK::kGetVertices,
PK::kGetNeighbors, PK::kProject, PK::kStart, PK::kGetNeighbors, PK::kStart,
};
EXPECT_TRUE(checkResult(query, expected));
}
Expand Down Expand Up @@ -603,7 +598,6 @@ TEST_F(QueryValidatorTest, GoOneStep) {
"YIELD DISTINCT $^.person.name, like._dst, "
"$$.person.name, $$.person.age + 1";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect,
PK::kDedup,
PK::kProject,
PK::kFilter,
Expand Down Expand Up @@ -641,7 +635,6 @@ TEST_F(QueryValidatorTest, GoOneStep) {
"GO FROM \"1\",\"2\",\"3\" OVER like WHERE $^.person.age > 20"
"YIELD distinct $^.person.name ";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect,
PK::kDedup,
PK::kProject,
PK::kFilter,
Expand Down Expand Up @@ -1011,7 +1004,7 @@ TEST_F(QueryValidatorTest, Limit) {
{
std::string query = "GO FROM \"Ann\" OVER like YIELD like._dst AS like | LIMIT 1, 3";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect, PK::kLimit, PK::kProject, PK::kGetNeighbors, PK::kStart};
PK::kLimit, PK::kProject, PK::kGetNeighbors, PK::kStart};
EXPECT_TRUE(checkResult(query, expected));
}
}
Expand All @@ -1021,8 +1014,7 @@ TEST_F(QueryValidatorTest, OrderBy) {
std::string query =
"GO FROM \"Ann\" OVER like YIELD $^.person.age AS age"
" | ORDER BY $-.age";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect, PK::kSort, PK::kProject, PK::kGetNeighbors, PK::kStart};
std::vector<PlanNode::Kind> expected = {PK::kSort, PK::kProject, PK::kGetNeighbors, PK::kStart};
EXPECT_TRUE(checkResult(query, expected));
}
// not exist factor
Expand All @@ -1040,7 +1032,7 @@ TEST_F(QueryValidatorTest, OrderByAndLimit) {
"GO FROM \"Ann\" OVER like YIELD $^.person.age AS age"
" | ORDER BY $-.age | LIMIT 1";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect, PK::kLimit, PK::kSort, PK::kProject, PK::kGetNeighbors, PK::kStart};
PK::kLimit, PK::kSort, PK::kProject, PK::kGetNeighbors, PK::kStart};
EXPECT_TRUE(checkResult(query, expected));
}
}
Expand All @@ -1053,7 +1045,6 @@ TEST_F(QueryValidatorTest, TestSetValidator) {
"\"2\" "
"OVER like YIELD like.start AS start";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect,
PK::kUnion,
PK::kProject,
PK::kProject,
Expand All @@ -1072,7 +1063,6 @@ TEST_F(QueryValidatorTest, TestSetValidator) {
"YIELD "
"like.start AS start";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect,
PK::kDedup,
PK::kUnion,
PK::kDedup,
Expand All @@ -1096,7 +1086,6 @@ TEST_F(QueryValidatorTest, TestSetValidator) {
"FROM \"2\" "
"OVER like YIELD like.start AS start";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect,
PK::kDedup,
PK::kUnion,
PK::kProject,
Expand All @@ -1121,7 +1110,6 @@ TEST_F(QueryValidatorTest, TestSetValidator) {
"GO FROM \"1\" OVER like YIELD like.start AS start INTERSECT GO FROM "
"\"2\" OVER like YIELD like.start AS start";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect,
PK::kIntersect,
PK::kProject,
PK::kProject,
Expand All @@ -1138,7 +1126,6 @@ TEST_F(QueryValidatorTest, TestSetValidator) {
"GO FROM \"1\" OVER like YIELD like.start AS start MINUS GO FROM "
"\"2\" OVER like YIELD like.start AS start";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect,
PK::kMinus,
PK::kProject,
PK::kProject,
Expand Down Expand Up @@ -1209,7 +1196,6 @@ TEST_F(QueryValidatorTest, TestMatch) {
"MATCH (:person{name:'Dwyane Wade'}) -[:like]-> () -[:like]-> (v3) "
"RETURN DISTINCT v3.person.name AS Name";
std::vector<PlanNode::Kind> expected = {
PK::kDataCollect,
PK::kDedup,
PK::kProject,
PK::kProject,
Expand Down
1 change: 0 additions & 1 deletion src/graph/validator/test/YieldValidatorTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,6 @@ TEST_F(YieldValidatorTest, AggCall) {
"like.likeness AS like"
"| YIELD DISTINCT AVG($-.age + 1), SUM($-.like), COUNT(*)";
expected_ = {
PlanNode::Kind::kDataCollect,
PlanNode::Kind::kDedup,
PlanNode::Kind::kAggregate,
PlanNode::Kind::kProject,
Expand Down
8 changes: 0 additions & 8 deletions tests/tck/features/lookup/LookUpLimit.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ Feature: Push Limit down IndexScan Rule
| /[a-zA-Z ']+/ |
And the execution plan should be:
| id | name | dependencies | operator info |
| 4 | DataCollect | 5 | |
| 5 | Sort | 6 | |
| 6 | Project | 7 | |
| 7 | Limit | 8 | {"count": "2"} |
Expand All @@ -33,7 +32,6 @@ Feature: Push Limit down IndexScan Rule
| /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ |
And the execution plan should be:
| id | name | dependencies | operator info |
| 4 | DataCollect | 5 | |
| 5 | Sort | 6 | |
| 6 | Project | 7 | |
| 7 | Limit | 8 | {"count": "2"} |
Expand All @@ -49,7 +47,6 @@ Feature: Push Limit down IndexScan Rule
| /[a-zA-Z ']+/ |
And the execution plan should be:
| id | name | dependencies | operator info |
| 4 | DataCollect | 5 | |
| 5 | Sort | 6 | |
| 6 | Project | 7 | |
| 7 | Limit | 8 | {"count": "2"} |
Expand All @@ -65,7 +62,6 @@ Feature: Push Limit down IndexScan Rule
| /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ |
And the execution plan should be:
| id | name | dependencies | operator info |
| 4 | DataCollect | 5 | |
| 5 | Sort | 6 | |
| 6 | Project | 7 | |
| 7 | Limit | 8 | {"count": "2"} |
Expand All @@ -84,7 +80,6 @@ Feature: Push Limit down IndexScan Rule
| /[a-zA-Z ']+/ |
And the execution plan should be:
| id | name | dependencies | operator info |
| 3 | DataCollect | 4 | |
| 4 | Sort | 5 | |
| 5 | Project | 7 | |
| 7 | Limit | 8 | |
Expand All @@ -101,7 +96,6 @@ Feature: Push Limit down IndexScan Rule
| /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ |
And the execution plan should be:
| id | name | dependencies | operator info |
| 3 | DataCollect | 4 | |
| 4 | Sort | 5 | |
| 5 | Project | 7 | |
| 7 | Limit | 8 | |
Expand All @@ -118,7 +112,6 @@ Feature: Push Limit down IndexScan Rule
| /[a-zA-Z ']+/ |
And the execution plan should be:
| id | name | dependencies | operator info |
| 3 | DataCollect | 4 | |
| 4 | Sort | 5 | |
| 5 | Project | 7 | |
| 7 | Limit | 8 | |
Expand All @@ -135,7 +128,6 @@ Feature: Push Limit down IndexScan Rule
| /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ |
And the execution plan should be:
| id | name | dependencies | operator info |
| 3 | DataCollect | 4 | |
| 4 | Sort | 5 | |
| 5 | Project | 7 | |
| 7 | Limit | 8 | |
Expand Down
Loading

0 comments on commit 6317917

Please sign in to comment.