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) (#8515)
  • Loading branch information
dbjoa authored and shenli committed Nov 30, 2018
1 parent def415f commit 5b17f82
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 18 deletions.
4 changes: 2 additions & 2 deletions cmd/explaintest/r/explain_easy.result
Expand Up @@ -152,7 +152,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
Expand All @@ -166,7 +166,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
Expand Down
12 changes: 12 additions & 0 deletions executor/executor_test.go
Expand Up @@ -1071,6 +1071,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 planner/core/errors.go
Expand Up @@ -49,6 +49,7 @@ const (
codeNonUniqTable = mysql.ErrNonuniqTable
codeWrongNumberOfColumnsInSelect = mysql.ErrWrongNumberOfColumnsInSelect
codeWrongValueCountOnRow = mysql.ErrWrongValueCountOnRow
codeTablenameNotAllowedHere = mysql.ErrTablenameNotAllowedHere
)

// error definitions.
Expand All @@ -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])
Expand Down
7 changes: 7 additions & 0 deletions planner/core/expression_rewriter.go
Expand Up @@ -1254,5 +1254,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])
}
7 changes: 5 additions & 2 deletions planner/core/logical_plan_builder.go
Expand Up @@ -661,7 +661,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(),
})
Expand Down Expand Up @@ -801,7 +800,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 {
Expand Down
10 changes: 5 additions & 5 deletions planner/core/logical_plan_test.go
Expand Up @@ -1788,7 +1788,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 @@ -1798,12 +1798,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 @@ -1931,12 +1931,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 planner/core/physical_plan_test.go
Expand Up @@ -698,7 +698,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 {
Expand Down
18 changes: 10 additions & 8 deletions planner/core/planbuilder.go
Expand Up @@ -93,17 +93,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 5b17f82

Please sign in to comment.