From 7ace089b138ac6716aba9d0c40fbb5a8c7b7d000 Mon Sep 17 00:00:00 2001 From: "Zhuomin(Charming) Liu" Date: Tue, 14 Jul 2020 11:43:48 +0800 Subject: [PATCH 1/2] cherry pick #18458 to release-4.0 Signed-off-by: ti-srebot --- executor/adapter.go | 10 +--- executor/executor.go | 3 +- executor/prepared.go | 19 -------- planner/core/integration_test.go | 84 ++++++++++++++++++++++++++++++++ planner/optimize.go | 62 +++++++++++++++++++++-- 5 files changed, 146 insertions(+), 32 deletions(-) diff --git a/executor/adapter.go b/executor/adapter.go index 3ee5a81cab3e5..bac7780e5230b 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -243,15 +243,7 @@ func (a *ExecStmt) IsPrepared() bool { // If current StmtNode is an ExecuteStmt, we can get its prepared stmt, // then using ast.IsReadOnly function to determine a statement is read only or not. func (a *ExecStmt) IsReadOnly(vars *variable.SessionVars) bool { - if execStmt, ok := a.StmtNode.(*ast.ExecuteStmt); ok { - s, err := getPreparedStmt(execStmt, vars) - if err != nil { - logutil.BgLogger().Error("getPreparedStmt failed", zap.Error(err)) - return false - } - return ast.IsReadOnly(s) - } - return ast.IsReadOnly(a.StmtNode) + return planner.IsReadOnly(a.StmtNode, vars) } // RebuildPlan rebuilds current execute statement plan. diff --git a/executor/executor.go b/executor/executor.go index 074f755a79226..dce96ccef3939 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -40,6 +40,7 @@ import ( "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/meta" "github.com/pingcap/tidb/meta/autoid" + "github.com/pingcap/tidb/planner" plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/privilege" "github.com/pingcap/tidb/sessionctx" @@ -1529,7 +1530,7 @@ func ResetContextOfStmt(ctx sessionctx.Context, s ast.StmtNode) (err error) { sc.MemTracker.SetActionOnExceed(action) } if execStmt, ok := s.(*ast.ExecuteStmt); ok { - s, err = getPreparedStmt(execStmt, vars) + s, err = planner.GetPreparedStmt(execStmt, vars) if err != nil { return } diff --git a/executor/prepared.go b/executor/prepared.go index 2b614b6c405c5..2d360d0673615 100644 --- a/executor/prepared.go +++ b/executor/prepared.go @@ -29,7 +29,6 @@ import ( "github.com/pingcap/tidb/planner" plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/sessionctx" - "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/types" driver "github.com/pingcap/tidb/types/parser_driver" "github.com/pingcap/tidb/util" @@ -328,21 +327,3 @@ func CompileExecutePreparedStmt(ctx context.Context, sctx sessionctx.Context, } return stmt, nil } - -func getPreparedStmt(stmt *ast.ExecuteStmt, vars *variable.SessionVars) (ast.StmtNode, error) { - var ok bool - execID := stmt.ExecID - if stmt.Name != "" { - if execID, ok = vars.PreparedStmtNameToID[stmt.Name]; !ok { - return nil, plannercore.ErrStmtNotFound - } - } - if preparedPointer, ok := vars.PreparedStmts[execID]; ok { - preparedObj, ok := preparedPointer.(*plannercore.CachedPrepareStmt) - if !ok { - return nil, errors.Errorf("invalid CachedPrepareStmt type") - } - return preparedObj.PreparedAst.Stmt, nil - } - return nil, plannercore.ErrStmtNotFound -} diff --git a/planner/core/integration_test.go b/planner/core/integration_test.go index 26494242cf09e..c50391bbb96cc 100644 --- a/planner/core/integration_test.go +++ b/planner/core/integration_test.go @@ -1056,6 +1056,90 @@ func (s *testIntegrationSuite) TestStreamAggProp(c *C) { } } +<<<<<<< HEAD +======= +func (s *testIntegrationSuite) TestOptimizeHintOnPartitionTable(c *C) { + tk := testkit.NewTestKit(c, s.store) + + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec(`create table t ( + a int, b int, c varchar(20), + primary key(a), key(b), key(c) + ) partition by range columns(a) ( + partition p0 values less than(6), + partition p1 values less than(11), + partition p2 values less than(16));`) + tk.MustExec(`insert into t values (1,1,"1"), (2,2,"2"), (8,8,"8"), (11,11,"11"), (15,15,"15")`) + + // Create virtual tiflash replica info. + dom := domain.GetDomain(tk.Se) + is := dom.InfoSchema() + db, exists := is.SchemaByName(model.NewCIStr("test")) + c.Assert(exists, IsTrue) + for _, tblInfo := range db.Tables { + if tblInfo.Name.L == "t" { + tblInfo.TiFlashReplica = &model.TiFlashReplicaInfo{ + Count: 1, + Available: true, + } + } + } + + var input []string + var output []struct { + SQL string + Plan []string + Warn []string + } + s.testData.GetTestCases(c, &input, &output) + for i, tt := range input { + s.testData.OnRecord(func() { + output[i].SQL = tt + output[i].Plan = s.testData.ConvertRowsToStrings(tk.MustQuery("explain " + tt).Rows()) + output[i].Warn = s.testData.ConvertRowsToStrings(tk.MustQuery("show warnings").Rows()) + }) + tk.MustQuery("explain " + tt).Check(testkit.Rows(output[i].Plan...)) + tk.MustQuery("show warnings").Check(testkit.Rows(output[i].Warn...)) + } +} + +func (s *testIntegrationSerialSuite) TestNotReadOnlySQLOnTiFlash(c *C) { + tk := testkit.NewTestKit(c, s.store) + + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create table t (a int, b varchar(20))") + tk.MustExec(`set @@tidb_isolation_read_engines = "tiflash"`) + // Create virtual tiflash replica info. + dom := domain.GetDomain(tk.Se) + is := dom.InfoSchema() + db, exists := is.SchemaByName(model.NewCIStr("test")) + c.Assert(exists, IsTrue) + for _, tblInfo := range db.Tables { + if tblInfo.Name.L == "t" { + tblInfo.TiFlashReplica = &model.TiFlashReplicaInfo{ + Count: 1, + Available: true, + } + } + } + err := tk.ExecToErr("select * from t for update") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, `[planner:1815]Internal : Can not find access path matching 'tidb_isolation_read_engines'(value: 'tiflash'). Available values are 'tiflash, tikv'.`) + + err = tk.ExecToErr("insert into t select * from t") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, `[planner:1815]Internal : Can not find access path matching 'tidb_isolation_read_engines'(value: 'tiflash'). Available values are 'tiflash, tikv'.`) + + tk.MustExec("prepare stmt_insert from 'insert into t select * from t where t.a = ?'") + tk.MustExec("set @a=1") + err = tk.ExecToErr("execute stmt_insert using @a") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, `[planner:1815]Internal : Can not find access path matching 'tidb_isolation_read_engines'(value: 'tiflash'). Available values are 'tiflash, tikv'.`) +} + +>>>>>>> b193db8... planner: ban tiflash engine when the statement is not read only (#18458) func (s *testIntegrationSuite) TestSelectLimit(c *C) { tk := testkit.NewTestKit(c, s.store) diff --git a/planner/optimize.go b/planner/optimize.go index f498cd44b80b0..84eaffe97a5a3 100644 --- a/planner/optimize.go +++ b/planner/optimize.go @@ -31,16 +31,72 @@ import ( "github.com/pingcap/tidb/privilege" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/stmtctx" + "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/hint" "github.com/pingcap/tidb/util/logutil" + "go.uber.org/zap" ) +// GetPreparedStmt extract the prepared statement from the execute statement. +func GetPreparedStmt(stmt *ast.ExecuteStmt, vars *variable.SessionVars) (ast.StmtNode, error) { + var ok bool + execID := stmt.ExecID + if stmt.Name != "" { + if execID, ok = vars.PreparedStmtNameToID[stmt.Name]; !ok { + return nil, plannercore.ErrStmtNotFound + } + } + if preparedPointer, ok := vars.PreparedStmts[execID]; ok { + preparedObj, ok := preparedPointer.(*plannercore.CachedPrepareStmt) + if !ok { + return nil, errors.Errorf("invalid CachedPrepareStmt type") + } + return preparedObj.PreparedAst.Stmt, nil + } + return nil, plannercore.ErrStmtNotFound +} + +// IsReadOnly check whether the ast.Node is a read only statement. +func IsReadOnly(node ast.Node, vars *variable.SessionVars) bool { + if execStmt, isExecStmt := node.(*ast.ExecuteStmt); isExecStmt { + s, err := GetPreparedStmt(execStmt, vars) + if err != nil { + logutil.BgLogger().Warn("GetPreparedStmt failed", zap.Error(err)) + return false + } + return ast.IsReadOnly(s) + } + return ast.IsReadOnly(node) +} + // Optimize does optimization and creates a Plan. // The node must be prepared first. func Optimize(ctx context.Context, sctx sessionctx.Context, node ast.Node, is infoschema.InfoSchema) (plannercore.Plan, types.NameSlice, error) { +<<<<<<< HEAD if _, isolationReadContainTiKV := sctx.GetSessionVars().GetIsolationReadEngines()[kv.TiKV]; isolationReadContainTiKV { fp := plannercore.TryFastPlan(sctx, node) +======= + sessVars := sctx.GetSessionVars() + + // Because for write stmt, TiFlash has a different results when lock the data in point get plan. We ban the TiFlash + // engine in not read only stmt. + if _, isolationReadContainTiFlash := sessVars.IsolationReadEngines[kv.TiFlash]; isolationReadContainTiFlash && !IsReadOnly(node, sessVars) { + delete(sessVars.IsolationReadEngines, kv.TiFlash) + defer func() { + sessVars.IsolationReadEngines[kv.TiFlash] = struct{}{} + }() + } + + if _, isolationReadContainTiKV := sessVars.IsolationReadEngines[kv.TiKV]; isolationReadContainTiKV { + var fp plannercore.Plan + if fpv, ok := sctx.Value(plannercore.PointPlanKey).(plannercore.PointPlanVal); ok { + // point plan is already tried in a multi-statement query. + fp = fpv.Plan + } else { + fp = plannercore.TryFastPlan(sctx, node) + } +>>>>>>> b193db8... planner: ban tiflash engine when the statement is not read only (#18458) if fp != nil { if !useMaxTS(sctx, fp) { sctx.PrepareTSFuture(ctx) @@ -54,17 +110,17 @@ func Optimize(ctx context.Context, sctx sessionctx.Context, node ast.Node, is in tableHints := hint.ExtractTableHintsFromStmtNode(node, sctx) stmtHints, warns := handleStmtHints(tableHints) defer func() { - sctx.GetSessionVars().StmtCtx.StmtHints = stmtHints + sessVars.StmtCtx.StmtHints = stmtHints for _, warn := range warns { sctx.GetSessionVars().StmtCtx.AppendWarning(warn) } }() - sctx.GetSessionVars().StmtCtx.StmtHints = stmtHints + sessVars.StmtCtx.StmtHints = stmtHints bestPlan, names, _, err := optimize(ctx, sctx, node, is) if err != nil { return nil, nil, err } - if !(sctx.GetSessionVars().UsePlanBaselines || sctx.GetSessionVars().EvolvePlanBaselines) { + if !(sessVars.UsePlanBaselines || sessVars.EvolvePlanBaselines) { return bestPlan, names, nil } stmtNode, ok := node.(ast.StmtNode) From 1a1233d6e6756bbe1dda2f2a89dedf47cf2fc09e Mon Sep 17 00:00:00 2001 From: lzmhhh123 Date: Tue, 14 Jul 2020 11:51:58 +0800 Subject: [PATCH 2/2] resolve conflicts --- planner/core/integration_test.go | 49 -------------------------------- planner/optimize.go | 16 ++--------- 2 files changed, 2 insertions(+), 63 deletions(-) diff --git a/planner/core/integration_test.go b/planner/core/integration_test.go index c50391bbb96cc..de08a3b84176c 100644 --- a/planner/core/integration_test.go +++ b/planner/core/integration_test.go @@ -1056,54 +1056,6 @@ func (s *testIntegrationSuite) TestStreamAggProp(c *C) { } } -<<<<<<< HEAD -======= -func (s *testIntegrationSuite) TestOptimizeHintOnPartitionTable(c *C) { - tk := testkit.NewTestKit(c, s.store) - - tk.MustExec("use test") - tk.MustExec("drop table if exists t") - tk.MustExec(`create table t ( - a int, b int, c varchar(20), - primary key(a), key(b), key(c) - ) partition by range columns(a) ( - partition p0 values less than(6), - partition p1 values less than(11), - partition p2 values less than(16));`) - tk.MustExec(`insert into t values (1,1,"1"), (2,2,"2"), (8,8,"8"), (11,11,"11"), (15,15,"15")`) - - // Create virtual tiflash replica info. - dom := domain.GetDomain(tk.Se) - is := dom.InfoSchema() - db, exists := is.SchemaByName(model.NewCIStr("test")) - c.Assert(exists, IsTrue) - for _, tblInfo := range db.Tables { - if tblInfo.Name.L == "t" { - tblInfo.TiFlashReplica = &model.TiFlashReplicaInfo{ - Count: 1, - Available: true, - } - } - } - - var input []string - var output []struct { - SQL string - Plan []string - Warn []string - } - s.testData.GetTestCases(c, &input, &output) - for i, tt := range input { - s.testData.OnRecord(func() { - output[i].SQL = tt - output[i].Plan = s.testData.ConvertRowsToStrings(tk.MustQuery("explain " + tt).Rows()) - output[i].Warn = s.testData.ConvertRowsToStrings(tk.MustQuery("show warnings").Rows()) - }) - tk.MustQuery("explain " + tt).Check(testkit.Rows(output[i].Plan...)) - tk.MustQuery("show warnings").Check(testkit.Rows(output[i].Warn...)) - } -} - func (s *testIntegrationSerialSuite) TestNotReadOnlySQLOnTiFlash(c *C) { tk := testkit.NewTestKit(c, s.store) @@ -1139,7 +1091,6 @@ func (s *testIntegrationSerialSuite) TestNotReadOnlySQLOnTiFlash(c *C) { c.Assert(err.Error(), Equals, `[planner:1815]Internal : Can not find access path matching 'tidb_isolation_read_engines'(value: 'tiflash'). Available values are 'tiflash, tikv'.`) } ->>>>>>> b193db8... planner: ban tiflash engine when the statement is not read only (#18458) func (s *testIntegrationSuite) TestSelectLimit(c *C) { tk := testkit.NewTestKit(c, s.store) diff --git a/planner/optimize.go b/planner/optimize.go index 84eaffe97a5a3..5fe4c09d59de0 100644 --- a/planner/optimize.go +++ b/planner/optimize.go @@ -73,10 +73,6 @@ func IsReadOnly(node ast.Node, vars *variable.SessionVars) bool { // Optimize does optimization and creates a Plan. // The node must be prepared first. func Optimize(ctx context.Context, sctx sessionctx.Context, node ast.Node, is infoschema.InfoSchema) (plannercore.Plan, types.NameSlice, error) { -<<<<<<< HEAD - if _, isolationReadContainTiKV := sctx.GetSessionVars().GetIsolationReadEngines()[kv.TiKV]; isolationReadContainTiKV { - fp := plannercore.TryFastPlan(sctx, node) -======= sessVars := sctx.GetSessionVars() // Because for write stmt, TiFlash has a different results when lock the data in point get plan. We ban the TiFlash @@ -87,16 +83,8 @@ func Optimize(ctx context.Context, sctx sessionctx.Context, node ast.Node, is in sessVars.IsolationReadEngines[kv.TiFlash] = struct{}{} }() } - - if _, isolationReadContainTiKV := sessVars.IsolationReadEngines[kv.TiKV]; isolationReadContainTiKV { - var fp plannercore.Plan - if fpv, ok := sctx.Value(plannercore.PointPlanKey).(plannercore.PointPlanVal); ok { - // point plan is already tried in a multi-statement query. - fp = fpv.Plan - } else { - fp = plannercore.TryFastPlan(sctx, node) - } ->>>>>>> b193db8... planner: ban tiflash engine when the statement is not read only (#18458) + if _, isolationReadContainTiKV := sctx.GetSessionVars().GetIsolationReadEngines()[kv.TiKV]; isolationReadContainTiKV { + fp := plannercore.TryFastPlan(sctx, node) if fp != nil { if !useMaxTS(sctx, fp) { sctx.PrepareTSFuture(ctx)