From 7e8005324cd9835ae3e989f20e9384ddf64a9abb Mon Sep 17 00:00:00 2001 From: Song Guo Date: Mon, 6 May 2019 15:40:20 +0800 Subject: [PATCH] planner: fix wrong `DATE/DATETIME` comparison in `BETWEEN` function (#10313) --- expression/builtin_compare.go | 8 ++++---- planner/core/expression_rewriter.go | 13 +++++++++++-- planner/core/expression_test.go | 2 ++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/expression/builtin_compare.go b/expression/builtin_compare.go index 895cf133bc291..a6e12a6b2474c 100644 --- a/expression/builtin_compare.go +++ b/expression/builtin_compare.go @@ -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 { @@ -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 @@ -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 diff --git a/planner/core/expression_rewriter.go b/planner/core/expression_rewriter.go index 0f36b2bc32eb2..8383b18824b0c 100644 --- a/planner/core/expression_rewriter.go +++ b/planner/core/expression_rewriter.go @@ -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 { diff --git a/planner/core/expression_test.go b/planner/core/expression_test.go index 232afd3a62d70..c44633b0a752f 100644 --- a/planner/core/expression_test.go +++ b/planner/core/expression_test.go @@ -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) }