From 3b68ee59a6576a3e89328ddd630e3c51c58a2ca4 Mon Sep 17 00:00:00 2001 From: Jun-Seok Heo Date: Thu, 29 Nov 2018 13:40:08 +0900 Subject: [PATCH] planner: for UNION statements, ORDER BY cannot use column references including a table name (#8389) --- cmd/explaintest/r/explain_easy.result | 4 ++-- executor/executor_test.go | 12 ++++++++++++ planner/core/errors.go | 2 ++ planner/core/expression_rewriter.go | 7 +++++++ planner/core/logical_plan_builder.go | 7 +++++-- planner/core/logical_plan_test.go | 10 +++++----- planner/core/physical_plan_test.go | 2 +- planner/core/planbuilder.go | 18 ++++++++++-------- 8 files changed, 44 insertions(+), 18 deletions(-) diff --git a/cmd/explaintest/r/explain_easy.result b/cmd/explaintest/r/explain_easy.result index e6f55b6dfdea3..1630f426f66ad 100644 --- a/cmd/explaintest/r/explain_easy.result +++ b/cmd/explaintest/r/explain_easy.result @@ -159,7 +159,7 @@ TableReader_5 10000.00 root data:TableScan_4 explain select c1 from t2 union select c1 from t2 union all select c1 from t2; id count task operator info Union_17 26000.00 root -├─HashAgg_21 16000.00 root group by:t2.c1, funcs:firstrow(join_agg_0) +├─HashAgg_21 16000.00 root group by:c1, funcs:firstrow(join_agg_0) │ └─Union_22 16000.00 root │ ├─StreamAgg_35 8000.00 root group by:col_2, funcs:firstrow(col_0), firstrow(col_1) │ │ └─IndexReader_36 8000.00 root index:StreamAgg_26 @@ -173,7 +173,7 @@ Union_17 26000.00 root └─TableScan_58 10000.00 cop table:t2, range:[-inf,+inf], keep order:false, stats:pseudo explain select c1 from t2 union all select c1 from t2 union select c1 from t2; id count task operator info -HashAgg_18 24000.00 root group by:t2.c1, funcs:firstrow(join_agg_0) +HashAgg_18 24000.00 root group by:c1, funcs:firstrow(join_agg_0) └─Union_19 24000.00 root ├─StreamAgg_32 8000.00 root group by:col_2, funcs:firstrow(col_0), firstrow(col_1) │ └─IndexReader_33 8000.00 root index:StreamAgg_23 diff --git a/executor/executor_test.go b/executor/executor_test.go index a5bfcf8988160..66c58f12411b0 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -1108,6 +1108,18 @@ func (s *testSuite) TestUnion(c *C) { tk.MustExec("CREATE TABLE t1 (uid int(1))") tk.MustExec("INSERT INTO t1 SELECT 150") tk.MustQuery("SELECT 'a' UNION SELECT uid FROM t1 order by 1 desc;").Check(testkit.Rows("a", "150")) + + // #issue 8196 + 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(3,'c'),(4,'d'),(5,'f'),(6,'e')") + tk.MustExec("analyze table t1") + tk.MustExec("analyze table t2") + _, err = tk.Exec("(select a,b from t1 limit 2) union all (select a,b from t2 order by a limit 1) order by t1.b") + c.Assert(err.Error(), Equals, "[planner:1250]Table 't1' from one of the SELECTs cannot be used in global ORDER clause") } func (s *testSuite) TestNeighbouringProj(c *C) { diff --git a/planner/core/errors.go b/planner/core/errors.go index 55f01919ddf1f..89c81fa47b086 100644 --- a/planner/core/errors.go +++ b/planner/core/errors.go @@ -49,6 +49,7 @@ const ( codeNonUniqTable = mysql.ErrNonuniqTable codeWrongNumberOfColumnsInSelect = mysql.ErrWrongNumberOfColumnsInSelect codeWrongValueCountOnRow = mysql.ErrWrongValueCountOnRow + codeTablenameNotAllowedHere = mysql.ErrTablenameNotAllowedHere ) // error definitions. @@ -59,6 +60,7 @@ var ( ErrStmtNotFound = terror.ClassOptimizer.New(codeStmtNotFound, "Prepared statement not found") ErrWrongParamCount = terror.ClassOptimizer.New(codeWrongParamCount, "Wrong parameter count") ErrSchemaChanged = terror.ClassOptimizer.New(codeSchemaChanged, "Schema has changed") + ErrTablenameNotAllowedHere = terror.ClassOptimizer.New(codeTablenameNotAllowedHere, "Table '%s' from one of the %ss cannot be used in %s") ErrWrongUsage = terror.ClassOptimizer.New(codeWrongUsage, mysql.MySQLErrName[mysql.ErrWrongUsage]) ErrAmbiguous = terror.ClassOptimizer.New(codeAmbiguous, mysql.MySQLErrName[mysql.ErrNonUniq]) diff --git a/planner/core/expression_rewriter.go b/planner/core/expression_rewriter.go index 80b1528e11a37..1da8e11ed81ce 100644 --- a/planner/core/expression_rewriter.go +++ b/planner/core/expression_rewriter.go @@ -1267,5 +1267,12 @@ func (er *expressionRewriter) toColumn(v *ast.ColumnName) { return } } + if _, ok := er.p.(*LogicalUnionAll); ok && v.Table.O != "" { + er.err = ErrTablenameNotAllowedHere.GenWithStackByArgs(v.Table.O, "SELECT", clauseMsg[er.b.curClause]) + return + } + if er.b.curClause == globalOrderByClause { + er.b.curClause = orderByClause + } er.err = ErrUnknownColumn.GenWithStackByArgs(v.String(), clauseMsg[er.b.curClause]) } diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 0b63efe58a945..fd5b5ffd58460 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -680,7 +680,6 @@ func (b *PlanBuilder) buildProjection4Union(u *LogicalUnionAll) { } unionCols = append(unionCols, &expression.Column{ ColName: col.ColName, - TblName: col.TblName, RetType: resultTp, UniqueID: b.ctx.GetSessionVars().AllocPlanColumnID(), }) @@ -820,7 +819,11 @@ func (by *ByItems) Clone() *ByItems { } func (b *PlanBuilder) buildSort(p LogicalPlan, byItems []*ast.ByItem, aggMapper map[*ast.AggregateFuncExpr]int) (*LogicalSort, error) { - b.curClause = orderByClause + if _, isUnion := p.(*LogicalUnionAll); isUnion { + b.curClause = globalOrderByClause + } else { + b.curClause = orderByClause + } sort := LogicalSort{}.Init(b.ctx) exprs := make([]*ByItems, 0, len(byItems)) for _, item := range byItems { diff --git a/planner/core/logical_plan_test.go b/planner/core/logical_plan_test.go index 939214e965bd6..ce66549e8658f 100644 --- a/planner/core/logical_plan_test.go +++ b/planner/core/logical_plan_test.go @@ -1799,7 +1799,7 @@ func (s *testPlanSuite) TestUnion(c *C) { }{ { sql: "select a from t union select a from t", - best: "UnionAll{DataScan(t)->Projection->DataScan(t)->Projection}->Aggr(firstrow(t.a))", + best: "UnionAll{DataScan(t)->Projection->DataScan(t)->Projection}->Aggr(firstrow(a))", err: false, }, { @@ -1809,12 +1809,12 @@ func (s *testPlanSuite) TestUnion(c *C) { }, { sql: "select a from t union select a from t union all select a from t", - best: "UnionAll{UnionAll{DataScan(t)->Projection->DataScan(t)->Projection}->Aggr(firstrow(t.a))->Projection->DataScan(t)->Projection}", + best: "UnionAll{UnionAll{DataScan(t)->Projection->DataScan(t)->Projection}->Aggr(firstrow(a))->Projection->DataScan(t)->Projection}", err: false, }, { sql: "select a from t union select a from t union all select a from t union select a from t union select a from t", - best: "UnionAll{DataScan(t)->Projection->DataScan(t)->Projection->DataScan(t)->Projection->DataScan(t)->Projection->DataScan(t)->Projection}->Aggr(firstrow(t.a))", + best: "UnionAll{DataScan(t)->Projection->DataScan(t)->Projection->DataScan(t)->Projection->DataScan(t)->Projection->DataScan(t)->Projection}->Aggr(firstrow(a))", err: false, }, { @@ -1942,12 +1942,12 @@ func (s *testPlanSuite) TestTopNPushDown(c *C) { // Test TopN + UA + Proj. { sql: "select * from t union all (select * from t s) order by a,b limit 5", - best: "UnionAll{DataScan(t)->TopN([test.t.a test.t.b],0,5)->Projection->DataScan(s)->TopN([s.a s.b],0,5)->Projection}->TopN([t.a t.b],0,5)", + best: "UnionAll{DataScan(t)->TopN([test.t.a test.t.b],0,5)->Projection->DataScan(s)->TopN([s.a s.b],0,5)->Projection}->TopN([a b],0,5)", }, // Test TopN + UA + Proj. { sql: "select * from t union all (select * from t s) order by a,b limit 5, 5", - best: "UnionAll{DataScan(t)->TopN([test.t.a test.t.b],0,10)->Projection->DataScan(s)->TopN([s.a s.b],0,10)->Projection}->TopN([t.a t.b],5,5)", + best: "UnionAll{DataScan(t)->TopN([test.t.a test.t.b],0,10)->Projection->DataScan(s)->TopN([s.a s.b],0,10)->Projection}->TopN([a b],5,5)", }, // Test Limit + UA + Proj + Sort. { diff --git a/planner/core/physical_plan_test.go b/planner/core/physical_plan_test.go index 12bf1c62e5b25..2290897dd8550 100644 --- a/planner/core/physical_plan_test.go +++ b/planner/core/physical_plan_test.go @@ -699,7 +699,7 @@ func (s *testPlanSuite) TestDAGPlanBuilderUnion(c *C) { // Test TopN + Union. { sql: "select a from t union all (select c from t) order by a limit 1", - best: "UnionAll{TableReader(Table(t)->Limit)->Limit->IndexReader(Index(t.c_d_e)[[NULL,+inf]]->Limit)->Limit}->TopN([t.a],0,1)", + best: "UnionAll{TableReader(Table(t)->Limit)->Limit->IndexReader(Index(t.c_d_e)[[NULL,+inf]]->Limit)->Limit}->TopN([a],0,1)", }, } for i, tt := range tests { diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index abf67bf9ff21c..e37532fe500f2 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -94,17 +94,19 @@ const ( whereClause groupByClause showStatement + globalOrderByClause ) var clauseMsg = map[clauseCode]string{ - unknowClause: "", - fieldList: "field list", - havingClause: "having clause", - onClause: "on clause", - orderByClause: "order clause", - whereClause: "where clause", - groupByClause: "group statement", - showStatement: "show statement", + unknowClause: "", + fieldList: "field list", + havingClause: "having clause", + onClause: "on clause", + orderByClause: "order clause", + whereClause: "where clause", + groupByClause: "group statement", + showStatement: "show statement", + globalOrderByClause: "global ORDER clause", } // PlanBuilder builds Plan from an ast.Node.