planner: label plans as over-optimized for plan cache after refining …

…cmp-function arguments (#41136)

close #40679, close #41032

expression/builtin_compare.go

@@ -1556,6 +1556,37 @@ 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 which may affect the index selection a lot, skip plan-cache,
+	// and for all other cases, skip the refining.
+	// 1. int-expr <cmp> string-const
+	// 2. int-expr <cmp> float/double/decimal-const
+	for conIdx := 0; conIdx < 2; conIdx++ {
+		if args[1-conIdx].GetType().EvalType() != types.ETInt {
+			continue // not a int-expr
+		}
+		if _, isCon := args[conIdx].(*Constant); !isCon {
+			continue // not a constant
+		}
+		conType := args[conIdx].GetType().EvalType()
+		if conType == types.ETString || conType == types.ETReal || conType == types.ETDecimal {
+			reason := errors.Errorf("skip plan-cache: '%v' may be converted to INT", args[conIdx].String())
+			ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(reason)
+			return true
+		}
+	}
+
+	return false
+}
+
 // refineArgs will rewrite the arguments if the compare expression is `int column <cmp> non-int constant` or
 // `non-int constant <cmp> 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,31 +1596,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 <cmp> str constant`
-		// here and skip this refine operation in all other cases for safety.
-		if (arg0IsInt && !arg0IsCon && arg1IsString && arg1IsCon) || (arg1IsInt && !arg1IsCon && arg0IsString && arg0IsCon) {
-			var reason error
-			if arg1IsString {
-				reason = errors.Errorf("skip plan-cache: '%v' may be converted to INT", arg1.String())
-			} else { // arg0IsString
-				reason = errors.Errorf("skip plan-cache: '%v' may be converted to INT", arg0.String())
-			}
-			ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(reason)
-			RemoveMutableConst(ctx, args)
-		} else {
-			return args
-		}
-	} else if !ctx.GetSessionVars().StmtCtx.UseCache {
-		// 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)

planner/core/plan_cache_test.go

@@ -457,6 +457,45 @@ func TestIssue40225(t *testing.T) {
 	tk.MustQuery("select @@last_plan_from_binding").Check(testkit.Rows("1"))
 }
 
+func TestIssue40679(t *testing.T) {
+	store := testkit.CreateMockStore(t)
+	tk := testkit.NewTestKit(t, store)
+	tk.MustExec("use test")
+	tk.MustExec("create table t (a int, key(a));")
+	tk.MustExec("prepare st from 'select * from t use index(a) where a < ?'")
+	tk.MustExec("set @a1=1.1")
+	tk.MustExec("execute st using @a1")
+
+	tkProcess := tk.Session().ShowProcess()
+	ps := []*util.ProcessInfo{tkProcess}
+	tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
+	rows := tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).Rows()
+	require.True(t, strings.Contains(rows[1][0].(string), "RangeScan")) // RangeScan not FullScan
+
+	tk.MustExec("execute st using @a1")
+	tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1105 skip plan-cache: '1.1' may be converted to INT"))
+}
+
+func TestIssue41032(t *testing.T) {
+	store := testkit.CreateMockStore(t)
+	tk := testkit.NewTestKit(t, store)
+	tk.MustExec("use test")
+	tk.MustExec(`CREATE TABLE PK_SIGNED_10087 (
+	 COL1 mediumint(8) unsigned NOT NULL,
+	 COL2 varchar(20) DEFAULT NULL,
+	 COL4 datetime DEFAULT NULL,
+	 COL3 bigint(20) DEFAULT NULL,
+	 COL5 float DEFAULT NULL,
+	 PRIMARY KEY (COL1) )`)
+	tk.MustExec(`insert into PK_SIGNED_10087 values(0, "痥腜蟿鮤枓欜喧檕澙姭袐裄钭僇剕焍哓閲疁櫘", "0017-11-14 05:40:55", -4504684261333179273, 7.97449e37)`)
+	tk.MustExec(`prepare stmt from 'SELECT/*+ HASH_JOIN(t1, t2) */ t2.* FROM PK_SIGNED_10087 t1 JOIN PK_SIGNED_10087 t2 ON t1.col1 = t2.col1 WHERE t2.col1 >= ? AND t1.col1 >= ?;'`)
+	tk.MustExec(`set @a=0, @b=0`)
+	tk.MustQuery(`execute stmt using @a,@b`).Check(testkit.Rows("0 痥腜蟿鮤枓欜喧檕澙姭袐裄钭僇剕焍哓閲疁櫘 0017-11-14 05:40:55 -4504684261333179273 79744900000000000000000000000000000000"))
+	tk.MustExec(`set @a=8950167, @b=16305982`)
+	tk.MustQuery(`execute stmt using @a,@b`).Check(testkit.Rows())
+	tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("1"))
+}
+
 func TestPlanCacheWithLimit(t *testing.T) {
 	store := testkit.CreateMockStore(t)
 	tk := testkit.NewTestKit(t, store)