Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

executor: only rebase auto increment ID when needed #7515

Merged
merged 8 commits into from
Aug 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions executor/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,27 +71,27 @@ func updateRecord(ctx sessionctx.Context, h int64, oldData, newData []types.Datu
return false, handleChanged, newHandle, errors.Trace(err)
}
}
// Rebase auto increment id if the field is changed.
if mysql.HasAutoIncrementFlag(col.Flag) {
if newData[i].IsNull() {
return false, handleChanged, newHandle, table.ErrColumnCantNull.GenByArgs(col.Name)
}
val, errTI := newData[i].ToInt64(sc)
if errTI != nil {
return false, handleChanged, newHandle, errors.Trace(errTI)
}
err := t.RebaseAutoID(ctx, val, true)
if err != nil {
return false, handleChanged, newHandle, errors.Trace(err)
}
}
cmp, err := newData[i].CompareDatum(sc, &oldData[i])
if err != nil {
return false, handleChanged, newHandle, errors.Trace(err)
}
if cmp != 0 {
changed = true
modified[i] = true
// Rebase auto increment id if the field is changed.
if mysql.HasAutoIncrementFlag(col.Flag) {
if newData[i].IsNull() {
return false, handleChanged, newHandle, table.ErrColumnCantNull.GenByArgs(col.Name)
}
val, errTI := newData[i].ToInt64(sc)
if errTI != nil {
return false, handleChanged, newHandle, errors.Trace(errTI)
}
err := t.RebaseAutoID(ctx, val, true)
if err != nil {
return false, handleChanged, newHandle, errors.Trace(err)
}
}
if col.IsPKHandleColumn(t.Meta()) {
handleChanged = true
newHandle = newData[i].GetInt64()
Expand Down
33 changes: 33 additions & 0 deletions executor/write_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2021,3 +2021,36 @@ func (s *testSuite) TestReplaceLog(c *C) {

tk.MustQuery(`admin cleanup index testLog b;`).Check(testkit.Rows("1"))
}

// For issue 7422.
// There is no need to do the rebase when updating a record if the auto-increment ID not changed.
// This could make the auto ID increasing speed slower.
func (s *testSuite) TestRebaseIfNeeded(c *C) {
Copy link
Contributor

@lysu lysu Aug 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe need add insert on duplicate key update testcase, and check last_insert_id() value.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already have the cases at line 595 to 627.

Copy link
Contributor

@lysu lysu Aug 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I means rebase auto_increment in on duplicate key update, current case seems not update id? e.g. use duplicate on second unique index then update primary key...

tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec(`create table t (a int not null primary key auto_increment, b int unique key);`)
tk.MustExec(`insert into t (b) values (1);`)

s.ctx = mock.NewContext()
s.ctx.Store = s.store
tbl, err := s.domain.InfoSchema().TableByName(model.NewCIStr("test"), model.NewCIStr("t"))
c.Assert(err, IsNil)
c.Assert(s.ctx.NewTxn(), IsNil)
// AddRecord directly here will skip to rebase the auto ID in the insert statement,
// which could simulate another TiDB adds a large auto ID.
_, err = tbl.AddRecord(s.ctx, types.MakeDatums(30001, 2), false)
c.Assert(err, IsNil)
c.Assert(s.ctx.Txn().Commit(context.Background()), IsNil)

tk.MustExec(`update t set b = 3 where a = 30001;`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add comments to this case,

tk.MustExec(`insert into t (b) values (4);`)
tk.MustQuery(`select a from t where b = 4;`).Check(testkit.Rows("2"))

tk.MustExec(`insert into t set b = 3 on duplicate key update a = a;`)
tk.MustExec(`insert into t (b) values (5);`)
tk.MustQuery(`select a from t where b = 5;`).Check(testkit.Rows("4"))

tk.MustExec(`insert into t set b = 3 on duplicate key update a = a + 1;`)
tk.MustExec(`insert into t (b) values (6);`)
tk.MustQuery(`select a from t where b = 6;`).Check(testkit.Rows("30003"))
}