From 4ec85cd783e0a4efffe4edb0facedd226dade534 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 14 Nov 2018 16:00:44 +0800 Subject: [PATCH] planner, executor: eliminate extra columns introduced by OrderBy upon Union (#8290) (#8306) --- executor/executor_test.go | 9 +++++++++ planner/core/logical_plan_builder.go | 20 ++++++++++++++++++-- planner/core/logical_plan_test.go | 5 +++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index 1aa27ce095110..2f9f004a1518c 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -1051,6 +1051,15 @@ func (s *testSuite) TestUnion(c *C) { tk.MustExec("insert into t1 value(1,2),(1,1),(2,2),(2,2),(3,2),(3,2)") tk.MustExec("set @@tidb_max_chunk_size=2;") tk.MustQuery("select count(*) from (select a as c, a as d from t1 union all select a, b from t1) t;").Check(testkit.Rows("12")) + + // #issue 8189 and #issue 8199 + tk.MustExec("drop table if exists t1") + tk.MustExec("drop table if exists t2") + tk.MustExec("CREATE TABLE t1 (a int not null, b char (10) not null)") + tk.MustExec("insert into t1 values(1,'a'),(2,'b'),(3,'c'),(3,'c')") + tk.MustExec("CREATE TABLE t2 (a int not null, b char (10) not null)") + tk.MustExec("insert into t2 values(1,'a'),(2,'b'),(3,'c'),(3,'c')") + tk.MustQuery("select a from t1 union select a from t1 order by (select a+1);").Check(testkit.Rows("1", "2", "3")) } func (s *testSuite) TestNeighbouringProj(c *C) { diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index a10cf7b82ec5e..42dec103e670c 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -705,6 +705,8 @@ func (b *planBuilder) buildUnion(union *ast.UnionStmt) (LogicalPlan, error) { unionPlan = unionAllPlan } + oldLen := unionPlan.Schema().Len() + if union.OrderBy != nil { unionPlan, err = b.buildSort(unionPlan, union.OrderBy.Items, nil) if err != nil { @@ -718,6 +720,20 @@ func (b *planBuilder) buildUnion(union *ast.UnionStmt) (LogicalPlan, error) { return nil, errors.Trace(err) } } + + // Fix issue #8189 (https://github.com/pingcap/tidb/issues/8189). + // If there are extra expressions generated from `ORDER BY` clause, generate a `Projection` to remove them. + if oldLen != unionPlan.Schema().Len() { + proj := LogicalProjection{Exprs: expression.Column2Exprs(unionPlan.Schema().Columns[:oldLen])}.init(b.ctx) + proj.SetChildren(unionPlan) + schema := expression.NewSchema(unionPlan.Schema().Clone().Columns[:oldLen]...) + for _, col := range schema.Columns { + col.UniqueID = b.ctx.GetSessionVars().AllocPlanColumnID() + } + proj.SetSchema(schema) + return proj, nil + } + return unionPlan, nil } @@ -860,7 +876,7 @@ func (b *planBuilder) buildLimit(src LogicalPlan, limit *ast.Limit) (LogicalPlan return li, nil } -// colMatch(a,b) means that if a match b, e.g. t.a can match test.t.a but test.t.a can't match t.a. +// colMatch means that if a match b, e.g. t.a can match test.t.a but test.t.a can't match t.a. // Because column a want column from database test exactly. func colMatch(a *ast.ColumnName, b *ast.ColumnName) bool { if a.Schema.L == "" || a.Schema.L == b.Schema.L { @@ -917,7 +933,7 @@ func resolveFromSelectFields(v *ast.ColumnNameExpr, fields []*ast.SelectField, i return } -// AggregateFuncExtractor visits Expr tree. +// havingAndOrderbyExprResolver visits Expr tree. // It converts ColunmNameExpr to AggregateFuncExpr and collects AggregateFuncExpr. type havingAndOrderbyExprResolver struct { inAggFunc bool diff --git a/planner/core/logical_plan_test.go b/planner/core/logical_plan_test.go index 8caf8dfeec1ec..91d093b7fa79b 100644 --- a/planner/core/logical_plan_test.go +++ b/planner/core/logical_plan_test.go @@ -1816,6 +1816,11 @@ func (s *testPlanSuite) TestUnion(c *C) { best: "UnionAll{UnionAll{Dual->Projection->Projection->Dual->Projection->Projection}->Aggr(firstrow(a))->Projection->Dual->Projection->Projection}->Projection->Sort", err: false, }, + { + sql: "select * from (select 1 as a union select 1 union all select 2) t order by (select a)", + best: "Apply{UnionAll{UnionAll{Dual->Projection->Projection->Dual->Projection->Projection}->Aggr(firstrow(a))->Projection->Dual->Projection->Projection}->Dual->Projection->MaxOneRow}->Sort->Projection", + err: false, + }, } for i, tt := range tests { comment := Commentf("case:%v sql:%s", i, tt.sql)