From 9c5a8328375024447083928d4fa46c31033c3ef9 Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Thu, 6 Sep 2018 02:03:12 +0800 Subject: [PATCH 1/4] plan: use the inferred type as the column type in the schema --- plan/logical_plan_builder.go | 7 ++++++- plan/rule_aggregation_push_down.go | 4 +++- plan/rule_decorrelate.go | 4 +++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/plan/logical_plan_builder.go b/plan/logical_plan_builder.go index db807b5315e11..024752b9dedd1 100644 --- a/plan/logical_plan_builder.go +++ b/plan/logical_plan_builder.go @@ -115,7 +115,9 @@ func (b *planBuilder) buildAggregation(p LogicalPlan, aggFuncList []*ast.Aggrega for _, col := range p.Schema().Columns { newFunc := aggregation.NewAggFuncDesc(b.ctx, ast.AggFuncFirstRow, []expression.Expression{col}, false) plan4Agg.AggFuncs = append(plan4Agg.AggFuncs, newFunc) - schema4Agg.Append(col) + newCol, _ := col.Clone().(*expression.Column) + newCol.RetType = newFunc.RetTp + schema4Agg.Append(newCol) } plan4Agg.SetChildren(p) plan4Agg.GroupByItems = gbyItems @@ -594,6 +596,9 @@ func (b *planBuilder) buildDistinct(child LogicalPlan, length int) *LogicalAggre } plan4Agg.SetChildren(child) plan4Agg.SetSchema(child.Schema().Clone()) + for i, col := range plan4Agg.schema.Columns { + col.RetType = plan4Agg.AggFuncs[i].RetTp + } // Distinct will be rewritten as first_row, we reset the type here since the return type // of first_row is not always the same as the column arg of first_row. for i, col := range plan4Agg.schema.Columns { diff --git a/plan/rule_aggregation_push_down.go b/plan/rule_aggregation_push_down.go index 3089f1d5c9cfd..0cd8d2c0fbc94 100644 --- a/plan/rule_aggregation_push_down.go +++ b/plan/rule_aggregation_push_down.go @@ -257,8 +257,10 @@ func (a *aggregationOptimizer) makeNewAgg(ctx sessionctx.Context, aggFuncs []*ag } for _, gbyCol := range gbyCols { firstRow := aggregation.NewAggFuncDesc(agg.ctx, ast.AggFuncFirstRow, []expression.Expression{gbyCol}, false) + newCol, _ := gbyCol.Clone().(*expression.Column) + newCol.RetType = firstRow.RetTp newAggFuncDescs = append(newAggFuncDescs, firstRow) - schema.Append(gbyCol) + schema.Append(newCol) } agg.AggFuncs = newAggFuncDescs agg.SetSchema(schema) diff --git a/plan/rule_decorrelate.go b/plan/rule_decorrelate.go index a56e30899a37b..761d80568d841 100644 --- a/plan/rule_decorrelate.go +++ b/plan/rule_decorrelate.go @@ -182,9 +182,10 @@ func (s *decorrelateSolver) optimize(p LogicalPlan) (LogicalPlan, error) { agg.SetSchema(apply.Schema()) agg.GroupByItems = expression.Column2Exprs(outerPlan.Schema().Keys[0]) newAggFuncs := make([]*aggregation.AggFuncDesc, 0, apply.Schema().Len()) - for _, col := range outerPlan.Schema().Columns { + for i, col := range outerPlan.Schema().Columns { first := aggregation.NewAggFuncDesc(agg.ctx, ast.AggFuncFirstRow, []expression.Expression{col}, false) newAggFuncs = append(newAggFuncs, first) + outerPlan.Schema().Columns[i].RetType = first.RetTp } newAggFuncs = append(newAggFuncs, agg.AggFuncs...) agg.AggFuncs = newAggFuncs @@ -229,6 +230,7 @@ func (s *decorrelateSolver) optimize(p LogicalPlan) (LogicalPlan, error) { newFunc := aggregation.NewAggFuncDesc(apply.ctx, ast.AggFuncFirstRow, []expression.Expression{clonedCol}, false) agg.AggFuncs = append(agg.AggFuncs, newFunc) agg.schema.Append(clonedCol.(*expression.Column)) + agg.schema.Columns[agg.schema.Len()-1].RetType = newFunc.RetTp } // If group by cols don't contain the join key, add it into this. if agg.getGbyColIndex(eqCond.GetArgs()[1].(*expression.Column)) == -1 { From bfc77549b14bc024fc45e6c6cbf18e63835d87f3 Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Mon, 10 Sep 2018 17:18:53 +0800 Subject: [PATCH 2/4] add UT && address comment --- executor/aggregate_test.go | 11 +++++++++++ plan/logical_plan_builder.go | 3 --- plan/rule_decorrelate.go | 9 +++++++-- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/executor/aggregate_test.go b/executor/aggregate_test.go index 3ce424ca18106..0ef14aefa4d39 100644 --- a/executor/aggregate_test.go +++ b/executor/aggregate_test.go @@ -602,3 +602,14 @@ func (s *testSuite) TestBuildProjBelowAgg(c *C) { "3 3 15 6,6,6 7", "4 3 18 7,7,7 8")) } + +func (s *testSuite) TestFirstRowEnum(c *C) { + tk := testkit.NewTestKitWithInit(c, s.store) + tk.MustExec(`use test;`) + tk.MustExec(`drop table if exists t;`) + tk.MustExec(`create table t(a enum('a', 'b'));`) + tk.MustExec(`insert into t values('a');`) + tk.MustQuery(`select a from t group by a;`).Check(testkit.Rows( + `a`, + )) +} diff --git a/plan/logical_plan_builder.go b/plan/logical_plan_builder.go index 024752b9dedd1..b6cb4dff31862 100644 --- a/plan/logical_plan_builder.go +++ b/plan/logical_plan_builder.go @@ -596,9 +596,6 @@ func (b *planBuilder) buildDistinct(child LogicalPlan, length int) *LogicalAggre } plan4Agg.SetChildren(child) plan4Agg.SetSchema(child.Schema().Clone()) - for i, col := range plan4Agg.schema.Columns { - col.RetType = plan4Agg.AggFuncs[i].RetTp - } // Distinct will be rewritten as first_row, we reset the type here since the return type // of first_row is not always the same as the column arg of first_row. for i, col := range plan4Agg.schema.Columns { diff --git a/plan/rule_decorrelate.go b/plan/rule_decorrelate.go index 761d80568d841..1e7172eafefbf 100644 --- a/plan/rule_decorrelate.go +++ b/plan/rule_decorrelate.go @@ -182,14 +182,19 @@ func (s *decorrelateSolver) optimize(p LogicalPlan) (LogicalPlan, error) { agg.SetSchema(apply.Schema()) agg.GroupByItems = expression.Column2Exprs(outerPlan.Schema().Keys[0]) newAggFuncs := make([]*aggregation.AggFuncDesc, 0, apply.Schema().Len()) + + outerColsInSchema := make([]*expression.Column, 0, outerPlan.Schema().Len()) for i, col := range outerPlan.Schema().Columns { first := aggregation.NewAggFuncDesc(agg.ctx, ast.AggFuncFirstRow, []expression.Expression{col}, false) newAggFuncs = append(newAggFuncs, first) - outerPlan.Schema().Columns[i].RetType = first.RetTp + + outerCol, _ := outerPlan.Schema().Columns[i].Clone().(*expression.Column) + outerCol.RetType = first.RetTp + outerColsInSchema = append(outerColsInSchema, outerCol) } newAggFuncs = append(newAggFuncs, agg.AggFuncs...) agg.AggFuncs = newAggFuncs - apply.SetSchema(expression.MergeSchema(outerPlan.Schema(), innerPlan.Schema())) + apply.SetSchema(expression.MergeSchema(expression.NewSchema(outerColsInSchema...), innerPlan.Schema())) np, err := s.optimize(p) if err != nil { return nil, errors.Trace(err) From 65baafc89d212f059117a64c5abb52b91fb07151 Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Tue, 11 Sep 2018 21:30:15 +0800 Subject: [PATCH 3/4] address comment --- plan/logical_plan_builder.go | 1 + plan/rule_aggregation_push_down.go | 1 + plan/rule_decorrelate.go | 1 + 3 files changed, 3 insertions(+) diff --git a/plan/logical_plan_builder.go b/plan/logical_plan_builder.go index b6cb4dff31862..f1895079ab20a 100644 --- a/plan/logical_plan_builder.go +++ b/plan/logical_plan_builder.go @@ -116,6 +116,7 @@ func (b *planBuilder) buildAggregation(p LogicalPlan, aggFuncList []*ast.Aggrega newFunc := aggregation.NewAggFuncDesc(b.ctx, ast.AggFuncFirstRow, []expression.Expression{col}, false) plan4Agg.AggFuncs = append(plan4Agg.AggFuncs, newFunc) newCol, _ := col.Clone().(*expression.Column) + newCol.UniqueID = plan4Agg.ctx.GetSessionVars().AllocPlanColumnID() newCol.RetType = newFunc.RetTp schema4Agg.Append(newCol) } diff --git a/plan/rule_aggregation_push_down.go b/plan/rule_aggregation_push_down.go index 0cd8d2c0fbc94..1465ce25267e5 100644 --- a/plan/rule_aggregation_push_down.go +++ b/plan/rule_aggregation_push_down.go @@ -258,6 +258,7 @@ func (a *aggregationOptimizer) makeNewAgg(ctx sessionctx.Context, aggFuncs []*ag for _, gbyCol := range gbyCols { firstRow := aggregation.NewAggFuncDesc(agg.ctx, ast.AggFuncFirstRow, []expression.Expression{gbyCol}, false) newCol, _ := gbyCol.Clone().(*expression.Column) + newCol.UniqueID = ctx.GetSessionVars().AllocPlanColumnID() newCol.RetType = firstRow.RetTp newAggFuncDescs = append(newAggFuncDescs, firstRow) schema.Append(newCol) diff --git a/plan/rule_decorrelate.go b/plan/rule_decorrelate.go index 1e7172eafefbf..4dc57372b615b 100644 --- a/plan/rule_decorrelate.go +++ b/plan/rule_decorrelate.go @@ -189,6 +189,7 @@ func (s *decorrelateSolver) optimize(p LogicalPlan) (LogicalPlan, error) { newAggFuncs = append(newAggFuncs, first) outerCol, _ := outerPlan.Schema().Columns[i].Clone().(*expression.Column) + outerCol.UniqueID = agg.ctx.GetSessionVars().AllocPlanColumnID() outerCol.RetType = first.RetTp outerColsInSchema = append(outerColsInSchema, outerCol) } From 0549c33f609a16713afe8e472e338b8736c455d1 Mon Sep 17 00:00:00 2001 From: zz-jason Date: Wed, 12 Sep 2018 14:41:09 +0800 Subject: [PATCH 4/4] fix ci --- plan/logical_plan_builder.go | 1 - plan/rule_aggregation_push_down.go | 1 - plan/rule_decorrelate.go | 1 - 3 files changed, 3 deletions(-) diff --git a/plan/logical_plan_builder.go b/plan/logical_plan_builder.go index fe24e2127a42d..871444b823603 100644 --- a/plan/logical_plan_builder.go +++ b/plan/logical_plan_builder.go @@ -116,7 +116,6 @@ func (b *planBuilder) buildAggregation(p LogicalPlan, aggFuncList []*ast.Aggrega newFunc := aggregation.NewAggFuncDesc(b.ctx, ast.AggFuncFirstRow, []expression.Expression{col}, false) plan4Agg.AggFuncs = append(plan4Agg.AggFuncs, newFunc) newCol, _ := col.Clone().(*expression.Column) - newCol.UniqueID = plan4Agg.ctx.GetSessionVars().AllocPlanColumnID() newCol.RetType = newFunc.RetTp schema4Agg.Append(newCol) } diff --git a/plan/rule_aggregation_push_down.go b/plan/rule_aggregation_push_down.go index 1465ce25267e5..0cd8d2c0fbc94 100644 --- a/plan/rule_aggregation_push_down.go +++ b/plan/rule_aggregation_push_down.go @@ -258,7 +258,6 @@ func (a *aggregationOptimizer) makeNewAgg(ctx sessionctx.Context, aggFuncs []*ag for _, gbyCol := range gbyCols { firstRow := aggregation.NewAggFuncDesc(agg.ctx, ast.AggFuncFirstRow, []expression.Expression{gbyCol}, false) newCol, _ := gbyCol.Clone().(*expression.Column) - newCol.UniqueID = ctx.GetSessionVars().AllocPlanColumnID() newCol.RetType = firstRow.RetTp newAggFuncDescs = append(newAggFuncDescs, firstRow) schema.Append(newCol) diff --git a/plan/rule_decorrelate.go b/plan/rule_decorrelate.go index 4dc57372b615b..1e7172eafefbf 100644 --- a/plan/rule_decorrelate.go +++ b/plan/rule_decorrelate.go @@ -189,7 +189,6 @@ func (s *decorrelateSolver) optimize(p LogicalPlan) (LogicalPlan, error) { newAggFuncs = append(newAggFuncs, first) outerCol, _ := outerPlan.Schema().Columns[i].Clone().(*expression.Column) - outerCol.UniqueID = agg.ctx.GetSessionVars().AllocPlanColumnID() outerCol.RetType = first.RetTp outerColsInSchema = append(outerColsInSchema, outerCol) }