planner: fix the issue that plan cache may return wrong result when c…

…omparing datetime column with unix_timestamp (#48413) (#53266)

close #41918, close #48165

expression/builtin_compare.go

@@ -1620,7 +1620,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
@@ -1631,17 +1631,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
@@ -1707,24 +1709,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
@@ -1817,7 +1819,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

expression/util.go

@@ -1485,16 +1485,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 (

planner/core/expression_rewriter.go

@@ -1580,7 +1580,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 {

planner/core/plan_cache_test.go

@@ -2504,6 +2504,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)