From 8e9c035178dd6182fd79f25100209e5daac82a6e Mon Sep 17 00:00:00 2001 From: Yu Shuaipeng Date: Tue, 9 Oct 2018 12:43:28 +0800 Subject: [PATCH 1/2] (cherry-pick) executor: only rebase auto increment ID when needed --- executor/write.go | 48 ++++++++++++++++++++++++++++-------------- executor/write_test.go | 33 +++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/executor/write.go b/executor/write.go index caa98c8ffaebf..60ec147d0d1b0 100644 --- a/executor/write.go +++ b/executor/write.go @@ -25,6 +25,7 @@ import ( "github.com/pingcap/tidb/model" "github.com/pingcap/tidb/mysql" "github.com/pingcap/tidb/sessionctx" + "github.com/pingcap/tidb/sessionctx/stmtctx" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/table/tables" "github.com/pingcap/tidb/tablecodec" @@ -77,27 +78,23 @@ func updateRecord(ctx sessionctx.Context, h int64, oldData, newData []types.Datu newData[i] = v } - // Rebase auto increment id if the field is changed. - if mysql.HasAutoIncrementFlag(col.Flag) { - if newData[i].IsNull() { - return false, handleChanged, newHandle, 0, - errors.Errorf("Column '%v' cannot be null", col.Name.O) - } - val, errTI := newData[i].ToInt64(sc) - if errTI != nil { - return false, handleChanged, newHandle, 0, errors.Trace(errTI) - } - lastInsertID = uint64(val) - err := t.RebaseAutoID(ctx, val, true) - if err != nil { - return false, handleChanged, newHandle, 0, errors.Trace(err) - } - } cmp, err := newData[i].CompareDatum(sc, &oldData[i]) if err != nil { return false, handleChanged, newHandle, 0, errors.Trace(err) } if cmp != 0 { + if mysql.HasAutoIncrementFlag(col.Flag) { + val, err := extractLastInsertID(newData[i], col.Name.O, sc) + if err != nil { + return false, handleChanged, newHandle, 0, errors.Trace(err) + } + lastInsertID = uint64(val) + // Rebase auto increment id if the field is changed. + err = t.RebaseAutoID(ctx, val, true) + if err != nil { + return false, handleChanged, newHandle, 0, errors.Trace(err) + } + } changed = true modified[i] = true if col.IsPKHandleColumn(t.Meta()) { @@ -105,6 +102,14 @@ func updateRecord(ctx sessionctx.Context, h int64, oldData, newData []types.Datu newHandle = newData[i].GetInt64() } } else { + // Update the lastInsertID. + if mysql.HasAutoIncrementFlag(col.Flag) { + val, err := extractLastInsertID(newData[i], col.Name.O, sc) + if err != nil { + return false, handleChanged, newHandle, 0, errors.Trace(err) + } + lastInsertID = uint64(val) + } if mysql.HasOnUpdateNowFlag(col.Flag) && modified[i] { // It's for "UPDATE t SET ts = ts" and ts is a timestamp. onUpdateSpecified[i] = true @@ -185,6 +190,17 @@ func updateRecord(ctx sessionctx.Context, h int64, oldData, newData []types.Datu return true, handleChanged, newHandle, lastInsertID, nil } +func extractLastInsertID(d types.Datum, colName string, sc *stmtctx.StatementContext) (int64, error) { + if d.IsNull() { + return 0, errors.Errorf("Column '%v' cannot be null", colName) + } + val, err := d.ToInt64(sc) + if err != nil { + return 0, errors.Trace(err) + } + return val, nil +} + // DeleteExec represents a delete executor. // See https://dev.mysql.com/doc/refman/5.7/en/delete.html type DeleteExec struct { diff --git a/executor/write_test.go b/executor/write_test.go index b6a25d871ddf7..6ee726942dd5b 100644 --- a/executor/write_test.go +++ b/executor/write_test.go @@ -1647,3 +1647,36 @@ func (s *testSuite) TestInsertDateTimeWithTimeZone(c *C) { `1 1970-01-01 09:20:34`, )) } + +// 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) { + 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;`) + 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")) +} From 5bec47fad7fbc623e0012b9108b02e52ac892f36 Mon Sep 17 00:00:00 2001 From: Yu Shuaipeng Date: Tue, 9 Oct 2018 12:55:16 +0800 Subject: [PATCH 2/2] tiny change --- executor/write.go | 44 +++++++++++++++----------------------------- 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/executor/write.go b/executor/write.go index 60ec147d0d1b0..286cc151969bf 100644 --- a/executor/write.go +++ b/executor/write.go @@ -25,7 +25,6 @@ import ( "github.com/pingcap/tidb/model" "github.com/pingcap/tidb/mysql" "github.com/pingcap/tidb/sessionctx" - "github.com/pingcap/tidb/sessionctx/stmtctx" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/table/tables" "github.com/pingcap/tidb/tablecodec" @@ -82,19 +81,25 @@ func updateRecord(ctx sessionctx.Context, h int64, oldData, newData []types.Datu if err != nil { return false, handleChanged, newHandle, 0, errors.Trace(err) } - if cmp != 0 { - if mysql.HasAutoIncrementFlag(col.Flag) { - val, err := extractLastInsertID(newData[i], col.Name.O, sc) - if err != nil { - return false, handleChanged, newHandle, 0, errors.Trace(err) - } - lastInsertID = uint64(val) - // Rebase auto increment id if the field is changed. - err = t.RebaseAutoID(ctx, val, true) + if mysql.HasAutoIncrementFlag(col.Flag) { + if newData[i].IsNull() { + return false, handleChanged, newHandle, 0, + errors.Errorf("Column '%v' cannot be null", col.Name.O) + } + val, errTI := newData[i].ToInt64(sc) + if errTI != nil { + return false, handleChanged, newHandle, 0, errors.Trace(errTI) + } + lastInsertID = uint64(val) + // Rebase auto increment id if the field is changed. + if cmp != 0 { + err := t.RebaseAutoID(ctx, val, true) if err != nil { return false, handleChanged, newHandle, 0, errors.Trace(err) } } + } + if cmp != 0 { changed = true modified[i] = true if col.IsPKHandleColumn(t.Meta()) { @@ -102,14 +107,6 @@ func updateRecord(ctx sessionctx.Context, h int64, oldData, newData []types.Datu newHandle = newData[i].GetInt64() } } else { - // Update the lastInsertID. - if mysql.HasAutoIncrementFlag(col.Flag) { - val, err := extractLastInsertID(newData[i], col.Name.O, sc) - if err != nil { - return false, handleChanged, newHandle, 0, errors.Trace(err) - } - lastInsertID = uint64(val) - } if mysql.HasOnUpdateNowFlag(col.Flag) && modified[i] { // It's for "UPDATE t SET ts = ts" and ts is a timestamp. onUpdateSpecified[i] = true @@ -190,17 +187,6 @@ func updateRecord(ctx sessionctx.Context, h int64, oldData, newData []types.Datu return true, handleChanged, newHandle, lastInsertID, nil } -func extractLastInsertID(d types.Datum, colName string, sc *stmtctx.StatementContext) (int64, error) { - if d.IsNull() { - return 0, errors.Errorf("Column '%v' cannot be null", colName) - } - val, err := d.ToInt64(sc) - if err != nil { - return 0, errors.Trace(err) - } - return val, nil -} - // DeleteExec represents a delete executor. // See https://dev.mysql.com/doc/refman/5.7/en/delete.html type DeleteExec struct {