planner, executor: set new child after injecting Project operator #9684
Conversation
|
|
|
|
Codecov Report
@@ Coverage Diff @@
## master #9684 +/- ##
================================================
- Coverage 67.3879% 67.3862% -0.0018%
================================================
Files 377 377
Lines 79345 79353 +8
================================================
+ Hits 53469 53473 +4
- Misses 21104 21108 +4
Partials 4772 4772 |
| @@ -205,5 +206,9 @@ func injectProjBelowSort(p PhysicalPlan, orderByItems []*ByItems) PhysicalPlan { | |||
| if origChildProj, isChildProj := childPlan.(*PhysicalProjection); isChildProj { | |||
| refine4NeighbourProj(bottomProj, origChildProj) | |||
| } | |||
|
|
|||
| if topProj.Schema().Len() == 0 { | |||
There was a problem hiding this comment.
Add a comment for when will this be true
There was a problem hiding this comment.
If the schema length is 0, is this whole plan tree below projection needed?
…son/tidb into bugfix/inject-proj-on-topn
| TopN_17 1.00 root tmp.a:desc, offset:0, count:1 | ||
| └─Union_21 2.00 root | ||
| ├─Projection_22 1.00 root cast(a) | ||
| │ └─Projection_23 1.00 root 4 |
There was a problem hiding this comment.
If we have a rule to merge sequential Project operators, these two Project operator can be merged to one.
There was a problem hiding this comment.
If we change 33 to 3, would this Projection_22 be removed?
There was a problem hiding this comment.
Yes, because in this time, FieldTypes of "3" and "4" are equal, no cast function will be added in the logical plan building phase.
|
/run-all-tests |
|
Please fix ci and update the description of this PR. |
|
/run-all-tests |
What problem does this PR solve?
What is changed and how it works?
do as the title said.
Check List
Tests
Code changes
Related changes