diff --git a/pkg/expression/builtin_compare.go b/pkg/expression/builtin_compare.go index e04d76025845d..6dcd41c42ec56 100644 --- a/pkg/expression/builtin_compare.go +++ b/pkg/expression/builtin_compare.go @@ -1570,7 +1570,7 @@ func allowCmpArgsRefining4PlanCache(ctx sessionctx.Context, args []Expression) ( // 3. It also handles comparing datetime/timestamp column with numeric constant, try to cast numeric constant as timestamp type, do nothing if failed. // This refining operation depends on the values of these args, but these values can change when using plan-cache. // So we have to skip this operation or mark the plan as over-optimized when using plan-cache. -func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Expression) []Expression { +func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Expression) ([]Expression, error) { arg0Type, arg1Type := args[0].GetType(), args[1].GetType() arg0EvalType, arg1EvalType := arg0Type.EvalType(), arg1Type.EvalType() arg0IsInt := arg0EvalType == types.ETInt @@ -1581,17 +1581,19 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express isPositiveInfinite, isNegativeInfinite := false, false if !allowCmpArgsRefining4PlanCache(ctx, args) { - return args + return args, nil } // We should remove the mutable constant for correctness, because its value may be changed. - RemoveMutableConst(ctx, args) + if err := RemoveMutableConst(ctx, args); err != nil { + return nil, err + } if arg0IsCon && !arg1IsCon && matchRefineRule3Pattern(arg0EvalType, arg1Type) { - return c.refineNumericConstantCmpDatetime(ctx, args, arg0, 0) + return c.refineNumericConstantCmpDatetime(ctx, args, arg0, 0), nil } if !arg0IsCon && arg1IsCon && matchRefineRule3Pattern(arg1EvalType, arg0Type) { - return c.refineNumericConstantCmpDatetime(ctx, args, arg1, 1) + return c.refineNumericConstantCmpDatetime(ctx, args, arg1, 1), nil } // int non-constant [cmp] non-int constant @@ -1657,24 +1659,24 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express } if isExceptional && (c.op == opcode.EQ || c.op == opcode.NullEQ) { // This will always be false. - return []Expression{NewZero(), NewOne()} + return []Expression{NewZero(), NewOne()}, nil } if isPositiveInfinite { // If the op is opcode.LT, opcode.LE // This will always be true. // If the op is opcode.GT, opcode.GE // This will always be false. - return []Expression{NewZero(), NewOne()} + return []Expression{NewZero(), NewOne()}, nil } if isNegativeInfinite { // If the op is opcode.GT, opcode.GE // This will always be true. // If the op is opcode.LT, opcode.LE // This will always be false. - return []Expression{NewOne(), NewZero()} + return []Expression{NewOne(), NewZero()}, nil } - return c.refineArgsByUnsignedFlag(ctx, []Expression{finalArg0, finalArg1}) + return c.refineArgsByUnsignedFlag(ctx, []Expression{finalArg0, finalArg1}), nil } // see https://github.com/pingcap/tidb/issues/38361 for more details @@ -1767,7 +1769,10 @@ func (c *compareFunctionClass) getFunction(ctx sessionctx.Context, rawArgs []Exp if err = c.verifyArgs(rawArgs); err != nil { return nil, err } - args := c.refineArgs(ctx, rawArgs) + args, err := c.refineArgs(ctx, rawArgs) + if err != nil { + return nil, err + } cmpType := GetAccurateCmpType(args[0], args[1]) sig, err = c.generateCmpSigs(ctx, args, cmpType) return sig, err diff --git a/pkg/expression/util.go b/pkg/expression/util.go index 8a493b885631e..2d41fb83927ad 100644 --- a/pkg/expression/util.go +++ b/pkg/expression/util.go @@ -1496,16 +1496,25 @@ func containMutableConst(ctx sessionctx.Context, exprs []Expression) bool { } // RemoveMutableConst used to remove the `ParamMarker` and `DeferredExpr` in the `Constant` expr. -func RemoveMutableConst(ctx sessionctx.Context, exprs []Expression) { +func RemoveMutableConst(ctx sessionctx.Context, exprs []Expression) (err error) { for _, expr := range exprs { switch v := expr.(type) { case *Constant: v.ParamMarker = nil - v.DeferredExpr = nil + if v.DeferredExpr != nil { // evaluate and update v.Value to convert v to a complete immutable constant. + // TODO: remove or hide DefferedExpr since it's too dangerous (hard to be consistent with v.Value all the time). + v.Value, err = v.DeferredExpr.Eval(chunk.Row{}) + if err != nil { + return err + } + v.DeferredExpr = nil + } + v.DeferredExpr = nil // do nothing since v.Value has already been evaluated in this case. case *ScalarFunction: - RemoveMutableConst(ctx, v.GetArgs()) + return RemoveMutableConst(ctx, v.GetArgs()) } } + return nil } const ( diff --git a/pkg/planner/core/expression_rewriter.go b/pkg/planner/core/expression_rewriter.go index 8d43879693180..4868da790c7c5 100644 --- a/pkg/planner/core/expression_rewriter.go +++ b/pkg/planner/core/expression_rewriter.go @@ -1677,7 +1677,10 @@ func (er *expressionRewriter) inToExpression(lLen int, not bool, tp *types.Field continue // no need to refine it } er.sctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.Errorf("'%v' may be converted to INT", c.String())) - expression.RemoveMutableConst(er.sctx, []expression.Expression{c}) + if err := expression.RemoveMutableConst(er.sctx, []expression.Expression{c}); err != nil { + er.err = err + return + } } args[i], isExceptional = expression.RefineComparedConstant(er.sctx, *leftFt, c, opcode.EQ) if isExceptional { diff --git a/pkg/planner/core/plan_cache_test.go b/pkg/planner/core/plan_cache_test.go index 3a70630733020..a8543eb7b7135 100644 --- a/pkg/planner/core/plan_cache_test.go +++ b/pkg/planner/core/plan_cache_test.go @@ -1349,6 +1349,16 @@ func TestNonPreparedPlanCacheBuiltinFuncs(t *testing.T) { } } +func TestIssue48165(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec(`create table t(a int)`) + tk.MustExec(`insert into t values(1)`) + tk.MustExec(`prepare s from "select * from t where tidb_parse_tso(a) > unix_timestamp()"`) + tk.MustQuery(`execute s`).Check(testkit.Rows("1")) +} + func BenchmarkPlanCacheInsert(b *testing.B) { store := testkit.CreateMockStore(b) tk := testkit.NewTestKit(b, store)