Skip to content

Commit

Permalink
*: fix the read-only check for the prepare/execute statement (pingcap…
Browse files Browse the repository at this point in the history
  • Loading branch information
jackysp committed Apr 4, 2019
1 parent e87c6f1 commit ff2c1b6
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 14 deletions.
21 changes: 12 additions & 9 deletions executor/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,15 +163,18 @@ func (a *ExecStmt) IsPrepared() bool {
}

// IsReadOnly returns true if a statement is read only.
// It will update readOnlyCheckStmt if current ExecStmt can be conveted to
// a plannercore.Execute. Last step is using ast.IsReadOnly function to determine
// a statement is read only or not.
func (a *ExecStmt) IsReadOnly() bool {
readOnlyCheckStmt := a.StmtNode
if checkPlan, ok := a.Plan.(*plannercore.Execute); ok {
readOnlyCheckStmt = checkPlan.Stmt
}
return ast.IsReadOnly(readOnlyCheckStmt)
// 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.Logger(context.Background()).Error("getPreparedStmt failed", zap.Error(err))
return false
}
return ast.IsReadOnly(s)
}
return ast.IsReadOnly(a.StmtNode)
}

// RebuildPlan rebuilds current execute statement plan.
Expand Down
3 changes: 3 additions & 0 deletions executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1260,6 +1260,9 @@ func ResetContextOfStmt(ctx sessionctx.Context, s ast.StmtNode) (err error) {

if execStmt, ok := s.(*ast.ExecuteStmt); ok {
s, err = getPreparedStmt(execStmt, vars)
if err != nil {
return
}
}
// TODO: Many same bool variables here.
// We should set only two variables (
Expand Down
3 changes: 0 additions & 3 deletions session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,9 +523,6 @@ func (s *session) retry(ctx context.Context, maxCnt uint) (err error) {
s.sessionVars.RetryInfo.ResetOffset()
for i, sr := range nh.history {
st := sr.st
if st.IsReadOnly() {
continue
}
s.sessionVars.StmtCtx = sr.stmtCtx
s.sessionVars.StmtCtx.ResetForRetry()
s.sessionVars.PreparedParams = s.sessionVars.PreparedParams[:0]
Expand Down
19 changes: 19 additions & 0 deletions session/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,25 @@ func (s *testSessionSuite) TestAutoIncrementWithRetry(c *C) {
c.Assert(lastInsertID+3, Equals, currLastInsertID)
}

func (s *testSessionSuite) TestBinaryReadOnly(c *C) {
tk := testkit.NewTestKitWithInit(c, s.store)
tk.MustExec("create table t (i int key)")
id, _, _, err := tk.Se.PrepareStmt("select i from t where i = ?")
c.Assert(err, IsNil)
id2, _, _, err := tk.Se.PrepareStmt("insert into t values (?)")
c.Assert(err, IsNil)
tk.MustExec("set autocommit = 0")
_, err = tk.Se.ExecutePreparedStmt(context.Background(), id, 1)
c.Assert(err, IsNil)
c.Assert(session.GetHistory(tk.Se).Count(), Equals, 0)
tk.MustExec("insert into t values (1)")
c.Assert(session.GetHistory(tk.Se).Count(), Equals, 1)
_, err = tk.Se.ExecutePreparedStmt(context.Background(), id2, 2)
c.Assert(err, IsNil)
c.Assert(session.GetHistory(tk.Se).Count(), Equals, 2)
tk.MustExec("commit")
}

func (s *testSessionSuite) TestPrepare(c *C) {
tk := testkit.NewTestKitWithInit(c, s.store)
tk.MustExec("create table t(id TEXT)")
Expand Down
2 changes: 1 addition & 1 deletion session/tidb.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func runStmt(ctx context.Context, sctx sessionctx.Context, s sqlexec.Statement)
sessVars := se.GetSessionVars()
// All the history should be added here.
sessVars.TxnCtx.StatementCount++
if !s.IsReadOnly() {
if !s.IsReadOnly(sessVars) {
if err == nil {
GetHistory(sctx).Add(0, s, se.sessionVars.StmtCtx)
}
Expand Down
3 changes: 2 additions & 1 deletion util/sqlexec/restricted_sql_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package sqlexec
import (
"github.com/pingcap/parser/ast"
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/sessionctx/variable"
"github.com/pingcap/tidb/util/chunk"
"golang.org/x/net/context"
)
Expand Down Expand Up @@ -69,7 +70,7 @@ type Statement interface {
IsPrepared() bool

// IsReadOnly returns if the statement is read only. For example: SelectStmt without lock.
IsReadOnly() bool
IsReadOnly(vars *variable.SessionVars) bool

// RebuildPlan rebuilds the plan of the statement.
RebuildPlan() (schemaVersion int64, err error)
Expand Down

0 comments on commit ff2c1b6

Please sign in to comment.