plan, executor: add physical stream agg and remove aggType #5274
Conversation
|
/run-all-tests tidb-test=pr/415 |
1 similar comment
|
/run-all-tests tidb-test=pr/415 |
|
/run-common-test tidb-test=pr/415 |
executor/builder.go
Outdated
| return &HashAggExec{ | ||
| baseExecutor: newBaseExecutor(v.Schema(), b.ctx, b.build(v.Children()[0])), | ||
| sc: b.ctx.GetSessionVars().StmtCtx, | ||
| AggFuncs: v.AggFuncs, | ||
| GroupByItems: v.GroupByItems, | ||
| aggType: v.AggType, |
There was a problem hiding this comment.
Can we remove the field of aggType？
plan/gen_physical_plans.go
Outdated
| agg := basePhysicalAgg{ | ||
| GroupByItems: p.GroupByItems, | ||
| AggFuncs: p.AggFuncs, | ||
| }.initForStream(p.ctx, cols[:len(keys)], p.inputCount) |
There was a problem hiding this comment.
Why not use keys directly?
|
/run-integration-common-test tidb-test=pr/415 |
1 similar comment
|
/run-integration-common-test tidb-test=pr/415 |
|
/run-common-test tidb-test=pr/415 |
|
LGTM |
|
@hanfei1991 |
| func (p *PhysicalAggregation) ExplainInfo() string { | ||
| buffer := bytes.NewBufferString(fmt.Sprintf("type:%s", p.AggType)) | ||
| func (p *basePhysicalAgg) ExplainInfo() string { | ||
| buffer := bytes.NewBufferString("") |
|
@coocood This is just a refactor. Replace field 'aggType' with an individual operator |
|
@hanfei1991 |
|
/run-common-test tidb-test=pr/415 |
hanfei1991
changed the title
executor/builder.go
Outdated
|
|
||
| func (b *executorBuilder) buildStreamAgg(v *plan.PhysicalStreamAgg) Executor { | ||
| return &StreamAggExec{ | ||
| baseExecutor: newBaseExecutor(v.Schema(), b.ctx, b.build(v.Children()[0])), |
There was a problem hiding this comment.
childExec := b.build(v.Children()[0])
if b.err != nil {
b.err = errors.Trace(b.err)
return nil
}
|
/run-common-test tidb-test=pr/415 |
|
/run-common-test tidb-test=pr/415 |
|
@zz-jason PTAL |
|
/run-common-test tidb-test=pr/415 |
|
/run-common-test tidb-test=pr/415 |
add stream agg operator
@zimulala