expression, plan: fix the index selection's bug

expression/builtin_compare.go

@@ -18,6 +18,8 @@ import (
 	"sort"
 
 	"github.com/juju/errors"
+	"github.com/ngaut/log"
+	"github.com/pingcap/tidb/ast"
 	"github.com/pingcap/tidb/context"
 	"github.com/pingcap/tidb/mysql"
 	"github.com/pingcap/tidb/parser/opcode"
@@ -459,11 +461,83 @@ func isTemporalColumn(expr Expression) bool {
 	return true
 }
 
+func tryToConvertConstantInt(con *Constant, ctx context.Context) *Constant {
+	if con.GetTypeClass() == types.ClassInt {
+		return con
+	}
+	sc := ctx.GetSessionVars().StmtCtx
+	i64, err := con.Value.ToInt64(sc)
+	if err != nil {
+		return con
+	}
+	return &Constant{
+		Value:   types.NewIntDatum(i64),
+		RetType: types.NewFieldType(mysql.TypeLonglong),
+	}
+}
+
+// refineConstantArg changes the constant argument to it's ceiling or flooring result by the given op.
+func refineConstantArg(con *Constant, op opcode.Op, ctx context.Context) *Constant {
+	sc := ctx.GetSessionVars().StmtCtx
+	i64, err := con.Value.ToInt64(sc)
+	if err != nil {
+		log.Warnf("failed to refine this expression, the constant argument is %s", con)
+		return con
+	}
+	datumInt := types.NewIntDatum(i64)
+	c, err := datumInt.CompareDatum(sc, con.Value)
+	if err != nil {
+		log.Warnf("failed to refine this expression, the constant argument is %s", con)
+		return con
+	}
+	if c == 0 {
+		return &Constant{
+			Value:   datumInt,
+			RetType: types.NewFieldType(mysql.TypeLonglong),
+		}
+	}
+	switch op {
+	case opcode.LT, opcode.GE:
+		resultExpr, _ := NewFunction(ctx, ast.Ceil, types.NewFieldType(mysql.TypeUnspecified), con)
+		if resultCon, ok := resultExpr.(*Constant); ok {
+			return tryToConvertConstantInt(resultCon, ctx)
+		}
+	case opcode.LE, opcode.GT:
+		resultExpr, _ := NewFunction(ctx, ast.Floor, types.NewFieldType(mysql.TypeUnspecified), con)
+		if resultCon, ok := resultExpr.(*Constant); ok {
+			return tryToConvertConstantInt(resultCon, ctx)
+		}
+	}
+	// TODO: argInt = 1.1 should be false forever.
+	return con
+}
+
+// refineArgs rewrite the arguments if one of them is int expression and another one is non-int constant.
+// Like a < 1.1 will be changed to a < 2.
+func (c *compareFunctionClass) refineArgs(args []Expression, ctx context.Context) []Expression {
+	arg0IsInt := args[0].GetTypeClass() == types.ClassInt
+	arg1IsInt := args[1].GetTypeClass() == types.ClassInt
+	arg0, arg0IsCon := args[0].(*Constant)
+	arg1, arg1IsCon := args[1].(*Constant)
+	// int non-constant [cmp] non-int constant
+	if arg0IsInt && !arg0IsCon && !arg1IsInt && arg1IsCon {
+		arg1 = refineConstantArg(arg1, c.op, ctx)
+		return []Expression{args[0], arg1}
+	}
+	// non-int constant [cmp] int non-constant
+	if arg1IsInt && !arg1IsCon && !arg0IsInt && arg0IsCon {
+		arg0 = refineConstantArg(arg0, symmetricOp[c.op], ctx)
+		return []Expression{arg0, args[1]}
+	}
+	return args
+}
+
 // getFunction sets compare built-in function signatures for various types.
-func (c *compareFunctionClass) getFunction(args []Expression, ctx context.Context) (sig builtinFunc, err error) {
-	if err = c.verifyArgs(args); err != nil {
+func (c *compareFunctionClass) getFunction(rawArgs []Expression, ctx context.Context) (sig builtinFunc, err error) {
+	if err = c.verifyArgs(rawArgs); err != nil {
 		return nil, errors.Trace(err)
 	}
+	args := c.refineArgs(rawArgs, ctx)
 	ft0, ft1 := args[0].GetType(), args[1].GetType()
 	tc0, tc1 := ft0.ToClass(), ft1.ToClass()
 	cmpType := getCmpType(tc0, tc1)

expression/builtin_compare_test.go

@@ -18,12 +18,74 @@ import (
 
 	. "github.com/pingcap/check"
 	"github.com/pingcap/tidb/ast"
+	"github.com/pingcap/tidb/model"
 	"github.com/pingcap/tidb/mysql"
 	"github.com/pingcap/tidb/util/testleak"
 	"github.com/pingcap/tidb/util/types"
 	"github.com/pingcap/tidb/util/types/json"
 )
 
+func (s *testEvaluatorSuite) TestCompareFunctionWithRefine(c *C) {
+	defer testleak.AfterTest(c)()
+
+	colInt := &Column{
+		ColName: model.NewCIStr("a"),
+		RetType: types.NewFieldType(mysql.TypeLong),
+	}
+
+	conOne := &Constant{
+		Value:   types.NewFloat64Datum(1.0),
+		RetType: types.NewFieldType(mysql.TypeDouble),
+	}
+
+	conOnePointOne := &Constant{
+		Value:   types.NewFloat64Datum(1.1),
+		RetType: types.NewFieldType(mysql.TypeDouble),
+	}
+
+	tests := []struct {
+		arg0     Expression
+		arg1     Expression
+		funcName string
+		result   string
+	}{
+		{colInt, conOne, ast.LT, "lt(a, 1)"},
+		{colInt, conOne, ast.LE, "le(a, 1)"},
+		{colInt, conOne, ast.GT, "gt(a, 1)"},
+		{colInt, conOne, ast.GE, "ge(a, 1)"},
+		{colInt, conOne, ast.EQ, "eq(a, 1)"},
+		{colInt, conOne, ast.NullEQ, "nulleq(a, 1)"},
+		{colInt, conOne, ast.NE, "ne(a, 1)"},
+		{colInt, conOnePointOne, ast.LT, "lt(a, 2)"},
+		{colInt, conOnePointOne, ast.LE, "le(a, 1)"},
+		{colInt, conOnePointOne, ast.GT, "gt(a, 1)"},
+		{colInt, conOnePointOne, ast.GE, "ge(a, 2)"},
+		{colInt, conOnePointOne, ast.EQ, "eq(cast(a), 1.1)"},
+		{colInt, conOnePointOne, ast.NullEQ, "nulleq(cast(a), 1.1)"},
+		{colInt, conOnePointOne, ast.NE, "ne(cast(a), 1.1)"},
+		{conOne, colInt, ast.LT, "lt(1, a)"},
+		{conOne, colInt, ast.LE, "le(1, a)"},
+		{conOne, colInt, ast.GT, "gt(1, a)"},
+		{conOne, colInt, ast.GE, "ge(1, a)"},
+		{conOne, colInt, ast.EQ, "eq(1, a)"},
+		{conOne, colInt, ast.NullEQ, "nulleq(1, a)"},
+		{conOne, colInt, ast.NE, "ne(1, a)"},
+		{conOnePointOne, colInt, ast.LT, "lt(1, a)"},
+		{conOnePointOne, colInt, ast.LE, "le(2, a)"},
+		{conOnePointOne, colInt, ast.GT, "gt(2, a)"},
+		{conOnePointOne, colInt, ast.GE, "ge(1, a)"},
+		{conOnePointOne, colInt, ast.EQ, "eq(1.1, cast(a))"},
+		{conOnePointOne, colInt, ast.NullEQ, "nulleq(1.1, cast(a))"},
+		{conOnePointOne, colInt, ast.NE, "ne(1.1, cast(a))"},
+	}
+
+	for _, t := range tests {
+		f, err := NewFunction(s.ctx, t.funcName, types.NewFieldType(mysql.TypeUnspecified), t.arg0, t.arg1)
+		c.Assert(err, IsNil)
+		c.Assert(f.String(), Equals, t.result)
+	}
+}
+
 func (s *testEvaluatorSuite) TestCompare(c *C) {
 	defer testleak.AfterTest(c)()
 

expression/util.go

@@ -23,6 +23,7 @@ import (
 	"github.com/pingcap/tidb/ast"
 	"github.com/pingcap/tidb/context"
 	"github.com/pingcap/tidb/mysql"
+	"github.com/pingcap/tidb/parser/opcode"
 	"github.com/pingcap/tidb/sessionctx/variable"
 	"github.com/pingcap/tidb/util/codec"
 	"github.com/pingcap/tidb/util/mvmap"
@@ -298,6 +299,17 @@ var oppositeOp = map[string]string{
 	ast.NE: ast.EQ,
 }
 
+// a op b is equal to b symmetricOp a
+var symmetricOp = map[opcode.Op]opcode.Op{
+	opcode.LT:     opcode.GT,
+	opcode.GE:     opcode.LE,
+	opcode.GT:     opcode.LT,
+	opcode.LE:     opcode.GE,
+	opcode.EQ:     opcode.EQ,
+	opcode.NE:     opcode.NE,
+	opcode.NullEQ: opcode.NullEQ,
+}
+
 // PushDownNot pushes the `not` function down to the expression's arguments.
 func PushDownNot(expr Expression, not bool, ctx context.Context) Expression {
 	if f, ok := expr.(*ScalarFunction); ok {