Skip to content

Commit

Permalink
*: check column in select uniformly
Browse files Browse the repository at this point in the history
  • Loading branch information
siddontang committed Sep 11, 2015
1 parent ca163c3 commit e37c62b
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 32 deletions.
4 changes: 0 additions & 4 deletions plan/plans/select_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,6 @@ func ResolveSelectList(selectFields []*field.Field, srcFields []*field.ResultFie

wildcardNum := 0
for _, v := range selectFields {
if err := expressions.CheckOneColumn(v.Expr); err != nil {
return nil, errors.Trace(err)
}

// Check metioned field.
names := expressions.MentionedColumns(v.Expr)
if len(names) == 0 {
Expand Down
4 changes: 0 additions & 4 deletions rset/rsets/groupby.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ func (r *GroupByRset) Plan(ctx context.Context) (plan.Plan, error) {
aggFields := r.SelectList.AggFields

for i, e := range r.By {
if err := expressions.CheckOneColumn(e); err != nil {
return nil, errors.Trace(err)
}

if v, ok := e.(expressions.Value); ok {
var position int
switch u := v.Val.(type) {
Expand Down
4 changes: 0 additions & 4 deletions rset/rsets/groupby_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,6 @@ func (s *testGroupByRsetSuite) TestGroupByRsetPlan(c *C) {

_, err = s.r.Plan(nil)
c.Assert(err, NotNil)

s.r.By[0] = &expressions.Row{Values: []expression.Expression{expressions.Value{Val: 1}, expressions.Value{Val: 1}}}
_, err = s.r.Plan(nil)
c.Assert(err, NotNil)
}

func (s *testGroupByRsetSuite) TestGroupByHasAmbiguousField(c *C) {
Expand Down
4 changes: 0 additions & 4 deletions rset/rsets/having.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ type HavingRset struct {

// CheckAndUpdateSelectList checks having fields validity and set hidden fields to selectList.
func (r *HavingRset) CheckAndUpdateSelectList(selectList *plans.SelectList, groupBy []expression.Expression, tableFields []*field.ResultField) error {
if err := expressions.CheckOneColumn(r.Expr); err != nil {
return errors.Trace(err)
}

if expressions.ContainAggregateFunc(r.Expr) {
expr, err := selectList.UpdateAggFields(r.Expr, tableFields)
if err != nil {
Expand Down
4 changes: 0 additions & 4 deletions rset/rsets/having_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,6 @@ func (s *testHavingRsetSuite) TestHavingRsetCheckAndUpdateSelectList(c *C) {

err = s.r.CheckAndUpdateSelectList(selectList, groupBy, resultFields)
c.Assert(err, NotNil)

s.r.Expr = &expressions.Row{Values: []expression.Expression{expressions.Value{Val: 1}, expressions.Value{Val: 1}}}
err = s.r.CheckAndUpdateSelectList(selectList, groupBy, resultFields)
c.Assert(err, NotNil)
}

func (s *testHavingRsetSuite) TestHavingRsetPlan(c *C) {
Expand Down
4 changes: 0 additions & 4 deletions rset/rsets/orderby.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,6 @@ func (r *OrderByRset) String() string {
// CheckAndUpdateSelectList checks order by fields validity and set hidden fields to selectList.
func (r *OrderByRset) CheckAndUpdateSelectList(selectList *plans.SelectList, tableFields []*field.ResultField) error {
for i, v := range r.By {
if err := expressions.CheckOneColumn(v.Expr); err != nil {
return errors.Trace(err)
}

if expressions.ContainAggregateFunc(v.Expr) {
expr, err := selectList.UpdateAggFields(v.Expr, tableFields)
if err != nil {
Expand Down
8 changes: 0 additions & 8 deletions rset/rsets/orderby_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,6 @@ func (s *testOrderByRsetSuite) TestOrderByRsetCheckAndUpdateSelectList(c *C) {

err = r.CheckAndUpdateSelectList(selectList, resultFields)
c.Assert(err, NotNil)

// `select id from t order by row(1, 1)`
r.By[0].Expr = &expressions.Row{Values: []expression.Expression{expressions.Value{Val: 1}, expressions.Value{Val: 1}}}
selectList.Fields[1].Name = "name"
selectList.ResultFields[1].Name = "name"

err = r.CheckAndUpdateSelectList(selectList, resultFields)
c.Assert(err, NotNil)
}

func (s *testOrderByRsetSuite) TestOrderByRsetPlan(c *C) {
Expand Down
41 changes: 41 additions & 0 deletions stmt/stmts/select.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/ngaut/log"
"github.com/pingcap/tidb/context"
"github.com/pingcap/tidb/expression"
"github.com/pingcap/tidb/expression/expressions"
"github.com/pingcap/tidb/field"
"github.com/pingcap/tidb/parser/coldef"
"github.com/pingcap/tidb/plan"
Expand Down Expand Up @@ -79,6 +80,42 @@ func (s *SelectStmt) SetText(text string) {
s.Text = text
}

func (s *SelectStmt) checkOneColumn(ctx context.Context) error {
// check select fields
for _, f := range s.Fields {
if err := expressions.CheckOneColumn(ctx, f.Expr); err != nil {
return errors.Trace(err)
}
}

// check group by
if s.GroupBy != nil {
for _, f := range s.GroupBy.By {
if err := expressions.CheckOneColumn(ctx, f); err != nil {
return errors.Trace(err)
}
}
}

// check order by
if s.OrderBy != nil {
for _, f := range s.OrderBy.By {
if err := expressions.CheckOneColumn(ctx, f.Expr); err != nil {
return errors.Trace(err)
}
}
}

// check having
if s.Having != nil {
if err := expressions.CheckOneColumn(ctx, s.Having.Expr); err != nil {
return errors.Trace(err)
}
}

return nil
}

// Plan implements the plan.Planner interface.
// The whole phase for select is
// `from -> where -> lock -> group by -> having -> select fields -> distinct -> order by -> limit -> final`
Expand Down Expand Up @@ -122,6 +159,10 @@ func (s *SelectStmt) Plan(ctx context.Context) (plan.Plan, error) {
return nil, err
}

if err := s.checkOneColumn(ctx); err != nil {
return nil, errors.Trace(err)
}

// Get select list for futher field values evaluation.
selectList, err := plans.ResolveSelectList(s.Fields, r.GetFields())
if err != nil {
Expand Down
12 changes: 12 additions & 0 deletions stmt/stmts/select_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,5 +140,17 @@ func (s *testStmtSuite) TestSelectErrorRow(c *C) {
_, err = tx.Query("select * from test having row(1, 1);")
c.Assert(err, NotNil)

_, err = tx.Query("select (select 1, 1) from test;")
c.Assert(err, NotNil)

_, err = tx.Query("select * from test group by (select 1, 1);")
c.Assert(err, NotNil)

_, err = tx.Query("select * from test order by (select 1, 1);")
c.Assert(err, NotNil)

_, err = tx.Query("select * from test having (select 1, 1);")
c.Assert(err, NotNil)

mustCommit(c, tx)
}
15 changes: 15 additions & 0 deletions tidb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,21 @@ func (s *testSessionSuite) TestRow(c *C) {
row, err = r.FirstRow()
c.Assert(err, IsNil)
match(c, row, 1)

r = mustExecSQL(c, se, "select row(1, 1) > (select 1, 0)")
row, err = r.FirstRow()
c.Assert(err, IsNil)
match(c, row, 1)

r = mustExecSQL(c, se, "select 1 > (select 1)")
row, err = r.FirstRow()
c.Assert(err, IsNil)
match(c, row, 0)

r = mustExecSQL(c, se, "select (select 1)")
row, err = r.FirstRow()
c.Assert(err, IsNil)
match(c, row, 1)
}

func newSession(c *C, store kv.Storage, dbName string) Session {
Expand Down

0 comments on commit e37c62b

Please sign in to comment.