Skip to content

Commit

Permalink
*: support orderby not in field list #539
Browse files Browse the repository at this point in the history
[summary]
If order by is not in field list, we need push the order by field into field list.
[test case]
src/executor/engine/operator/orderby_operator_test.go
src/planner/builder/builder_test.go
src/planner/builder/orderby_plan_test.go
src/planner/builder/plan_node_test.go
[patch codecov]
src/executor/engine/operator/orderby_operator.go 96.0%
src/planner/builder/orderby_plan.go 97.6%
  • Loading branch information
zhyass authored and BohuTANG committed Nov 29, 2019
1 parent 5ab0417 commit b689ebe
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 45 deletions.
2 changes: 1 addition & 1 deletion src/executor/engine/operator/orderby_operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,6 @@ func (operator *OrderByOperator) Execute(ctx *xcontext.ResultContext) error {
}
return true
})

rs.RemoveColumns(plan.RemovedIdxs...)
return err
}
2 changes: 2 additions & 0 deletions src/executor/engine/operator/orderby_operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 3 additions & 5 deletions src/planner/builder/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)",
Expand Down
2 changes: 1 addition & 1 deletion src/planner/builder/join_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
4 changes: 2 additions & 2 deletions src/planner/builder/merge_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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()
}
Expand Down
63 changes: 39 additions & 24 deletions src/planner/builder/orderby_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ package builder

import (
"encoding/json"
"fmt"

"github.com/pkg/errors"
"github.com/xelabs/go-mysqlstack/sqlparser"
Expand Down Expand Up @@ -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:
Expand All @@ -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 != "*" {
Expand Down
15 changes: 8 additions & 7 deletions src/planner/builder/orderby_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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()
Expand All @@ -59,30 +60,30 @@ 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)
assert.Nil(t, err)
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()
Expand Down
6 changes: 2 additions & 4 deletions src/planner/builder/plan_node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion src/planner/builder/union_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down

0 comments on commit b689ebe

Please sign in to comment.