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

ddl: support alter table auto_increment #5472

Merged
merged 12 commits into from
Dec 27, 2017
15 changes: 15 additions & 0 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Member

Choose a reason for hiding this comment

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

Why this is opt.UintValue-1?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is compatible with MySQL, if we set auto_increment = 10, the next auto increment insert column should take 10.

Copy link
Member Author

Choose a reason for hiding this comment

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

int64(opt.UintValue)-1 should be better. I'll fix it.

break
}
}
default:
// Nothing to do now.
}
Expand All @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

We'd better clear all TiDBs' cache.
So you can add a DDL Job likes https://github.com/pingcap/tidb/blob/master/ddl/ddl_api.go#L72
Then you can clear cache when we load the schema.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. PTAL @zimulala

}

func checkColumnConstraint(constraints []*ast.ColumnOption) error {
for _, constraint := range constraints {
switch constraint.Tp {
Expand Down
18 changes: 18 additions & 0 deletions ddl/ddl_db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
7 changes: 0 additions & 7 deletions plan/preprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
Expand Down
7 changes: 3 additions & 4 deletions plan/preprocess_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Copy link
Contributor

Choose a reason for hiding this comment

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

This SQL is different with SQL in 78. Should we add tests in ddl_db_test.go?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done! PTAL @zimulala

{"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},
Expand Down Expand Up @@ -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))
}
}