dev row constructor

expression/expressions/between.go

@@ -20,6 +20,7 @@ package expressions
 import (
 	"fmt"
 
+	"github.com/juju/errors"
 	"github.com/pingcap/tidb/context"
 	"github.com/pingcap/tidb/expression"
 	"github.com/pingcap/tidb/parser/opcode"
@@ -75,19 +76,23 @@ func (b *Between) String() string {
 
 // Eval implements the Expression Eval interface.
 func (b *Between) Eval(ctx context.Context, args map[interface{}]interface{}) (interface{}, error) {
+	if err := CheckAllOneColumns(b.Expr, b.Left, b.Right); err != nil {
+		return nil, errors.Trace(err)
+	}
+
 	v, err := b.Expr.Eval(ctx, args)
 	if err != nil {
-		return nil, err
+		return nil, errors.Trace(err)
 	}
 
 	lv, err := b.Left.Eval(ctx, args)
 	if err != nil {
-		return nil, err
+		return nil, errors.Trace(err)
 	}
 
 	rv, err := b.Right.Eval(ctx, args)
 	if err != nil {
-		return nil, err
+		return nil, errors.Trace(err)
 	}
 
 	var l, r expression.Expression
@@ -148,19 +153,23 @@ func (b *Between) convert() expression.Expression {
 //	a between 10 and 15 -> a >= 10 && a <= 15
 //	a not between 10 and 15 -> a < 10 || b > 15
 func NewBetween(expr, lo, hi expression.Expression, not bool) (expression.Expression, error) {
+	if err := CheckAllOneColumns(expr, lo, hi); err != nil {
+		return nil, errors.Trace(err)
+	}
+
 	e, err := staticExpr(expr)
 	if err != nil {
-		return nil, err
+		return nil, errors.Trace(err)
 	}
 
 	l, err := staticExpr(lo)
 	if err != nil {
-		return nil, err
+		return nil, errors.Trace(err)
 	}
 
 	h, err := staticExpr(hi)
 	if err != nil {
-		return nil, err
+		return nil, errors.Trace(err)
 	}
 
 	b := &Between{Expr: e, Left: l, Right: h, Not: not}

expression/expressions/between_test.go

@@ -93,4 +93,23 @@ func (s *testBetweenSuite) TestBetween(c *C) {
 		_, err = b.Eval(nil, nil)
 		c.Assert(err, NotNil)
 	}
+
+	tableError = []struct {
+		Expr  expression.Expression
+		Left  expression.Expression
+		Right expression.Expression
+	}{
+		{newTestRow(1, 2), f(false), f(false)},
+		{f(false), newTestRow(1, 2), f(false)},
+		{f(false), f(false), newTestRow(1, 2)},
+	}
+
+	for _, t := range tableError {
+		_, err := NewBetween(t.Expr, t.Left, t.Right, false)
+		c.Assert(err, NotNil)
+		b := Between{Expr: t.Expr, Left: t.Left, Right: t.Right, Not: false}
+
+		_, err = b.Eval(nil, nil)
+		c.Assert(err, NotNil)
+	}
 }

expression/expressions/binop.go

@@ -147,6 +147,20 @@ func (o *BinaryOperation) Eval(ctx context.Context, args map[interface{}]interfa
 		}
 	}()
 
+	// all operands must have same column.
+	if err := hasSameColumn(o.L, o.R); err != nil {
+		return nil, o.traceErr(err)
+	}
+
+	// row constructor only supports comparison operation.
+	switch o.Op {
+	case opcode.LT, opcode.LE, opcode.GE, opcode.GT, opcode.EQ, opcode.NE:
+	default:
+		if err := CheckOneColumn(o.L); err != nil {
+			return nil, o.traceErr(err)
+		}
+	}
+
 	switch o.Op {
 	case opcode.AndAnd, opcode.OrOr, opcode.LogicXor:
 		return o.evalLogicOp(ctx, args)

expression/expressions/binop_test.go

@@ -526,4 +526,19 @@ func (s *testBinOpSuite) TestNumericOp(c *C) {
 	expr.Op = 0
 	_, err = expr.Eval(nil, nil)
 	c.Assert(err, NotNil)
+
+	expr.L = newTestRow(1, 2)
+	expr.R = Value{nil}
+	expr.Op = opcode.Plus
+
+	_, err = expr.Eval(nil, nil)
+	c.Assert(err, NotNil)
+
+	expr.Op = opcode.LE
+	_, err = expr.Eval(nil, nil)
+	c.Assert(err, NotNil)
+
+	expr.R = newTestRow(1, 2)
+	expr.Op = opcode.Plus
+	c.Assert(err, NotNil)
 }

expression/expressions/helper.go

@@ -222,6 +222,10 @@ func mentionedAggregateFuncs(e expression.Expression, m *[]expression.Expression
 		mentionedAggregateFuncs(x.Expr, m)
 		mentionedAggregateFuncs(x.Left, m)
 		mentionedAggregateFuncs(x.Right, m)
+	case *Row:
+		for _, expr := range x.Values {
+			mentionedAggregateFuncs(expr, m)
+		}
 	default:
 		log.Errorf("Unknown Expression: %T", e)
 	}
@@ -306,6 +310,10 @@ func mentionedColumns(e expression.Expression, m map[string]bool, names *[]strin
 		mentionedColumns(x.Expr, m, names)
 		mentionedColumns(x.Left, m, names)
 		mentionedColumns(x.Right, m, names)
+	case *Row:
+		for _, expr := range x.Values {
+			mentionedColumns(expr, m, names)
+		}
 	default:
 		log.Errorf("Unknown Expression: %T", e)
 	}
@@ -467,3 +475,45 @@ func EvalBoolExpr(ctx context.Context, expr expression.Expression, m map[interfa
 
 	return x != 0, nil
 }
+
+// CheckOneColumn checks whether expression e has only one column for the evaluation result.
+// Now most of the expressions have one column except Row expression.
+func CheckOneColumn(e expression.Expression) error {
+	_, ok := e.(*Row)
+	if ok {
+		return errors.Errorf("Operand should contain 1 column(s)")
+	}
+
+	return nil
+}
+
+// CheckAllOneColumns checks all expressions have one column.
+func CheckAllOneColumns(args ...expression.Expression) error {
+	for _, e := range args {
+		if err := CheckOneColumn(e); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func hasSameColumn(lhs expression.Expression, rhs expression.Expression) error {
+	l, okl := lhs.(*Row)
+	r, okr := rhs.(*Row)
+	if okl && okr {
+		if len(l.Values) != len(r.Values) {
+			return errors.Errorf("Operand should contain %d column(s)", len(l.Values))
+		}
+		return nil
+	} else if !okl && !okr {
+		// lhs and rhs are not Row
+		return nil
+	} else if okl {
+		// lhs is row, rhs is not
+		return errors.Errorf("Operand should contain %d column(s)", len(l.Values))
+	}
+
+	// here mean rhs is row, lhs is not
+	return errors.Errorf("Operand should contain 1 column(s)")
+}

expression/expressions/helper_test.go

@@ -40,6 +40,7 @@ func (s *testHelperSuite) TestContainAggFunc(c *C) {
 		{&WhenClause{Expr: v, Result: v}, false},
 		{&IsTruth{Expr: v}, false},
 		{&Between{Expr: v, Left: v, Right: v}, false},
+		{&Row{Values: []expression.Expression{v, v}}, false},
 	}
 
 	for _, t := range tbl {
@@ -71,6 +72,7 @@ func (s *testHelperSuite) TestMentionedColumns(c *C) {
 		{&WhenClause{Expr: v, Result: v}, 0},
 		{&IsTruth{Expr: v}, 0},
 		{&Between{Expr: v, Left: v, Right: v}, 0},
+		{&Row{Values: []expression.Expression{v, v}}, 0},
 	}
 
 	for _, t := range tbl {
@@ -79,6 +81,16 @@ func (s *testHelperSuite) TestMentionedColumns(c *C) {
 	}
 }
 
+func newTestRow(v1 interface{}, v2 interface{}, args ...interface{}) *Row {
+	r := &Row{}
+	a := make([]expression.Expression, len(args))
+	for i := range a {
+		a[i] = Value{args[i]}
+	}
+	r.Values = append([]expression.Expression{Value{v1}, Value{v2}}, a...)
+	return r
+}
+
 func (s *testHelperSuite) TestBase(c *C) {
 	e1 := Value{1}
 	e2 := &PExpr{Expr: e1}
@@ -114,6 +126,31 @@ func (s *testHelperSuite) TestBase(c *C) {
 
 	v, err = EvalBoolExpr(nil, mockExpr{err: errors.New("must error")}, nil)
 	c.Assert(err, NotNil)
+
+	err = CheckOneColumn(&Row{})
+	c.Assert(err, NotNil)
+
+	err = CheckOneColumn(Value{nil})
+	c.Assert(err, IsNil)
+
+	columns := []struct {
+		lhs     expression.Expression
+		rhs     expression.Expression
+		checker Checker
+	}{
+		{Value{nil}, Value{nil}, IsNil},
+		{Value{nil}, &Row{}, NotNil},
+		{newTestRow(1, 2), newTestRow(1, 2), IsNil},
+		{newTestRow(1, 2, 3), newTestRow(1, 2), NotNil},
+	}
+
+	for _, t := range columns {
+		err = hasSameColumn(t.lhs, t.rhs)
+		c.Assert(err, t.checker)
+
+		err = hasSameColumn(t.rhs, t.lhs)
+		c.Assert(err, t.checker)
+	}
 }
 
 func (s *testHelperSuite) TestGetTimeValue(c *C) {

expression/expressions/isnull.go

@@ -16,6 +16,7 @@ package expressions
 import (
 	"fmt"
 
+	"github.com/juju/errors"
 	"github.com/pingcap/tidb/context"
 	"github.com/pingcap/tidb/expression"
 )
@@ -58,10 +59,14 @@ func (is *IsNull) String() string {
 
 // Eval implements the Expression Eval interface.
 func (is *IsNull) Eval(ctx context.Context, args map[interface{}]interface{}) (v interface{}, err error) {
-	Val, err := is.Expr.Eval(ctx, args)
+	if err := CheckOneColumn(is.Expr); err != nil {
+		return nil, errors.Trace(err)
+	}
+
+	val, err := is.Expr.Eval(ctx, args)
 	if err != nil {
 		return
 	}
 
-	return Val == nil != is.Not, nil
+	return val == nil != is.Not, nil
 }

expression/expressions/isnull_test.go

@@ -14,6 +14,8 @@
 package expressions
 
 import (
+	"errors"
+
 	. "github.com/pingcap/check"
 )
 
@@ -50,4 +52,19 @@ func (t *testIsNullSuite) TestIsNull(c *C) {
 
 	str = e2.String()
 	c.Assert(len(str), Greater, 0)
+
+	// check error
+	expr := mockExpr{}
+	expr.err = errors.New("must error")
+	e.Expr = expr
+
+	_, err = e.Clone()
+	c.Assert(err, NotNil)
+
+	_, err = e.Eval(nil, nil)
+	c.Assert(err, NotNil)
+
+	e.Expr = newTestRow(1, 2)
+	_, err = e.Eval(nil, nil)
+	c.Assert(err, NotNil)
 }

expression/expressions/istruth.go

@@ -16,6 +16,7 @@ package expressions
 import (
 	"fmt"
 
+	"github.com/juju/errors"
 	"github.com/pingcap/tidb/context"
 	"github.com/pingcap/tidb/expression"
 	"github.com/pingcap/tidb/util/types"
@@ -67,6 +68,10 @@ func (is *IsTruth) String() string {
 
 // Eval implements the Expression Eval interface.
 func (is *IsTruth) Eval(ctx context.Context, args map[interface{}]interface{}) (v interface{}, err error) {
+	if err := CheckOneColumn(is.Expr); err != nil {
+		return nil, errors.Trace(err)
+	}
+
 	val, err := is.Expr.Eval(ctx, args)
 	if err != nil {
 		return

expression/expressions/istruth_test.go

@@ -56,4 +56,9 @@ func (t *testIsTruthSuite) TestIsTruth(c *C) {
 	vvv, err := e2.Eval(nil, nil)
 	c.Assert(err, IsNil)
 	c.Assert(vvv, IsTrue)
+
+	// check error
+	e.Expr = newTestRow(1, 2)
+	_, err = e.Eval(nil, nil)
+	c.Assert(err, NotNil)
 }