Eliminte the useless row collect.

[Shylock-Hg]

What type of PR is this?

• bug
• feature
• enhancement

What does this PR do?

Which issue(s)/PR(s) this PR relates to?

Special notes for your reviewer, ex. impact of this fix, etc:

Additional context:

Checklist：

• Documentation affected （Please add the label if documentation needs to be modified.)
• Incompatible （If it is incompatible, please describe it and add corresponding label.）
• Need to cherry-pick （If need to cherry-pick to some branches, please label the destination version(s).)
• Performance impacted: Consumes more CPU/Memory

Release notes：

Please confirm whether to reflect in release notes and how to describe:

                                                            `

[yixinglu]

Maybe we could also revise the DataCollect Node generation rule, check whether this node is not necessary in some scenarios. WDYT?

[codecov-commenter]

Codecov Report

Merging #3363 (832d4f9) into master (fe68b0e) will decrease coverage by 0.04%.
The diff coverage is 84.21%.

@@            Coverage Diff             @@
##           master    #3363      +/-   ##
==========================================
- Coverage   85.27%   85.23%   -0.05%     
==========================================
  Files        1276     1277       +1     
  Lines      118999   119022      +23     
==========================================
- Hits       101475   101444      -31     
- Misses      17524    17578      +54     

Impacted Files	Coverage Δ
src/common/expression/ContainerExpression.cpp	90.00% <66.66%> (-2.13%)	⬇️
...c/graph/optimizer/rule/EliminateRowCollectRule.cpp	88.46% <88.46%> (ø)
src/graph/validator/test/YieldValidatorTest.cpp	91.78% <100.00%> (ø)
src/parser/test/ParserTest.cpp	100.00% <100.00%> (ø)
src/meta/processors/parts/GetSpaceProcessor.cpp	60.60% <0.00%> (-18.19%)	⬇️
src/meta/processors/BaseProcessor.h	75.67% <0.00%> (-16.22%)	⬇️
src/common/thrift/ThriftClientManager-inl.h	82.97% <0.00%> (-6.39%)	⬇️
src/kvstore/wal/WalFileIterator.cpp	71.00% <0.00%> (-5.00%)	⬇️
...eta/processors/session/SessionManagerProcessor.cpp	74.19% <0.00%> (-4.52%)	⬇️
src/graph/executor/StorageAccessExecutor.h	66.66% <0.00%> (-2.57%)	⬇️
... and 26 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71c73e1...832d4f9. Read the comment docs.

[Shylock-Hg]

Maybe we could also revise the DataCollect Node generation rule, check whether this node is not necessary in some scenarios. WDYT?

The plan generated by planner is DataCollect -> Limit -> Project, after optimization it's DataCollect -> Project -> Limit. So in planner, we don't know it's useless.

[yixinglu]

Maybe we could also revise the DataCollect Node generation rule, check whether this node is not necessary in some scenarios. WDYT?

The plan generated by planner is DataCollect -> Limit -> Project, after optimization it's DataCollect -> Project -> Limit. So in planner, we don't know it's useless.

I mean you can check when we need to append the DataCollect plan node into the plan, maybe now it's not necessary to do that.

[czpmango]

Maybe we could also revise the DataCollect Node generation rule, check whether this node is not necessary in some scenarios. WDYT?

IMHO, it's unnecessary in most cases, bcz Project may also works. Besides, introducing redundant plannode types is not optimizer friendly.

nebula/src/graph/planner/SequentialPlanner.cpp

Lines 39 to 60 in 32394f4

	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;
	}