From 18273fcdf043674b0b79eb37f3fe78ef193f2b29 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Wed, 18 Jan 2023 14:56:58 +0800 Subject: [PATCH 1/9] fixup --- planner/core/cacheable_checker.go | 13 +++++++++++++ planner/core/cacheable_checker_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/planner/core/cacheable_checker.go b/planner/core/cacheable_checker.go index 90c0fcc3bbd83..492937025461b 100644 --- a/planner/core/cacheable_checker.go +++ b/planner/core/cacheable_checker.go @@ -87,6 +87,19 @@ func (checker *cacheableChecker) Enter(in ast.Node) (out ast.Node, skipChildren return in, true } } + case *ast.InsertStmt: + if node.Select == nil { + // do not cache insert-values-stmt like 'insert into t values (...)' since + // no performance benefit and to save memory. + checker.cacheable = false + return in, true + } + for _, hints := range node.TableHints { + if hints.HintName.L == HintIgnorePlanCache { + checker.cacheable = false + return in, true + } + } case *ast.VariableExpr, *ast.ExistsSubqueryExpr, *ast.SubqueryExpr: checker.cacheable = false return in, true diff --git a/planner/core/cacheable_checker_test.go b/planner/core/cacheable_checker_test.go index d4802db30f183..3e97253663ec6 100644 --- a/planner/core/cacheable_checker_test.go +++ b/planner/core/cacheable_checker_test.go @@ -27,6 +27,30 @@ import ( "github.com/stretchr/testify/require" ) +func TestIgnoreInsertStmt(t *testing.T) { + store, clean := testkit.CreateMockStore(t) + defer clean() + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("create table t (a int)") + + // do not cache native insert-stmt + tk.MustExec("prepare st from 'insert into t values (1)'") + tk.MustExec("execute st") + tk.MustExec("execute st") + tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) + + // ignore-hint in insert-stmt can work + tk.MustExec("prepare st from 'insert into t select * from t'") + tk.MustExec("execute st") + tk.MustExec("execute st") + tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1")) + tk.MustExec("prepare st from 'insert /*+ ignore_plan_cache() */ into t select * from t'") + tk.MustExec("execute st") + tk.MustExec("execute st") + tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) +} + func TestCacheable(t *testing.T) { store, clean := testkit.CreateMockStore(t) defer clean() From 1f9f7fb0e74409f9905c6f33ed622f9073310283 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Wed, 18 Jan 2023 15:01:20 +0800 Subject: [PATCH 2/9] fixup --- executor/seqtest/prepared_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/executor/seqtest/prepared_test.go b/executor/seqtest/prepared_test.go index b8d81a5f5206c..19faabf9dea49 100644 --- a/executor/seqtest/prepared_test.go +++ b/executor/seqtest/prepared_test.go @@ -495,14 +495,14 @@ func TestPreparedInsert(t *testing.T) { err = counter.Write(pb) require.NoError(t, err) hit := pb.GetCounter().GetValue() - require.Equal(t, float64(1), hit) + require.Equal(t, float64(0), hit) } tk.MustExec(`set @a=3,@b=3; execute stmt_insert using @a, @b;`) if flag { err = counter.Write(pb) require.NoError(t, err) hit := pb.GetCounter().GetValue() - require.Equal(t, float64(2), hit) + require.Equal(t, float64(0), hit) } result := tk.MustQuery("select id, c1 from prepare_test where id = ?", 1) @@ -518,21 +518,21 @@ func TestPreparedInsert(t *testing.T) { err = counter.Write(pb) require.NoError(t, err) hit := pb.GetCounter().GetValue() - require.Equal(t, float64(2), hit) + require.Equal(t, float64(0), hit) } tk.MustExec(`set @a=2; execute stmt_insert_select using @a;`) if flag { err = counter.Write(pb) require.NoError(t, err) hit := pb.GetCounter().GetValue() - require.Equal(t, float64(3), hit) + require.Equal(t, float64(1), hit) } tk.MustExec(`set @a=3; execute stmt_insert_select using @a;`) if flag { err = counter.Write(pb) require.NoError(t, err) hit := pb.GetCounter().GetValue() - require.Equal(t, float64(4), hit) + require.Equal(t, float64(2), hit) } result = tk.MustQuery("select id, c1 from prepare_test where id = ?", 101) From 75e80d40f9949c8ebca2765a47d173a34f62c5cf Mon Sep 17 00:00:00 2001 From: qw4990 Date: Wed, 18 Jan 2023 15:02:31 +0800 Subject: [PATCH 3/9] fixup --- planner/core/cacheable_checker_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/planner/core/cacheable_checker_test.go b/planner/core/cacheable_checker_test.go index 3e97253663ec6..28c8e93ab79d8 100644 --- a/planner/core/cacheable_checker_test.go +++ b/planner/core/cacheable_checker_test.go @@ -76,7 +76,9 @@ func TestCacheable(t *testing.T) { tableRefsClause := &ast.TableRefsClause{TableRefs: &ast.Join{Left: &ast.TableSource{Source: tbl}}} // test InsertStmt - stmt = &ast.InsertStmt{Table: tableRefsClause} + stmt = &ast.InsertStmt{Table: tableRefsClause} // insert-values-stmt + require.False(t, core.Cacheable(stmt, is)) + stmt = &ast.InsertStmt{Table: tableRefsClause, Select: &ast.SelectStmt{}} // insert-select-stmt require.True(t, core.Cacheable(stmt, is)) // test DeleteStmt From d2f2259794eb6d4d46ea6f0fafba84e4f8d05798 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Wed, 18 Jan 2023 15:46:31 +0800 Subject: [PATCH 4/9] fixup --- planner/core/cacheable_checker_test.go | 87 ++++++++++++++++++++++++++ util/ranger/detacher.go | 3 +- 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/planner/core/cacheable_checker_test.go b/planner/core/cacheable_checker_test.go index 28c8e93ab79d8..6ff4fba104111 100644 --- a/planner/core/cacheable_checker_test.go +++ b/planner/core/cacheable_checker_test.go @@ -15,6 +15,8 @@ package core_test import ( + "fmt" + "github.com/pingcap/tidb/util" "testing" "github.com/pingcap/tidb/expression" @@ -27,6 +29,91 @@ import ( "github.com/stretchr/testify/require" ) +func TestInvalidRange(t *testing.T) { + store, clean := testkit.CreateMockStore(t) + defer clean() + 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 where a>? and a= ? and c <= ?' - return nil, conditions, nil, nil, false + sctx.GetSessionVars().StmtCtx.SkipPlanCache = true } } } From 9089be29b15985d7eefb2b39a7b6435ef6177f36 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Wed, 18 Jan 2023 15:47:55 +0800 Subject: [PATCH 5/9] fixup --- planner/core/cacheable_checker_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/cacheable_checker_test.go b/planner/core/cacheable_checker_test.go index 6ff4fba104111..016d1a0ca7085 100644 --- a/planner/core/cacheable_checker_test.go +++ b/planner/core/cacheable_checker_test.go @@ -16,7 +16,6 @@ package core_test import ( "fmt" - "github.com/pingcap/tidb/util" "testing" "github.com/pingcap/tidb/expression" @@ -26,6 +25,7 @@ import ( "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/testkit" driver "github.com/pingcap/tidb/types/parser_driver" + "github.com/pingcap/tidb/util" "github.com/stretchr/testify/require" ) From 22ced4ed4a8b2bad81ca7aabf0ca2033f397e898 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Wed, 18 Jan 2023 15:49:24 +0800 Subject: [PATCH 6/9] fixup --- planner/core/cacheable_checker_test.go | 33 ++++++++++++++++++++++++++ planner/core/expression_rewriter.go | 11 ++++----- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/planner/core/cacheable_checker_test.go b/planner/core/cacheable_checker_test.go index 016d1a0ca7085..e223129b59095 100644 --- a/planner/core/cacheable_checker_test.go +++ b/planner/core/cacheable_checker_test.go @@ -29,6 +29,39 @@ import ( "github.com/stretchr/testify/require" ) +func TestIssue40224(t *testing.T) { + store, clean := testkit.CreateMockStore(t) + defer clean() + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("create table t (a int, key(a))") + tk.MustExec("prepare st from 'select a from t where a in (?, ?)'") + tk.MustExec("set @a=1.0, @b=2.0") + tk.MustExec("execute st using @a, @b") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1105 skip plan-cache: '1.0' may be converted to INT")) + tk.MustExec("execute st using @a, @b") + tkProcess := tk.Session().ShowProcess() + ps := []*util.ProcessInfo{tkProcess} + tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps}) + tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).CheckAt([]int{0}, + [][]interface{}{ + {"IndexReader_6"}, + {"└─IndexRangeScan_5"}, // range scan not full scan + }) + + tk.MustExec("set @a=1, @b=2") + tk.MustExec("execute st using @a, @b") + tk.MustQuery("show warnings").Check(testkit.Rows()) // no warning for INT values + tk.MustExec("execute st using @a, @b") + tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1")) // cacheable for INT + tk.MustExec("execute st using @a, @b") + tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).CheckAt([]int{0}, + [][]interface{}{ + {"IndexReader_6"}, + {"└─IndexRangeScan_5"}, // range scan not full scan + }) +} + func TestInvalidRange(t *testing.T) { store, clean := testkit.CreateMockStore(t) defer clean() diff --git a/planner/core/expression_rewriter.go b/planner/core/expression_rewriter.go index 3026574d8e424..68c76725fe118 100644 --- a/planner/core/expression_rewriter.go +++ b/planner/core/expression_rewriter.go @@ -1496,14 +1496,11 @@ func (er *expressionRewriter) inToExpression(lLen int, not bool, tp *types.Field if c, ok := args[i].(*expression.Constant); ok { var isExceptional bool if expression.MaybeOverOptimized4PlanCache(er.sctx, []expression.Expression{c}) { - if c.GetType().EvalType() == types.ETString { - // 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. - er.sctx.GetSessionVars().StmtCtx.SkipPlanCache = true - expression.RemoveMutableConst(er.sctx, []expression.Expression{c}) - } else { - continue + if c.GetType().EvalType() == types.ETInt { + continue // no need to refine it } + er.sctx.GetSessionVars().StmtCtx.SkipPlanCache = true + expression.RemoveMutableConst(er.sctx, args) } else if er.sctx.GetSessionVars().StmtCtx.SkipPlanCache { // We should remove the mutable constant for correctness, because its value may be changed. expression.RemoveMutableConst(er.sctx, []expression.Expression{c}) From f6fd296104050ff78f73b0ece99b095be4d24555 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Sat, 28 Jan 2023 14:16:24 +0800 Subject: [PATCH 7/9] fixup --- planner/core/prepare_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/planner/core/prepare_test.go b/planner/core/prepare_test.go index 71dd3cde99752..02b9cd5cbadc6 100644 --- a/planner/core/prepare_test.go +++ b/planner/core/prepare_test.go @@ -2222,7 +2222,7 @@ func TestPlanCachePointGetAndTableDual(t *testing.T) { tk.MustQuery("execute s1 using @a1, @b1, @b1").Check(testkit.Rows()) tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) tk.MustQuery("execute s1 using @a1, @a1, @b1").Check(testkit.Rows("0000 7777 1")) - tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1")) + tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) tk.MustExec("create table t2(c1 bigint(20) primary key, c2 varchar(20))") tk.MustExec("insert into t2 values(1,'7777')") @@ -2242,13 +2242,13 @@ func TestPlanCachePointGetAndTableDual(t *testing.T) { tk.MustQuery("execute s3 using @a3,@a3").Check(testkit.Rows()) tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) tk.MustQuery("execute s3 using @a3,@b3").Check(testkit.Rows("2 1 1")) - tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1")) + tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) tk.MustExec("prepare s3 from 'select /*+ use_index_merge(t3) */ * from t3 where (c1 >= ? and c1 <= ?) or c2 > 1'") tk.MustExec("set @a3=1,@b3=3") // TableReader plan would be built, we should cache it. tk.MustQuery("execute s3 using @b3,@a3").Check(testkit.Rows()) - tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1")) + tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) tk.MustQuery("execute s3 using @a3,@b3").Check(testkit.Rows("2 1 1")) tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1")) @@ -2259,7 +2259,7 @@ func TestPlanCachePointGetAndTableDual(t *testing.T) { tk.MustQuery("execute s4 using @a4,@a4").Check(testkit.Rows()) tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) tk.MustQuery("execute s4 using @a4,@b4").Check(testkit.Rows("2 1 1")) - tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1")) + tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) tk.MustExec("prepare s4 from 'select /*+ use_index_merge(t4) */ * from t4 where (c1 >= ? and c1 <= ?) or c2 > 1'") tk.MustExec("set @a4=1,@b4=3") @@ -2337,7 +2337,7 @@ func TestIssue23671(t *testing.T) { tk.MustQuery("execute s1 using @a, @b, @c").Check(testkit.Rows("1 1")) tk.MustExec("set @a=1, @b=1, @c=10") tk.MustQuery("execute s1 using @a, @b, @c").Check(testkit.Rows("1 1", "2 2")) - tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1")) + tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) } func TestIssue29296(t *testing.T) { From 0339ad3cc93488a9d5df916341f9d414c3dfa74d Mon Sep 17 00:00:00 2001 From: qw4990 Date: Sat, 28 Jan 2023 14:19:06 +0800 Subject: [PATCH 8/9] fixup --- planner/core/prepare_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/prepare_test.go b/planner/core/prepare_test.go index 02b9cd5cbadc6..7e1b8e9d7771e 100644 --- a/planner/core/prepare_test.go +++ b/planner/core/prepare_test.go @@ -2842,7 +2842,7 @@ func TestCachedTable(t *testing.T) { tk.MustExec("prepare indexScan from 'select b from t use index(i_b) where b>?'") tk.MustExec("prepare indexLookup from 'select a from t use index(i_b) where b>? and b Date: Sat, 28 Jan 2023 14:23:38 +0800 Subject: [PATCH 9/9] fixup --- executor/explainfor_test.go | 2 +- planner/core/cacheable_checker_test.go | 1 - planner/core/prepare_test.go | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/executor/explainfor_test.go b/executor/explainfor_test.go index fc16cd93eea3f..8a5bf7b0b91cb 100644 --- a/executor/explainfor_test.go +++ b/executor/explainfor_test.go @@ -949,7 +949,7 @@ func TestIndexMerge4PlanCache(t *testing.T) { tk.MustExec("set @a=9, @b=10, @c=11;") tk.MustQuery("execute stmt using @a, @a;").Check(testkit.Rows("10 10 10")) tk.MustQuery("execute stmt using @a, @c;").Check(testkit.Rows("10 10 10", "11 11 11")) - tk.MustQuery("select @@last_plan_from_cache;").Check(testkit.Rows("1")) + tk.MustQuery("select @@last_plan_from_cache;").Check(testkit.Rows("0")) tk.MustQuery("execute stmt using @c, @a;").Check(testkit.Rows("10 10 10")) tk.MustQuery("select @@last_plan_from_cache;").Check(testkit.Rows("1")) diff --git a/planner/core/cacheable_checker_test.go b/planner/core/cacheable_checker_test.go index e223129b59095..91017116b7eb3 100644 --- a/planner/core/cacheable_checker_test.go +++ b/planner/core/cacheable_checker_test.go @@ -38,7 +38,6 @@ func TestIssue40224(t *testing.T) { tk.MustExec("prepare st from 'select a from t where a in (?, ?)'") tk.MustExec("set @a=1.0, @b=2.0") tk.MustExec("execute st using @a, @b") - tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1105 skip plan-cache: '1.0' may be converted to INT")) tk.MustExec("execute st using @a, @b") tkProcess := tk.Session().ShowProcess() ps := []*util.ProcessInfo{tkProcess} diff --git a/planner/core/prepare_test.go b/planner/core/prepare_test.go index 7e1b8e9d7771e..087b8e6128839 100644 --- a/planner/core/prepare_test.go +++ b/planner/core/prepare_test.go @@ -1583,7 +1583,7 @@ func TestIssue29303(t *testing.T) { tk.MustQuery(`execute stmt using @a,@b,@c,@d`).Check(testkit.Rows()) tk.MustExec(`set @a="龂", @b="龂", @c="龂", @d="龂"`) tk.MustQuery(`execute stmt using @a,@b,@c,@d`).Check(testkit.Rows("� 龂 � 龂")) - tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0")) + tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("1")) } func TestIssue34725(t *testing.T) {