From 137230d04f31414440e1b3f896cf11844fc3c86f Mon Sep 17 00:00:00 2001 From: AndrewDi Date: Tue, 13 Nov 2018 13:46:49 +0800 Subject: [PATCH 1/4] fix issue #8189 and issue #8199,eliminate extra columns introduced by OrderBy upon Union --- executor/executor_test.go | 9 +++++++++ planner/core/logical_plan_builder.go | 20 ++++++++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index f40cf0060b026..cab8f33fab653 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -1088,6 +1088,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 d67d4940966d1..a61d956494cf8 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -725,6 +725,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 { @@ -738,6 +740,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 } @@ -880,7 +896,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 { @@ -937,7 +953,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 From 172a5dead8fb6dbf5ca2f9ebfe32be06ba5669af Mon Sep 17 00:00:00 2001 From: AndrewDi Date: Tue, 13 Nov 2018 15:37:50 +0800 Subject: [PATCH 2/4] add logical_plan_test for union with orderby --- planner/core/logical_plan_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/planner/core/logical_plan_test.go b/planner/core/logical_plan_test.go index e65d9a8811348..2fce8b75ce703 100644 --- a/planner/core/logical_plan_test.go +++ b/planner/core/logical_plan_test.go @@ -1829,6 +1829,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: "UnionAll{UnionAll{Dual->Projection->Projection->Dual->Projection->Projection}->Aggr(firstrow(a))->Projection->Dual->Projection->Projection}->Projection->Sort->Projection", + err: false, + }, } for i, tt := range tests { comment := Commentf("case:%v sql:%s", i, tt.sql) From 54533300ac01b12fd159e6e7193d99f0efd4cc48 Mon Sep 17 00:00:00 2001 From: AndrewDi Date: Tue, 13 Nov 2018 15:39:34 +0800 Subject: [PATCH 3/4] fix ci --- executor/executor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index 5b60a68f29274..3565a68dbdecf 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -1097,7 +1097,7 @@ func (s *testSuite) TestUnion(c *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")) - + // #issue 8201 for i := 0; i < 4; i++ { tk.MustQuery("SELECT(SELECT 0 AS a FROM dual UNION SELECT 1 AS a FROM dual ORDER BY a ASC LIMIT 1) AS dev").Check(testkit.Rows("0")) From 4f16673f7bba794894453bb01f30bb222a6157fd Mon Sep 17 00:00:00 2001 From: AndrewDi Date: Tue, 13 Nov 2018 15:46:51 +0800 Subject: [PATCH 4/4] fix ci --- planner/core/logical_plan_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/logical_plan_test.go b/planner/core/logical_plan_test.go index 2fce8b75ce703..c76d55e7dc973 100644 --- a/planner/core/logical_plan_test.go +++ b/planner/core/logical_plan_test.go @@ -1831,7 +1831,7 @@ func (s *testPlanSuite) TestUnion(c *C) { }, { sql: "select * from (select 1 as a union select 1 union all select 2) t order by (select a)", - best: "UnionAll{UnionAll{Dual->Projection->Projection->Dual->Projection->Projection}->Aggr(firstrow(a))->Projection->Dual->Projection->Projection}->Projection->Sort->Projection", + best: "Apply{UnionAll{UnionAll{Dual->Projection->Projection->Dual->Projection->Projection}->Aggr(firstrow(a))->Projection->Dual->Projection->Projection}->Dual->Projection->MaxOneRow}->Sort->Projection", err: false, }, }