From 370c69363a8f77a46d7457472a28bbdb63157c9d Mon Sep 17 00:00:00 2001 From: qw4990 Date: Wed, 29 Mar 2023 17:56:56 +0800 Subject: [PATCH] fixup --- expression/builtin_compare.go | 57 ++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/expression/builtin_compare.go b/expression/builtin_compare.go index 1c72eaa012149..12e8417e0d5b2 100644 --- a/expression/builtin_compare.go +++ b/expression/builtin_compare.go @@ -1556,6 +1556,43 @@ func RefineComparedConstant(ctx sessionctx.Context, targetFieldType types.FieldT return con, false } +// Since the argument refining of cmp functions can bring some risks to the plan-cache, the optimizer +// needs to decide to whether to skip the refining or skip plan-cache for safety. +// For example, `unsigned_int_col > ?(-1)` can be refined to `True`, but the validation of this result +// can be broken if the parameter changes to 1 after. +func allowCmpArgsRefining4PlanCache(ctx sessionctx.Context, args []Expression) (allowRefining bool) { + if !MaybeOverOptimized4PlanCache(ctx, args) { + return true // plan-cache disabled or no parameter in these args + } + + // For these 2 cases below, we skip the refining: + // 1. year-expr const + // 2. int-expr string/float/double/decimal-const + for conIdx := 0; conIdx < 2; conIdx++ { + if _, isCon := args[conIdx].(*Constant); !isCon { + continue // not a constant + } + + // case 1: year-expr const + // refine `year < 12` to `year < 2012` to guarantee the correctness. + // see https://github.com/pingcap/tidb/issues/41626 for more details. + exprType := args[1-conIdx].GetType() + if exprType.GetType() == mysql.TypeYear { + ctx.GetSessionVars().StmtCtx.SkipPlanCache = true + return true + } + + conType := args[conIdx].GetType().EvalType() + if exprType.EvalType() == types.ETInt && + (conType == types.ETString || conType == types.ETReal || conType == types.ETDecimal) { + ctx.GetSessionVars().StmtCtx.SkipPlanCache = true + return true + } + } + + return false +} + // refineArgs will rewrite the arguments if the compare expression is `int column non-int constant` or // `non-int constant int column`. E.g., `a < 1.1` will be rewritten to `a < 2`. It also handles comparing year type // with int constant if the int constant falls into a sensible year representation. @@ -1565,25 +1602,17 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express arg0Type, arg1Type := args[0].GetType(), args[1].GetType() arg0IsInt := arg0Type.EvalType() == types.ETInt arg1IsInt := arg1Type.EvalType() == types.ETInt - arg0IsString := arg0Type.EvalType() == types.ETString - arg1IsString := arg1Type.EvalType() == types.ETString arg0, arg0IsCon := args[0].(*Constant) arg1, arg1IsCon := args[1].(*Constant) isExceptional, finalArg0, finalArg1 := false, args[0], args[1] isPositiveInfinite, isNegativeInfinite := false, false - if MaybeOverOptimized4PlanCache(ctx, args) { - // To keep the result be compatible with MySQL, refine `int non-constant str constant` - // here and skip this refine operation in all other cases for safety. - if (arg0IsInt && !arg0IsCon && arg1IsString && arg1IsCon) || (arg1IsInt && !arg1IsCon && arg0IsString && arg0IsCon) { - ctx.GetSessionVars().StmtCtx.SkipPlanCache = true - RemoveMutableConst(ctx, args) - } else { - return args - } - } else if ctx.GetSessionVars().StmtCtx.SkipPlanCache { - // We should remove the mutable constant for correctness, because its value may be changed. - RemoveMutableConst(ctx, args) + + if !allowCmpArgsRefining4PlanCache(ctx, args) { + return args } + // We should remove the mutable constant for correctness, because its value may be changed. + RemoveMutableConst(ctx, args) + // int non-constant [cmp] non-int constant if arg0IsInt && !arg0IsCon && !arg1IsInt && arg1IsCon { arg1, isExceptional = RefineComparedConstant(ctx, *arg0Type, arg1, c.op)