From 3e75f1619e8f252a0271a3f42d51d0bcc324945d Mon Sep 17 00:00:00 2001 From: qw4990 Date: Tue, 7 Feb 2023 14:55:24 +0800 Subject: [PATCH 1/4] fixup --- expression/builtin_compare.go | 57 +++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/expression/builtin_compare.go b/expression/builtin_compare.go index dec5d06983679..bed48c0e59096 100644 --- a/expression/builtin_compare.go +++ b/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 string-const + // 2. int-expr 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 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,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 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) From a5c4d8d734a392f4b0f4892b2929874e6d9458ee Mon Sep 17 00:00:00 2001 From: qw4990 Date: Tue, 7 Feb 2023 14:58:53 +0800 Subject: [PATCH 2/4] fixup --- planner/core/plan_cache_test.go | 39 +++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/planner/core/plan_cache_test.go b/planner/core/plan_cache_test.go index 8278050681553..479f96b183d1c 100644 --- a/planner/core/plan_cache_test.go +++ b/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 rewritten")) +} + +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) From cf506d717d4ead2f2ef7e8878573cff14bf5b7e4 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Tue, 7 Feb 2023 15:49:33 +0800 Subject: [PATCH 3/4] fixup --- planner/core/plan_cache_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/plan_cache_test.go b/planner/core/plan_cache_test.go index 479f96b183d1c..2cdec76f571fd 100644 --- a/planner/core/plan_cache_test.go +++ b/planner/core/plan_cache_test.go @@ -473,7 +473,7 @@ func TestIssue40679(t *testing.T) { 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 rewritten")) + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1105 skip plan-cache: '1.1' may be rewritten")) } func TestIssue41032(t *testing.T) { From b1b353bb9f9b1793a53a8251659f7a19ea39448b Mon Sep 17 00:00:00 2001 From: qw4990 Date: Tue, 7 Feb 2023 16:13:58 +0800 Subject: [PATCH 4/4] fixup --- planner/core/plan_cache_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/plan_cache_test.go b/planner/core/plan_cache_test.go index 2cdec76f571fd..4ecb7b3731317 100644 --- a/planner/core/plan_cache_test.go +++ b/planner/core/plan_cache_test.go @@ -473,7 +473,7 @@ func TestIssue40679(t *testing.T) { 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 rewritten")) + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1105 skip plan-cache: '1.1' may be converted to INT")) } func TestIssue41032(t *testing.T) {