From 0703a64f76baf8f79126ee45780310737f55df0b Mon Sep 17 00:00:00 2001 From: you06 Date: Fri, 13 May 2022 10:42:35 +0800 Subject: [PATCH] planner: plan cache always check scheme valid in RC isolation level (#34523) close pingcap/tidb#34447 --- executor/adapter.go | 2 +- planner/core/logical_plan_builder.go | 4 ++- planner/core/planbuilder.go | 4 ++- planner/core/point_get_plan.go | 1 + planner/core/prepare_test.go | 47 ++++++++++++++++++++++++++++ planner/core/rule_build_key_info.go | 4 ++- planner/optimize.go | 7 ++++- session/session.go | 3 +- 8 files changed, 66 insertions(+), 6 deletions(-) diff --git a/executor/adapter.go b/executor/adapter.go index aaa1c5cfc4f6b..b2e7c2c78f643 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -841,7 +841,7 @@ type pessimisticTxn interface { KeysNeedToLock() ([]kv.Key, error) } -// buildExecutor build a executor from plan, prepared statement may need additional procedure. +// buildExecutor build an executor from plan, prepared statement may need additional procedure. func (a *ExecStmt) buildExecutor() (Executor, error) { ctx := a.Ctx stmtCtx := ctx.GetSessionVars().StmtCtx diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 69773f62d9535..a7083de475ba9 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -4488,7 +4488,9 @@ func (ds *DataSource) ExtractFD() *fd.FDSet { changed bool err error ) - if ds.isForUpdateRead { + check := ds.ctx.GetSessionVars().IsIsolation(ast.ReadCommitted) || ds.isForUpdateRead + check = check && ds.ctx.GetSessionVars().ConnectionID > 0 + if check { latestIndexes, changed, err = getLatestIndexInfo(ds.ctx, ds.table.Meta().ID, 0) if err != nil { ds.fdSet = fds diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index f2005a20c40be..e41731103b91e 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -1095,6 +1095,7 @@ func getPossibleAccessPaths(ctx sessionctx.Context, tableHints *tableHintInfo, i optimizerUseInvisibleIndexes := ctx.GetSessionVars().OptimizerUseInvisibleIndexes + check = check || ctx.GetSessionVars().IsIsolation(ast.ReadCommitted) check = check && ctx.GetSessionVars().ConnectionID > 0 var latestIndexes map[int64]*model.IndexInfo var err error @@ -1594,7 +1595,8 @@ func (b *PlanBuilder) buildPhysicalIndexLookUpReaders(ctx context.Context, dbNam indexInfos := make([]*model.IndexInfo, 0, len(tblInfo.Indices)) indexLookUpReaders := make([]Plan, 0, len(tblInfo.Indices)) - check := b.isForUpdateRead && b.ctx.GetSessionVars().ConnectionID > 0 + check := b.isForUpdateRead || b.ctx.GetSessionVars().IsIsolation(ast.ReadCommitted) + check = check && b.ctx.GetSessionVars().ConnectionID > 0 var latestIndexes map[int64]*model.IndexInfo var err error diff --git a/planner/core/point_get_plan.go b/planner/core/point_get_plan.go index 794cc01490a3e..afb34814f7196 100644 --- a/planner/core/point_get_plan.go +++ b/planner/core/point_get_plan.go @@ -945,6 +945,7 @@ func tryPointGetPlan(ctx sessionctx.Context, selStmt *ast.SelectStmt, check bool return nil } + check = check || ctx.GetSessionVars().IsIsolation(ast.ReadCommitted) check = check && ctx.GetSessionVars().ConnectionID > 0 var latestIndexes map[int64]*model.IndexInfo var err error diff --git a/planner/core/prepare_test.go b/planner/core/prepare_test.go index f20b9673a33ca..a02e955a93eba 100644 --- a/planner/core/prepare_test.go +++ b/planner/core/prepare_test.go @@ -2805,3 +2805,50 @@ func TestCachedTable(t *testing.T) { require.True(t, lastReadFromCache(tk)) tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1")) } + +func TestPlanCacheWithRCWhenInfoSchemaChange(t *testing.T) { + orgEnable := core.PreparedPlanCacheEnabled() + defer func() { + core.SetPreparedPlanCache(orgEnable) + }() + core.SetPreparedPlanCache(true) + + ctx := context.Background() + store, clean := testkit.CreateMockStore(t) + defer clean() + + tk1 := testkit.NewTestKit(t, store) + tk2 := testkit.NewTestKit(t, store) + tk1.MustExec("use test") + tk2.MustExec("use test") + tk1.MustExec("drop table if exists t1") + tk1.MustExec("create table t1(id int primary key, c int, index ic (c))") + // prepare text protocol + tk1.MustExec("prepare s from 'select /*+use_index(t1, ic)*/ * from t1 where 1'") + // prepare binary protocol + stmtID, _, _, err := tk2.Session().PrepareStmt("select /*+use_index(t1, ic)*/ * from t1 where 1") + require.Nil(t, err) + tk1.MustExec("set tx_isolation='READ-COMMITTED'") + tk1.MustExec("begin pessimistic") + tk2.MustExec("set tx_isolation='READ-COMMITTED'") + tk2.MustExec("begin pessimistic") + tk1.MustQuery("execute s").Check(testkit.Rows()) + rs, err := tk2.Session().ExecutePreparedStmt(ctx, stmtID, []types.Datum{}) + require.Nil(t, err) + tk2.ResultSetToResult(rs, fmt.Sprintf("%v", rs)).Check(testkit.Rows()) + + tk3 := testkit.NewTestKit(t, store) + tk3.MustExec("use test") + tk3.MustExec("alter table t1 drop index ic") + tk3.MustExec("insert into t1 values(1, 0)") + + // The execution after schema changed should not hit plan cache. + // execute text protocol + tk1.MustQuery("execute s").Check(testkit.Rows("1 0")) + tk1.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) + // execute binary protocol + rs, err = tk2.Session().ExecutePreparedStmt(ctx, stmtID, []types.Datum{}) + require.Nil(t, err) + tk2.ResultSetToResult(rs, fmt.Sprintf("%v", rs)).Check(testkit.Rows("1 0")) + tk2.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) +} diff --git a/planner/core/rule_build_key_info.go b/planner/core/rule_build_key_info.go index a1e7798ac0dfa..22ec84d150bc4 100644 --- a/planner/core/rule_build_key_info.go +++ b/planner/core/rule_build_key_info.go @@ -267,8 +267,10 @@ func (ds *DataSource) BuildKeyInfo(selfSchema *expression.Schema, childSchema [] var latestIndexes map[int64]*model.IndexInfo var changed bool var err error + check := ds.ctx.GetSessionVars().IsIsolation(ast.ReadCommitted) || ds.isForUpdateRead + check = check && ds.ctx.GetSessionVars().ConnectionID > 0 // we should check index valid while forUpdateRead, see detail in https://github.com/pingcap/tidb/pull/22152 - if ds.isForUpdateRead { + if check { latestIndexes, changed, err = getLatestIndexInfo(ds.ctx, ds.table.Meta().ID, 0) if err != nil { return diff --git a/planner/optimize.go b/planner/optimize.go index 26328f64b610a..112932b6c595b 100644 --- a/planner/optimize.go +++ b/planner/optimize.go @@ -73,7 +73,12 @@ func GetExecuteForUpdateReadIS(node ast.Node, sctx sessionctx.Context) infoschem execID = vars.PreparedStmtNameToID[execStmt.Name] } if preparedPointer, ok := vars.PreparedStmts[execID]; ok { - if preparedObj, ok := preparedPointer.(*core.CachedPrepareStmt); ok && preparedObj.ForUpdateRead { + checkSchema := vars.IsIsolation(ast.ReadCommitted) + if !checkSchema { + preparedObj, ok := preparedPointer.(*core.CachedPrepareStmt) + checkSchema = ok && preparedObj.ForUpdateRead + } + if checkSchema { is := domain.GetDomain(sctx).InfoSchema() return temptable.AttachLocalTemporaryTableInfoSchema(sctx, is) } diff --git a/session/session.go b/session/session.go index 6f79bfb968609..eecc0be9000dc 100644 --- a/session/session.go +++ b/session/session.go @@ -2373,8 +2373,9 @@ func (s *session) ExecutePreparedStmt(ctx context.Context, stmtID uint32, args [ if err != nil { return nil, err } - } else if preparedStmt.ForUpdateRead { + } else if s.sessionVars.IsIsolation(ast.ReadCommitted) || preparedStmt.ForUpdateRead { is = domain.GetDomain(s).InfoSchema() + is = temptable.AttachLocalTemporaryTableInfoSchema(s, is) } else { is = s.GetInfoSchema().(infoschema.InfoSchema) }