plan: covert max/min to Limit + Sort operators #5105
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/run-all-tests |
|
/run-all-tests |
1 similar comment
|
/run-all-tests |
|
@hanfei1991 @winoros PTAL |
hanfei1991
reviewed
Nov 15, 2017
plan/aggregation_eliminate.go
Outdated
| sort := Sort{}.init(a.ctx) | ||
| sort.ByItems = append(sort.ByItems, &ByItems{f.GetArgs()[0], desc}) | ||
| sort.SetSchema(p.Schema().Clone()) | ||
| sort.SetChildren(p.Children()...) |
plan/aggregation_eliminate.go
Outdated
| // Compose Sort operator. | ||
| sort := Sort{}.init(a.ctx) | ||
| sort.ByItems = append(sort.ByItems, &ByItems{f.GetArgs()[0], desc}) | ||
| sort.SetSchema(p.Schema().Clone()) |
There was a problem hiding this comment.
the schema of sort should be set as its child's
plan/physical_plans.go
Outdated
| @@ -562,6 +562,7 @@ func buildJoinSchema(joinType JoinType, join Plan, outerID int) *expression.Sche | |||
| return newSchema | |||
| } | |||
| return expression.MergeSchema(join.Children()[outerID].Schema(), join.Children()[1-outerID].Schema()) | |||
|
|
|||
|
@hanfei1991 @winoros PTAL |
hanfei1991
reviewed
Nov 16, 2017
plan/logical_plan_builder.go
Outdated
| @@ -63,14 +63,24 @@ func (p *LogicalAggregation) collectGroupByColumns() { | |||
| func (b *planBuilder) buildAggregation(p LogicalPlan, aggFuncList []*ast.AggregateFuncExpr, gbyItems []expression.Expression) (LogicalPlan, map[int]int) { | |||
| b.optFlag = b.optFlag | flagBuildKeyInfo | |||
| b.optFlag = b.optFlag | flagAggregationOptimize | |||
| eliminateAgg := len(gbyItems) == 0 | |||
There was a problem hiding this comment.
This optimize is too complex. If we have an aggregate, we set flag eliminateAgg. That's enough.
|
@coocood The CBO framework should consider this. |
winoros
reviewed
Nov 16, 2017
plan/logical_plan_builder.go
Outdated
| @@ -1491,6 +1505,9 @@ func (b *planBuilder) buildSelect(sel *ast.SelectStmt) LogicalPlan { | |||
| if b.err != nil { | |||
| return nil | |||
| } | |||
| if b.optFlag&flagAggEliminate == flagAggEliminate { | |||
There was a problem hiding this comment.
convert to topn will be no worse than original. it's ok not to do this
|
@hanfei1991 @winoros PTAL |
coocood
changed the title
|
So we can add some complex plan test. |
|
@winoros Add a new test case. I will add more tests in tidb-test. |
|
@shenli plan test meets building fail. |
|
@hanfei1991 Fixed. |
coocood
approved these changes
Nov 16, 2017
winoros
added a commit
to winoros/tidb
that referenced
this pull request
Nov 16, 2017
This reverts commit 4b669b7.
winoros
added a commit
that referenced
this pull request
Nov 16, 2017
winoros
pushed a commit
to winoros/tidb
that referenced
this pull request
Dec 27, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For SQL like
select max(c) from t;, we could optimize it toselect c from t order by c desc limit 1;.For SQL like
select min(c) from t;, we could optimize it toselect c from t order by c limit 1;.