Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove useless datacollect plan node #4533

Merged
merged 2 commits into from
Aug 18, 2022
Merged

remove useless datacollect plan node #4533

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c39203
remove useless dc
jievince Aug 17, 2022
b6dcf86
Merge branch 'master' into remove-dc
Sophie-Xie Aug 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 0 additions & 26 deletions src/graph/planner/SequentialPlanner.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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