diff --git a/src/executor/engine/operator/orderby_operator.go b/src/executor/engine/operator/orderby_operator.go index 8ef4dd18..01ae8cb3 100644 --- a/src/executor/engine/operator/orderby_operator.go +++ b/src/executor/engine/operator/orderby_operator.go @@ -77,6 +77,6 @@ func (operator *OrderByOperator) Execute(ctx *xcontext.ResultContext) error { } return true }) - + rs.RemoveColumns(plan.RemovedIdxs...) return err } diff --git a/src/executor/engine/operator/orderby_operator_test.go b/src/executor/engine/operator/orderby_operator_test.go index 8589f4ad..01e760ee 100644 --- a/src/executor/engine/operator/orderby_operator_test.go +++ b/src/executor/engine/operator/orderby_operator_test.go @@ -80,9 +80,11 @@ func TestOrderByOperator(t *testing.T) { querys := []string{ "select id, name from A where id>8 order by id desc, name asc", + "select id from A where id>8 order by id desc, name asc", } results := []string{ "[[51 lang] [5 g] [5 g] [3 go] [3 z] [1 x]]", + "[[51] [5] [5] [3] [3] [1]]", } for i, query := range querys { diff --git a/src/planner/builder/builder_test.go b/src/planner/builder/builder_test.go index 44bf350e..4752c062 100644 --- a/src/planner/builder/builder_test.go +++ b/src/planner/builder/builder_test.go @@ -488,7 +488,6 @@ func TestSelectUnsupported(t *testing.T) { "select * from A as A1 where id in (select id from B)", "select distinct(b) from A", "select * from A join B on B.id=A.id", - "select id from A order by b", "select id from A limit x", "select age,count(*) from A group by age having count(*) >=2", "select * from A where B.a >1", @@ -517,7 +516,6 @@ func TestSelectUnsupported(t *testing.T) { "select A.id from G, A left join B on A.id=B.id where abs(B.a) > G.a", "select A.id from (G, A left join B on A.id=B.id),C where abs(B.a) > G.a", "select A.id from C,(G, A left join B on A.id=B.id) where abs(B.a+B.b) > G.a", - "select A.a as b from A order by A.b", "select a+1 from A order by a+1", "select b as a from A group by A.a", "select a+1 from A group by a+1", @@ -530,7 +528,6 @@ func TestSelectUnsupported(t *testing.T) { "unsupported: subqueries.in.select", "unsupported: distinct", "unsupported: '*'.expression.in.cross-shard.query", - "unsupported: orderby[b].should.in.select.list", "unsupported: limit.offset.or.counts.must.be.IntVal", "unsupported: expr[count(*)].in.having.clause", "unsupported: unknown.column.'B.a'.in.clause", @@ -559,7 +556,6 @@ func TestSelectUnsupported(t *testing.T) { "unsupported: expr.'abs(B.a)'.in.cross-shard.left.join", "unsupported: expr.'abs(B.a)'.in.cross-shard.left.join", "unsupported: expr.'abs(B.a + B.b)'.in.cross-shard.left.join", - "unsupported: orderby[A.b].should.in.select.list", "unsupported: orderby:[a + 1].type.should.be.colname", "unsupported: group.by.field[A.a].should.be.in.select.list", "unsupported: group.by.[a + 1].type.should.be.colname", @@ -1053,13 +1049,15 @@ func TestUnionUnsupported(t *testing.T) { querys := []string{ "select a from A union select a,b from B", "select a from A union select b from B order by b", + "select a from A union select b from B order by A.a", "select a from A union select b from B order by a limit x", "select a from C union select b from A limit 1", "select a from A union select b from C", } results := []string{ "unsupported: the.used.'select'.statements.have.a.different.number.of.columns", - "unsupported: orderby[b].should.in.select.list", + "unsupported: unknown.column.'b'.in.'order.clause'", + "unsupported: table.'A'.from.one.of.the.SELECTs.cannot.be.used.in.field.list", "unsupported: limit.offset.or.counts.must.be.IntVal", "Table 'C' doesn't exist (errno 1146) (sqlstate 42S02)", "Table 'C' doesn't exist (errno 1146) (sqlstate 42S02)", diff --git a/src/planner/builder/join_node.go b/src/planner/builder/join_node.go index cb743405..33d29c78 100644 --- a/src/planner/builder/join_node.go +++ b/src/planner/builder/join_node.go @@ -793,7 +793,7 @@ func (j *JoinNode) pushHaving(having exprInfo) error { // pushOrderBy used to push the order by exprs. func (j *JoinNode) pushOrderBy(orderBy sqlparser.OrderBy) error { - orderPlan := NewOrderByPlan(j.log, orderBy, j.fields, j.referTables) + orderPlan := NewOrderByPlan(j.log, orderBy, j) j.children = append(j.children, orderPlan) return orderPlan.Build() } diff --git a/src/planner/builder/merge_node.go b/src/planner/builder/merge_node.go index e0289dfe..24c999c2 100644 --- a/src/planner/builder/merge_node.go +++ b/src/planner/builder/merge_node.go @@ -192,7 +192,7 @@ func (m *MergeNode) pushSelectExprs(fields, groups []selectTuple, sel *sqlparser } if len(groups) == 0 { if len(node.OrderBy) > 0 { - orderPlan := NewOrderByPlan(m.log, node.OrderBy, m.fields, m.referTables) + orderPlan := NewOrderByPlan(m.log, node.OrderBy, m) if err := orderPlan.Build(); err != nil { return err } @@ -236,7 +236,7 @@ func (m *MergeNode) pushHaving(having exprInfo) error { // pushOrderBy used to push the order by exprs. func (m *MergeNode) pushOrderBy(orderBy sqlparser.OrderBy) error { m.Sel.(*sqlparser.Select).OrderBy = orderBy - orderPlan := NewOrderByPlan(m.log, orderBy, m.fields, m.referTables) + orderPlan := NewOrderByPlan(m.log, orderBy, m) m.children = append(m.children, orderPlan) return orderPlan.Build() } diff --git a/src/planner/builder/orderby_plan.go b/src/planner/builder/orderby_plan.go index 23a0900e..bf208e48 100644 --- a/src/planner/builder/orderby_plan.go +++ b/src/planner/builder/orderby_plan.go @@ -10,7 +10,6 @@ package builder import ( "encoding/json" - "fmt" "github.com/pkg/errors" "github.com/xelabs/go-mysqlstack/sqlparser" @@ -41,32 +40,28 @@ type OrderBy struct { // OrderByPlan represents order-by plan. type OrderByPlan struct { - log *xlog.Log - node sqlparser.OrderBy - tuples []selectTuple - tbInfos map[string]*tableInfo - OrderBys []OrderBy `json:"OrderBy(s)"` - typ ChildType + log *xlog.Log + node sqlparser.OrderBy + root PlanNode + // The indexes mark the fields to be removed. + RemovedIdxs []int + OrderBys []OrderBy `json:"OrderBy(s)"` + typ ChildType } // NewOrderByPlan used to create OrderByPlan. -func NewOrderByPlan(log *xlog.Log, node sqlparser.OrderBy, tuples []selectTuple, tbInfos map[string]*tableInfo) *OrderByPlan { +func NewOrderByPlan(log *xlog.Log, node sqlparser.OrderBy, root PlanNode) *OrderByPlan { return &OrderByPlan{ - log: log, - node: node, - tuples: tuples, - tbInfos: tbInfos, - typ: ChildTypeOrderby, + log: log, + node: node, + root: root, + typ: ChildTypeOrderby, } } // analyze used to check the 'order by' is at the support level. -// Supports: -// 1. sqlparser.ColName: 'select a from t order by a' -// -// Unsupported(orderby field must be in select list): -// 1. 'select a from t order by b' func (p *OrderByPlan) analyze() error { + tbInfos := p.root.getReferTables() for _, o := range p.node { switch e := o.Expr.(type) { case *sqlparser.ColName: @@ -80,18 +75,38 @@ func (p *OrderByPlan) analyze() error { orderBy.Field = e.Name.String() orderBy.Table = e.Qualifier.Name.String() if orderBy.Table != "" { - if _, ok := p.tbInfos[orderBy.Table]; !ok { + if _, ok := p.root.(*UnionNode); ok { + return errors.Errorf("unsupported: table.'%s'.from.one.of.the.SELECTs.cannot.be.used.in.field.list", orderBy.Table) + } + if _, ok := tbInfos[orderBy.Table]; !ok { return errors.Errorf("unsupported: unknow.table.in.order.by.field[%s.%s]", orderBy.Table, orderBy.Field) } } - ok, tuple := checkInTuple(orderBy.Field, orderBy.Table, p.tuples) + ok, tuple := checkInTuple(orderBy.Field, orderBy.Table, p.root.getFields()) if !ok { - field := orderBy.Field - if orderBy.Table != "" { - field = fmt.Sprintf("%s.%s", orderBy.Table, orderBy.Field) + if _, ok := p.root.(*UnionNode); ok { + return errors.Errorf("unsupported: unknown.column.'%s'.in.'order.clause'", orderBy.Field) + } + + tablename := orderBy.Table + if tablename == "" { + if len(tbInfos) == 1 { + tablename, _ = getOneTableInfo(tbInfos) + } else { + return errors.Errorf("unsupported: column.'%s'.in.order.clause.is.ambiguous", orderBy.Field) + } + } + // If `orderby.field` not exists in the field list, + // we need push it into field list and record in RemovedIdxs. + tuple = &selectTuple{ + expr: &sqlparser.AliasedExpr{Expr: e}, + info: exprInfo{e, []string{tablename}, []*sqlparser.ColName{e}, nil}, + field: orderBy.Field, + isCol: true, } - return errors.Errorf("unsupported: orderby[%s].should.in.select.list", field) + index, _ := p.root.pushSelectExpr(*tuple) + p.RemovedIdxs = append(p.RemovedIdxs, index) } if tuple.field != "*" { diff --git a/src/planner/builder/orderby_plan_test.go b/src/planner/builder/orderby_plan_test.go index 87472c03..c6bb8a8c 100644 --- a/src/planner/builder/orderby_plan_test.go +++ b/src/planner/builder/orderby_plan_test.go @@ -31,6 +31,7 @@ func TestOrderByPlan(t *testing.T) { "select A.a from A order by a", "select a as b from A order by a", "select a as b from A order by A.a", + "select a,b from A order by c", } log := xlog.NewStdLog(xlog.Level(xlog.PANIC)) @@ -44,9 +45,9 @@ func TestOrderByPlan(t *testing.T) { node := tree.(*sqlparser.Select) p, err := scanTableExprs(log, route, "sbtest", node.From) assert.Nil(t, err) - tuples, _, err := parseSelectExprs(node.SelectExprs, p) + _, _, err = parseSelectExprs(node.SelectExprs, p) assert.Nil(t, err) - plan := NewOrderByPlan(log, node.OrderBy, tuples, p.getReferTables()) + plan := NewOrderByPlan(log, node.OrderBy, p) // plan build { err := plan.Build() @@ -59,20 +60,20 @@ func TestOrderByPlan(t *testing.T) { func TestOrderByPlanError(t *testing.T) { querys := []string{ - "select a,b from A order by c", "select a,b from A order by rand()", "select A.* from A order by X.a", + "select A.a from A join B on A.id=B.id order by b", } results := []string{ - "unsupported: orderby[c].should.in.select.list", "unsupported: orderby:[rand()].type.should.be.colname", "unsupported: unknow.table.in.order.by.field[X.a]", + "unsupported: column.'b'.in.order.clause.is.ambiguous", } log := xlog.NewStdLog(xlog.Level(xlog.PANIC)) route, cleanup := router.MockNewRouter(log) defer cleanup() - err := route.AddForTest("sbtest", router.MockTableMConfig()) + err := route.AddForTest("sbtest", router.MockTableMConfig(), router.MockTableBConfig()) assert.Nil(t, err) for i, query := range querys { tree, err := sqlparser.Parse(query) @@ -80,9 +81,9 @@ func TestOrderByPlanError(t *testing.T) { node := tree.(*sqlparser.Select) p, err := scanTableExprs(log, route, "sbtest", node.From) assert.Nil(t, err) - tuples, _, err := parseSelectExprs(node.SelectExprs, p) + _, _, err = parseSelectExprs(node.SelectExprs, p) assert.Nil(t, err) - plan := NewOrderByPlan(log, node.OrderBy, tuples, p.getReferTables()) + plan := NewOrderByPlan(log, node.OrderBy, p) // plan build { err := plan.Build() diff --git a/src/planner/builder/plan_node_test.go b/src/planner/builder/plan_node_test.go index 92110e5a..af490ec1 100644 --- a/src/planner/builder/plan_node_test.go +++ b/src/planner/builder/plan_node_test.go @@ -58,13 +58,11 @@ func TestPushOrderBy(t *testing.T) { func TestPushOrderByError(t *testing.T) { querys := []string{ - "select a from A order by b", - "select A.a from A join B on A.id=B.id order by B.b", + "select A.a from A join B on A.id=B.id order by b", "select A.a from A join B on A.id=B.id order by C.a", } wants := []string{ - "unsupported: orderby[b].should.in.select.list", - "unsupported: orderby[B.b].should.in.select.list", + "unsupported: column.'b'.in.order.clause.is.ambiguous", "unsupported: unknow.table.in.order.by.field[C.a]", } log := xlog.NewStdLog(xlog.Level(xlog.PANIC)) diff --git a/src/planner/builder/union_node.go b/src/planner/builder/union_node.go index d81bfc74..bfd84613 100644 --- a/src/planner/builder/union_node.go +++ b/src/planner/builder/union_node.go @@ -77,7 +77,7 @@ func (u *UnionNode) getFields() []selectTuple { // pushOrderBy used to push the order by exprs. func (u *UnionNode) pushOrderBy(orderBy sqlparser.OrderBy) error { - orderPlan := NewOrderByPlan(u.log, orderBy, u.getFields(), u.referTables) + orderPlan := NewOrderByPlan(u.log, orderBy, u) u.children = append(u.children, orderPlan) return orderPlan.Build() }