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

…omparing datetime column with `unix_timestamp` (#48413) (#48455)

close #48165

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

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 (

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 {

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)