Skip to content

Commit

Permalink
planner: for UNION statements, ORDER BY cannot use column references …
Browse files Browse the repository at this point in the history
…including a table name (#8389) (#8514)
  • Loading branch information
dbjoa authored and zz-jason committed Dec 13, 2018
1 parent fe00792 commit 240fd12
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 15 deletions.
12 changes: 12 additions & 0 deletions executor/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,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) {
Expand Down
2 changes: 2 additions & 0 deletions plan/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const (
codeDupFieldName = mysql.ErrDupFieldName
codeNonUpdatableTable = mysql.ErrNonUpdatableTable
codeInternal = mysql.ErrInternal
codeTablenameNotAllowedHere = mysql.ErrTablenameNotAllowedHere
)

// error definitions.
Expand All @@ -56,6 +57,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])
Expand Down
7 changes: 7 additions & 0 deletions plan/expression_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -1202,5 +1202,12 @@ func (er *expressionRewriter) toColumn(v *ast.ColumnName) {
return
}
}
if _, ok := er.p.(*LogicalUnionAll); ok && v.Table.O != "" {
er.err = ErrTablenameNotAllowedHere.GenByArgs(v.Table.O, "SELECT", clauseMsg[er.b.curClause])
return
}
if er.b.curClause == globalOrderByClause {
er.b.curClause = orderByClause
}
er.err = ErrUnknownColumn.GenByArgs(v.String(), clauseMsg[er.b.curClause])
}
7 changes: 6 additions & 1 deletion plan/logical_plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,7 @@ func (b *planBuilder) buildProjection4Union(u *LogicalUnionAll) {
}
}
col.RetType = resultTp
col.TblName = model.NewCIStr("")
col.DBName = model.NewCIStr("")
}
// If the types of some child don't match the types of union, we add a projection with cast function.
Expand Down Expand Up @@ -810,7 +811,11 @@ func (by *ByItems) Clone() *ByItems {
}

func (b *planBuilder) buildSort(p LogicalPlan, byItems []*ast.ByItem, aggMapper map[*ast.AggregateFuncExpr]int) LogicalPlan {
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 {
Expand Down
10 changes: 5 additions & 5 deletions plan/logical_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1436,7 +1436,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,
},
{
Expand All @@ -1446,12 +1446,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,
},
{
Expand Down Expand Up @@ -1580,12 +1580,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.
{
Expand Down
2 changes: 1 addition & 1 deletion plan/physical_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,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)[[<nil>,+inf]]->Limit)->Limit}->TopN([t.a],0,1)",
best: "UnionAll{TableReader(Table(t)->Limit)->Limit->IndexReader(Index(t.c_d_e)[[<nil>,+inf]]->Limit)->Limit}->TopN([a],0,1)",
},
}
for i, tt := range tests {
Expand Down
18 changes: 10 additions & 8 deletions plan/planbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,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.
Expand Down

0 comments on commit 240fd12

Please sign in to comment.