Skip to content

Commit

Permalink
planner, executor: eliminate extra columns introduced by OrderBy upon…
Browse files Browse the repository at this point in the history
… Union (#8290) (#8306)
  • Loading branch information
AndrewDi authored and zz-jason committed Nov 14, 2018
1 parent a5afbaf commit 4ec85cd
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 2 deletions.
9 changes: 9 additions & 0 deletions executor/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
20 changes: 18 additions & 2 deletions planner/core/logical_plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions planner/core/logical_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 4ec85cd

Please sign in to comment.