From 84f2adbb6e2ec6d1234ac92c28745ae7b0fca69f Mon Sep 17 00:00:00 2001 From: Yu Shuaipeng Date: Fri, 22 Dec 2017 13:32:48 +0800 Subject: [PATCH 1/9] support alter table auto_increment --- ddl/ddl_api.go | 15 +++++++++++++++ ddl/ddl_db_test.go | 18 ++++++++++++++++++ plan/preprocess.go | 7 ------- plan/preprocess_test.go | 7 +++---- 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 7749d64f227c4..f0e2ff4638a79 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -885,6 +885,13 @@ func (d *ddl) AlterTable(ctx context.Context, ident ast.Ident, specs []*ast.Alte err = d.RenameTable(ctx, ident, newIdent) case ast.AlterTableDropPrimaryKey: err = ErrUnsupportedModifyPrimaryKey.GenByArgs("drop") + case ast.AlterTableOption: + for _, opt := range spec.Options { + if opt.Tp == ast.TableOptionAutoIncrement { + err = d.RebaseAutoID(ctx, ident, int64(opt.UintValue-1)) + break + } + } default: // Nothing to do now. } @@ -897,6 +904,14 @@ func (d *ddl) AlterTable(ctx context.Context, ident ast.Ident, specs []*ast.Alte return nil } +func (d *ddl) RebaseAutoID(ctx context.Context, ident ast.Ident, newBase int64) error { + t, err := d.infoHandle.Get().TableByName(ident.Schema, ident.Name) + if err != nil { + return errors.Trace(infoschema.ErrTableNotExists.GenByArgs(ident.Schema, ident.Name)) + } + return t.RebaseAutoID(ctx, newBase, true) +} + func checkColumnConstraint(constraints []*ast.ColumnOption) error { for _, constraint := range constraints { switch constraint.Tp { diff --git a/ddl/ddl_db_test.go b/ddl/ddl_db_test.go index 5277eba8739fa..c920a72a64226 100644 --- a/ddl/ddl_db_test.go +++ b/ddl/ddl_db_test.go @@ -1676,3 +1676,21 @@ func (s *testDBSuite) TestComment(c *C) { s.tk.MustExec("drop table if exists ct, ct1") } + +func (s *testDBSuite) TestRebaseAutoID(c *C) { + s.tk = testkit.NewTestKit(c, s.store) + s.tk.MustExec("use " + s.schemaName) + + s.tk.MustExec("drop database if exists tidb;") + s.tk.MustExec("create database tidb;") + s.tk.MustExec("use tidb;") + s.tk.MustExec("create table tidb.test (a int auto_increment primary key, b int);") + s.tk.MustExec("insert tidb.test values (null, 1);") + s.tk.MustQuery("select * from tidb.test").Check(testkit.Rows("1 1")) + s.tk.MustExec("alter table tidb.test auto_increment = 10;") + s.tk.MustExec("insert tidb.test values (null, 1);") + s.tk.MustQuery("select * from tidb.test").Check(testkit.Rows("1 1", "10 1")) + s.tk.MustExec("alter table tidb.test auto_increment = 5;") + s.tk.MustExec("insert tidb.test values (null, 1);") + s.tk.MustQuery("select * from tidb.test").Check(testkit.Rows("1 1", "10 1", "11 1")) +} diff --git a/plan/preprocess.go b/plan/preprocess.go index 9ff624098ea19..04a78adbbdbe0 100644 --- a/plan/preprocess.go +++ b/plan/preprocess.go @@ -333,13 +333,6 @@ func (p *preprocessor) checkAlterTableGrammar(stmt *ast.AlterTableStmt) { default: // Nothing to do now. } - case ast.AlterTableOption: - for _, opt := range spec.Options { - if opt.Tp == ast.TableOptionAutoIncrement { - p.err = ErrAlterAutoID - return - } - } default: // Nothing to do now. } diff --git a/plan/preprocess_test.go b/plan/preprocess_test.go index 390b2a5011b50..c85c5f81c4daf 100644 --- a/plan/preprocess_test.go +++ b/plan/preprocess_test.go @@ -75,9 +75,8 @@ func (s *testValidatorSuite) TestValidator(c *C) { errors.New("[schema:1068]Multiple primary key defined")}, {"create table t(c1 int not null, c2 int not null, primary key(c1), primary key(c2))", true, errors.New("[schema:1068]Multiple primary key defined")}, - {"alter table t auto_increment=1", true, errors.New("[autoid:3]No support for setting auto_increment using alter_table")}, - {"alter table t add column c int auto_increment key, auto_increment=10", true, - errors.New("[autoid:3]No support for setting auto_increment using alter_table")}, + {"alter table t auto_increment=1", true, nil}, + {"alter table t add column c int auto_increment key, auto_increment=10", true, nil}, {"alter table t add column c int auto_increment key", true, nil}, {"alter table t add column char4294967295 char(255)", true, nil}, {"create table t (c float(53))", true, nil}, @@ -184,6 +183,6 @@ func (s *testValidatorSuite) TestValidator(c *C) { c.Assert(stmts, HasLen, 1) stmt := stmts[0] err = plan.Preprocess(ctx, stmt, is, tt.inPrepare) - c.Assert(terror.ErrorEqual(err, tt.err), IsTrue) + c.Assert(terror.ErrorEqual(err, tt.err), IsTrue, Commentf("sql: %s", tt.sql)) } } From 20da91929ddeaf8c60a4c43424ea28e63d94390d Mon Sep 17 00:00:00 2001 From: Yu Shuaipeng Date: Fri, 22 Dec 2017 14:23:42 +0800 Subject: [PATCH 2/9] tiny fix --- ddl/ddl_api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index f0e2ff4638a79..b7791d8127ca1 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -888,7 +888,7 @@ func (d *ddl) AlterTable(ctx context.Context, ident ast.Ident, specs []*ast.Alte case ast.AlterTableOption: for _, opt := range spec.Options { if opt.Tp == ast.TableOptionAutoIncrement { - err = d.RebaseAutoID(ctx, ident, int64(opt.UintValue-1)) + err = d.RebaseAutoID(ctx, ident, int64(opt.UintValue)-1) break } } From 7731152e98f9857f4a4771fa965f8853b63f5646 Mon Sep 17 00:00:00 2001 From: Yu Shuaipeng Date: Mon, 25 Dec 2017 11:05:24 +0800 Subject: [PATCH 3/9] add a job for rebase auto ID --- ddl/ddl_api.go | 30 +++++++++++++++++++++++++++--- ddl/ddl_worker.go | 2 ++ ddl/table.go | 24 ++++++++++++++++++++++++ infoschema/builder.go | 2 +- model/ddl.go | 3 +++ 5 files changed, 57 insertions(+), 4 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index b7791d8127ca1..4e26ddd318142 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -888,7 +888,7 @@ func (d *ddl) AlterTable(ctx context.Context, ident ast.Ident, specs []*ast.Alte case ast.AlterTableOption: for _, opt := range spec.Options { if opt.Tp == ast.TableOptionAutoIncrement { - err = d.RebaseAutoID(ctx, ident, int64(opt.UintValue)-1) + err = d.RebaseAutoID(ctx, ident, int64(opt.UintValue)) break } } @@ -905,11 +905,35 @@ func (d *ddl) AlterTable(ctx context.Context, ident ast.Ident, specs []*ast.Alte } func (d *ddl) RebaseAutoID(ctx context.Context, ident ast.Ident, newBase int64) error { - t, err := d.infoHandle.Get().TableByName(ident.Schema, ident.Name) + is := d.GetInformationSchema() + schema, ok := is.SchemaByName(ident.Schema) + if !ok { + return infoschema.ErrDatabaseNotExists.GenByArgs(ident.Schema) + } + t, err := is.TableByName(ident.Schema, ident.Name) if err != nil { return errors.Trace(infoschema.ErrTableNotExists.GenByArgs(ident.Schema, ident.Name)) } - return t.RebaseAutoID(ctx, newBase, true) + job := &model.Job{ + SchemaID: schema.ID, + TableID: t.Meta().ID, + Type: model.ActionRebaseAutoID, + BinlogInfo: &model.HistoryInfo{}, + Args: []interface{}{newBase}, + } + err = d.doDDLJob(ctx, job) + err = d.callHookOnChanged(err) + if err != nil { + return errors.Trace(err) + } + // The operation of the minus 1 to make sure that the current value doesn't be used, + // the next Alloc operation will get this value. + // Its behavior is consistent with MySQL. + err = t.RebaseAutoID(ctx, newBase-1, true) + if err != nil { + return errors.Trace(err) + } + return nil } func checkColumnConstraint(constraints []*ast.ColumnOption) error { diff --git a/ddl/ddl_worker.go b/ddl/ddl_worker.go index c36672a8b6d43..dc3fb3da5f9bf 100644 --- a/ddl/ddl_worker.go +++ b/ddl/ddl_worker.go @@ -299,6 +299,8 @@ func (d *ddl) runDDLJob(t *meta.Meta, job *model.Job) (ver int64) { ver, err = d.onDropForeignKey(t, job) case model.ActionTruncateTable: ver, err = d.onTruncateTable(t, job) + case model.ActionRebaseAutoID: + ver, err = d.onRebaseAutoID(t, job) case model.ActionRenameTable: ver, err = d.onRenameTable(t, job) case model.ActionSetDefaultValue: diff --git a/ddl/table.go b/ddl/table.go index 0afefc5917258..61693b21a5d9c 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -220,6 +220,30 @@ func (d *ddl) onTruncateTable(t *meta.Meta, job *model.Job) (ver int64, _ error) return ver, nil } +func (d *ddl) onRebaseAutoID(t *meta.Meta, job *model.Job) (ver int64, _ error) { + schemaID := job.SchemaID + var newBase int64 + err := job.DecodeArgs(&newBase) + if err != nil { + job.State = model.JobStateCancelled + return ver, errors.Trace(err) + } + tblInfo, err := getTableInfo(t, job, schemaID) + if err != nil { + job.State = model.JobStateCancelled + return ver, errors.Trace(err) + } + tblInfo.AutoIncID = newBase + job.State = model.JobStateDone + job.BinlogInfo.AddTableInfo(ver, tblInfo) + ver, err = updateTableInfo(t, job, tblInfo, job.SchemaState) + if err != nil { + job.State = model.JobStateCancelled + return ver, errors.Trace(err) + } + return ver, nil +} + func (d *ddl) onRenameTable(t *meta.Meta, job *model.Job) (ver int64, _ error) { var oldSchemaID int64 var tableName model.CIStr diff --git a/infoschema/builder.go b/infoschema/builder.go index 9e4cb6db95f0a..d9f53c09c1e87 100644 --- a/infoschema/builder.go +++ b/infoschema/builder.go @@ -73,7 +73,7 @@ func (b *Builder) ApplyDiff(m *meta.Meta, diff *model.SchemaDiff) ([]int64, erro // We try to reuse the old allocator, so the cached auto ID can be reused. var alloc autoid.Allocator if tableIDIsValid(oldTableID) { - if oldTableID == newTableID && diff.Type != model.ActionRenameTable { + if oldTableID == newTableID && diff.Type != model.ActionRenameTable && diff.Type != model.ActionRebaseAutoID { alloc, _ = b.is.AllocByID(oldTableID) } if diff.Type == model.ActionRenameTable { diff --git a/model/ddl.go b/model/ddl.go index 47e1240c809da..3880aeaafb8df 100644 --- a/model/ddl.go +++ b/model/ddl.go @@ -40,6 +40,7 @@ const ( ActionDropForeignKey ActionTruncateTable ActionModifyColumn + ActionRebaseAutoID ActionRenameTable ActionSetDefaultValue ) @@ -70,6 +71,8 @@ func (action ActionType) String() string { return "truncate table" case ActionModifyColumn: return "modify column" + case ActionRebaseAutoID: + return "rebase auto_increment ID" case ActionRenameTable: return "rename table" case ActionSetDefaultValue: From 2041e2e5c8502275290f3f8d0fc893a47618e4a4 Mon Sep 17 00:00:00 2001 From: Yu Shuaipeng Date: Mon, 25 Dec 2017 12:44:25 +0800 Subject: [PATCH 4/9] mv rebase in the job --- ddl/ddl_api.go | 12 +----------- ddl/ddl_db_test.go | 6 +++--- ddl/table.go | 15 ++++++++++++++- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 4e26ddd318142..392311148e68f 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -923,17 +923,7 @@ func (d *ddl) RebaseAutoID(ctx context.Context, ident ast.Ident, newBase int64) } err = d.doDDLJob(ctx, job) err = d.callHookOnChanged(err) - if err != nil { - return errors.Trace(err) - } - // The operation of the minus 1 to make sure that the current value doesn't be used, - // the next Alloc operation will get this value. - // Its behavior is consistent with MySQL. - err = t.RebaseAutoID(ctx, newBase-1, true) - if err != nil { - return errors.Trace(err) - } - return nil + return errors.Trace(err) } func checkColumnConstraint(constraints []*ast.ColumnOption) error { diff --git a/ddl/ddl_db_test.go b/ddl/ddl_db_test.go index c920a72a64226..a1d276df8dd63 100644 --- a/ddl/ddl_db_test.go +++ b/ddl/ddl_db_test.go @@ -1687,10 +1687,10 @@ func (s *testDBSuite) TestRebaseAutoID(c *C) { s.tk.MustExec("create table tidb.test (a int auto_increment primary key, b int);") s.tk.MustExec("insert tidb.test values (null, 1);") s.tk.MustQuery("select * from tidb.test").Check(testkit.Rows("1 1")) - s.tk.MustExec("alter table tidb.test auto_increment = 10;") + s.tk.MustExec("alter table tidb.test auto_increment = 6000;") s.tk.MustExec("insert tidb.test values (null, 1);") - s.tk.MustQuery("select * from tidb.test").Check(testkit.Rows("1 1", "10 1")) + s.tk.MustQuery("select * from tidb.test").Check(testkit.Rows("1 1", "6000 1")) s.tk.MustExec("alter table tidb.test auto_increment = 5;") s.tk.MustExec("insert tidb.test values (null, 1);") - s.tk.MustQuery("select * from tidb.test").Check(testkit.Rows("1 1", "10 1", "11 1")) + s.tk.MustQuery("select * from tidb.test").Check(testkit.Rows("1 1", "6000 1", "11000 1")) } diff --git a/ddl/table.go b/ddl/table.go index 61693b21a5d9c..9931aeeb9a134 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -234,9 +234,22 @@ func (d *ddl) onRebaseAutoID(t *meta.Meta, job *model.Job) (ver int64, _ error) return ver, errors.Trace(err) } tblInfo.AutoIncID = newBase + tbl, err := d.getTable(schemaID, tblInfo) + if err != nil { + job.State = model.JobStateCancelled + return ver, errors.Trace(err) + } + // The operation of the minus 1 to make sure that the current value doesn't be used, + // the next Alloc operation will get this value. + // Its behavior is consistent with MySQL. + err = tbl.RebaseAutoID(nil, tblInfo.AutoIncID-1, false) + if err != nil { + job.State = model.JobStateCancelled + return ver, errors.Trace(err) + } job.State = model.JobStateDone job.BinlogInfo.AddTableInfo(ver, tblInfo) - ver, err = updateTableInfo(t, job, tblInfo, job.SchemaState) + ver, err = updateTableInfo(t, job, tblInfo, tblInfo.State) if err != nil { job.State = model.JobStateCancelled return ver, errors.Trace(err) From 02018161274ce165ee3b5c61e6b2be387fb4f03d Mon Sep 17 00:00:00 2001 From: Yu Shuaipeng Date: Mon, 25 Dec 2017 13:38:40 +0800 Subject: [PATCH 5/9] address comments --- ddl/table.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ddl/table.go b/ddl/table.go index 9931aeeb9a134..ac5145d9448b9 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -233,7 +233,9 @@ func (d *ddl) onRebaseAutoID(t *meta.Meta, job *model.Job) (ver int64, _ error) job.State = model.JobStateCancelled return ver, errors.Trace(err) } - tblInfo.AutoIncID = newBase + if newBase > tblInfo.AutoIncID { + tblInfo.AutoIncID = newBase + } tbl, err := d.getTable(schemaID, tblInfo) if err != nil { job.State = model.JobStateCancelled From 1244fbf099aeabdfe0cfc565fac1f2fc534271d1 Mon Sep 17 00:00:00 2001 From: Yu Shuaipeng Date: Mon, 25 Dec 2017 13:53:12 +0800 Subject: [PATCH 6/9] tiny fix --- ddl/ddl_api.go | 6 ++++++ ddl/table.go | 4 +--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 392311148e68f..7a3a307a85294 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -23,6 +23,7 @@ import ( "strings" "time" + "github.com/cznic/mathutil" "github.com/juju/errors" "github.com/pingcap/tidb/ast" "github.com/pingcap/tidb/context" @@ -914,6 +915,11 @@ func (d *ddl) RebaseAutoID(ctx context.Context, ident ast.Ident, newBase int64) if err != nil { return errors.Trace(infoschema.ErrTableNotExists.GenByArgs(ident.Schema, ident.Name)) } + autoIncID, err := t.Allocator(ctx).NextGlobalAutoID(t.Meta().ID) + if err != nil { + return errors.Trace(err) + } + newBase = mathutil.MaxInt64(newBase, autoIncID) job := &model.Job{ SchemaID: schema.ID, TableID: t.Meta().ID, diff --git a/ddl/table.go b/ddl/table.go index ac5145d9448b9..9931aeeb9a134 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -233,9 +233,7 @@ func (d *ddl) onRebaseAutoID(t *meta.Meta, job *model.Job) (ver int64, _ error) job.State = model.JobStateCancelled return ver, errors.Trace(err) } - if newBase > tblInfo.AutoIncID { - tblInfo.AutoIncID = newBase - } + tblInfo.AutoIncID = newBase tbl, err := d.getTable(schemaID, tblInfo) if err != nil { job.State = model.JobStateCancelled From 115dbe60f7e728c69117e667f365029c67d3e910 Mon Sep 17 00:00:00 2001 From: Yu Shuaipeng Date: Tue, 26 Dec 2017 11:18:59 +0800 Subject: [PATCH 7/9] address comments --- ddl/ddl_db_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ddl/ddl_db_test.go b/ddl/ddl_db_test.go index a1d276df8dd63..2a620f46c5a24 100644 --- a/ddl/ddl_db_test.go +++ b/ddl/ddl_db_test.go @@ -1693,4 +1693,7 @@ func (s *testDBSuite) TestRebaseAutoID(c *C) { s.tk.MustExec("alter table tidb.test auto_increment = 5;") s.tk.MustExec("insert tidb.test values (null, 1);") s.tk.MustQuery("select * from tidb.test").Check(testkit.Rows("1 1", "6000 1", "11000 1")) + + s.tk.MustExec("create table tidb.test2 (a int);") + s.testErrorCode(c,"alter table tidb.test2 add column b int auto_increment key, auto_increment=10;", tmysql.ErrUnknown) } From 06f0c3a3d58071fafadda497730ba25e396d8d94 Mon Sep 17 00:00:00 2001 From: Yu Shuaipeng Date: Tue, 26 Dec 2017 11:20:50 +0800 Subject: [PATCH 8/9] format --- ddl/ddl_db_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/ddl_db_test.go b/ddl/ddl_db_test.go index 2a620f46c5a24..6cf03556b0a76 100644 --- a/ddl/ddl_db_test.go +++ b/ddl/ddl_db_test.go @@ -1695,5 +1695,5 @@ func (s *testDBSuite) TestRebaseAutoID(c *C) { s.tk.MustQuery("select * from tidb.test").Check(testkit.Rows("1 1", "6000 1", "11000 1")) s.tk.MustExec("create table tidb.test2 (a int);") - s.testErrorCode(c,"alter table tidb.test2 add column b int auto_increment key, auto_increment=10;", tmysql.ErrUnknown) + s.testErrorCode(c, "alter table tidb.test2 add column b int auto_increment key, auto_increment=10;", tmysql.ErrUnknown) } From 6c60f0c487055d0a03282a5b8c49837d15477cb7 Mon Sep 17 00:00:00 2001 From: Yu Shuaipeng Date: Wed, 27 Dec 2017 11:21:10 +0800 Subject: [PATCH 9/9] add a test --- ddl/ddl_db_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ddl/ddl_db_test.go b/ddl/ddl_db_test.go index 6cf03556b0a76..cdc9cea42a144 100644 --- a/ddl/ddl_db_test.go +++ b/ddl/ddl_db_test.go @@ -1694,6 +1694,13 @@ func (s *testDBSuite) TestRebaseAutoID(c *C) { s.tk.MustExec("insert tidb.test values (null, 1);") s.tk.MustQuery("select * from tidb.test").Check(testkit.Rows("1 1", "6000 1", "11000 1")) + // Current range for table test is [11000, 15999]. + // Though it does not have a tuple "a = 15999", its global next auto increment id should be 16000. + // Anyway it is not compatible with MySQL. + s.tk.MustExec("alter table tidb.test auto_increment = 12000;") + s.tk.MustExec("insert tidb.test values (null, 1);") + s.tk.MustQuery("select * from tidb.test").Check(testkit.Rows("1 1", "6000 1", "11000 1", "16000 1")) + s.tk.MustExec("create table tidb.test2 (a int);") s.testErrorCode(c, "alter table tidb.test2 add column b int auto_increment key, auto_increment=10;", tmysql.ErrUnknown) }