Skip to content

Commit

Permalink
executor: execute some statement (create user grant etc) would co…
Browse files Browse the repository at this point in the history
…mmit current transaction automically (#10739)
  • Loading branch information
tiancaiamao authored and zz-jason committed Jun 6, 2019
1 parent 480bd62 commit 1602766
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 4 deletions.
11 changes: 10 additions & 1 deletion executor/grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (e *GrantExec) Next(ctx context.Context, chk *chunk.Chunk) error {
dbName = e.ctx.GetSessionVars().CurrentDB
}
// Grant for each user
for _, user := range e.Users {
for idx, user := range e.Users {
// Check if user exists.
exists, err := userExists(e.ctx, user.User.Username, user.User.Hostname)
if err != nil {
Expand Down Expand Up @@ -103,6 +103,15 @@ func (e *GrantExec) Next(ctx context.Context, chk *chunk.Chunk) error {
if e.WithGrant {
privs = append(privs, &ast.PrivElem{Priv: mysql.GrantPriv})
}

if idx == 0 {
// Commit the old transaction, like DDL.
if err := e.ctx.NewTxn(); err != nil {
return err
}
defer func() { e.ctx.GetSessionVars().SetStatusFlag(mysql.ServerStatusInTrans, false) }()
}

// Grant each priv to the user.
for _, priv := range privs {
if len(priv.Cols) > 0 {
Expand Down
9 changes: 8 additions & 1 deletion executor/revoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (e *RevokeExec) Next(ctx context.Context, chk *chunk.Chunk) error {
e.done = true

// Revoke for each user.
for _, user := range e.Users {
for idx, user := range e.Users {
// Check if user exists.
exists, err := userExists(e.ctx, user.User.Username, user.User.Hostname)
if err != nil {
Expand All @@ -68,6 +68,13 @@ func (e *RevokeExec) Next(ctx context.Context, chk *chunk.Chunk) error {
return errors.Errorf("Unknown user: %s", user.User)
}

if idx == 0 {
// Commit the old transaction, like DDL.
if err := e.ctx.NewTxn(); err != nil {
return err
}
defer func() { e.ctx.GetSessionVars().SetStatusFlag(mysql.ServerStatusInTrans, false) }()
}
err = e.revokeOneUser(user.User.Username, user.User.Hostname)
if err != nil {
return errors.Trace(err)
Expand Down
22 changes: 20 additions & 2 deletions executor/simple.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@ func (e *SimpleExec) Next(ctx context.Context, chk *chunk.Chunk) (err error) {
if e.done {
return nil
}

if e.autoNewTxn() {
// Commit the old transaction, like DDL.
if err := e.ctx.NewTxn(); err != nil {
return err
}
defer func() { e.ctx.GetSessionVars().SetStatusFlag(mysql.ServerStatusInTrans, false) }()
}

switch x := e.Statement.(type) {
case *ast.UseStmt:
err = e.executeUse(x)
Expand All @@ -66,7 +75,7 @@ func (e *SimpleExec) Next(ctx context.Context, chk *chunk.Chunk) (err error) {
case *ast.RollbackStmt:
err = e.executeRollback(x)
case *ast.CreateUserStmt:
err = e.executeCreateUser(x)
err = e.executeCreateUser(ctx, x)
case *ast.AlterUserStmt:
err = e.executeAlterUser(x)
case *ast.DropUserStmt:
Expand Down Expand Up @@ -141,7 +150,7 @@ func (e *SimpleExec) executeRollback(s *ast.RollbackStmt) error {
return nil
}

func (e *SimpleExec) executeCreateUser(s *ast.CreateUserStmt) error {
func (e *SimpleExec) executeCreateUser(ctx context.Context, s *ast.CreateUserStmt) error {
users := make([]string, 0, len(s.Specs))
for _, spec := range s.Specs {
exists, err1 := userExists(e.ctx, spec.User.Username, spec.User.Hostname)
Expand All @@ -164,6 +173,7 @@ func (e *SimpleExec) executeCreateUser(s *ast.CreateUserStmt) error {
if len(users) == 0 {
return nil
}

sql := fmt.Sprintf(`INSERT INTO %s.%s (Host, User, Password) VALUES %s;`, mysql.SystemDB, mysql.UserTable, strings.Join(users, ", "))
_, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), sql)
if err != nil {
Expand Down Expand Up @@ -370,3 +380,11 @@ func (e *SimpleExec) executeDropStats(s *ast.DropStatsStmt) error {
}
return errors.Trace(h.Update(GetInfoSchema(e.ctx)))
}

func (e *SimpleExec) autoNewTxn() bool {
switch e.Statement.(type) {
case *ast.CreateUserStmt, *ast.AlterUserStmt, *ast.DropUserStmt:
return true
}
return false
}
30 changes: 30 additions & 0 deletions executor/simple_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,3 +296,33 @@ func (s *testSuite) TestDropStats(c *C) {
c.Assert(statsTbl.Pseudo, IsTrue)
h.Lease = 0
}

func (s *testSuite) TestStmtAutoNewTxn(c *C) {
// Some statements are like DDL, they commit the previous txn automically.
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")

// Fix issue https://github.com/pingcap/tidb/issues/10705
tk.MustExec("begin")
tk.MustExec("create user 'xxx'@'%';")
tk.MustExec("grant all privileges on *.* to 'xxx'@'%';")

tk.MustExec("create table auto_new (id int)")
tk.MustExec("begin")
tk.MustExec("insert into auto_new values (1)")
tk.MustExec("revoke all privileges on *.* from 'xxx'@'%'")
tk.MustExec("rollback") // insert statement has already committed
tk.MustQuery("select * from auto_new").Check(testkit.Rows("1"))

// Test the behavior when autocommit is false.
tk.MustExec("set autocommit = 0")
tk.MustExec("insert into auto_new values (2)")
tk.MustExec("create user 'yyy'@'%'")
tk.MustExec("rollback")
tk.MustQuery("select * from auto_new").Check(testkit.Rows("1", "2"))

tk.MustExec("drop user 'yyy'@'%'")
tk.MustExec("insert into auto_new values (3)")
tk.MustExec("rollback")
tk.MustQuery("select * from auto_new").Check(testkit.Rows("1", "2"))
}

0 comments on commit 1602766

Please sign in to comment.