Skip to content

Commit

Permalink
planner: fix wrong DATE/DATETIME comparison in BETWEEN function (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
erjiaqing authored and qw4990 committed May 6, 2019
1 parent 190990b commit 7e80053
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
8 changes: 4 additions & 4 deletions expression/builtin_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,8 @@ func temporalWithDateAsNumEvalType(argTp *types.FieldType) (argEvalType types.Ev
return
}

// getCmpTp4MinMax gets compare type for GREATEST and LEAST.
func getCmpTp4MinMax(args []Expression) (argTp types.EvalType) {
// GetCmpTp4MinMax gets compare type for GREATEST and LEAST and BETWEEN (mainly for datetime).
func GetCmpTp4MinMax(args []Expression) (argTp types.EvalType) {
datetimeFound, isAllStr := false, true
cmpEvalType, isStr, isTemporalWithDate := temporalWithDateAsNumEvalType(args[0].GetType())
if !isStr {
Expand Down Expand Up @@ -421,7 +421,7 @@ func (c *greatestFunctionClass) getFunction(ctx sessionctx.Context, args []Expre
if err = c.verifyArgs(args); err != nil {
return nil, err
}
tp, cmpAsDatetime := getCmpTp4MinMax(args), false
tp, cmpAsDatetime := GetCmpTp4MinMax(args), false
if tp == types.ETDatetime {
cmpAsDatetime = true
tp = types.ETString
Expand Down Expand Up @@ -615,7 +615,7 @@ func (c *leastFunctionClass) getFunction(ctx sessionctx.Context, args []Expressi
if err = c.verifyArgs(args); err != nil {
return nil, err
}
tp, cmpAsDatetime := getCmpTp4MinMax(args), false
tp, cmpAsDatetime := GetCmpTp4MinMax(args), false
if tp == types.ETDatetime {
cmpAsDatetime = true
tp = types.ETString
Expand Down
13 changes: 11 additions & 2 deletions planner/core/expression_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -1259,11 +1259,20 @@ func (er *expressionRewriter) betweenToExpression(v *ast.BetweenExpr) {
if er.err != nil {
return
}

expr, lexp, rexp := er.ctxStack[stkLen-3], er.ctxStack[stkLen-2], er.ctxStack[stkLen-1]

if expression.GetCmpTp4MinMax([]expression.Expression{expr, lexp, rexp}) == types.ETDatetime {
expr = expression.WrapWithCastAsTime(er.ctx, expr, types.NewFieldType(mysql.TypeDatetime))
lexp = expression.WrapWithCastAsTime(er.ctx, lexp, types.NewFieldType(mysql.TypeDatetime))
rexp = expression.WrapWithCastAsTime(er.ctx, rexp, types.NewFieldType(mysql.TypeDatetime))
}

var op string
var l, r expression.Expression
l, er.err = er.newFunction(ast.GE, &v.Type, er.ctxStack[stkLen-3], er.ctxStack[stkLen-2])
l, er.err = er.newFunction(ast.GE, &v.Type, expr, lexp)
if er.err == nil {
r, er.err = er.newFunction(ast.LE, &v.Type, er.ctxStack[stkLen-3], er.ctxStack[stkLen-1])
r, er.err = er.newFunction(ast.LE, &v.Type, expr, rexp)
}
op = ast.LogicAnd
if er.err != nil {
Expand Down
2 changes: 2 additions & 0 deletions planner/core/expression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ func (s *testExpressionSuite) TestBetween(c *C) {
tests := []testCase{
{exprStr: "1 between 2 and 3", resultStr: "0"},
{exprStr: "1 not between 2 and 3", resultStr: "1"},
{exprStr: "'2001-04-10 12:34:56' between cast('2001-01-01 01:01:01' as datetime) and '01-05-01'", resultStr: "1"},
{exprStr: "20010410123456 between cast('2001-01-01 01:01:01' as datetime) and 010501", resultStr: "0"},
}
s.runTests(c, tests)
}
Expand Down

0 comments on commit 7e80053

Please sign in to comment.