ddl: support alter table auto_increment #5472
Conversation
|
/run-all-tests |
ddl/ddl_api.go
Outdated
| case ast.AlterTableOption: | ||
| for _, opt := range spec.Options { | ||
| if opt.Tp == ast.TableOptionAutoIncrement { | ||
| err = d.RebaseAutoID(ctx, ident, int64(opt.UintValue-1)) |
There was a problem hiding this comment.
It is compatible with MySQL, if we set auto_increment = 10, the next auto increment insert column should take 10.
There was a problem hiding this comment.
int64(opt.UintValue)-1 should be better. I'll fix it.
ddl/ddl_api.go
Outdated
| if err != nil { | ||
| return errors.Trace(infoschema.ErrTableNotExists.GenByArgs(ident.Schema, ident.Name)) | ||
| } | ||
| return t.RebaseAutoID(ctx, newBase, true) |
There was a problem hiding this comment.
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.
|
/run-all-tests |
| job.State = model.JobStateCancelled | ||
| return ver, errors.Trace(err) | ||
| } | ||
| tblInfo.AutoIncID = newBase |
There was a problem hiding this comment.
Should we check that if the new auto-id is not less than current id?
There was a problem hiding this comment.
Yes, though it will be checked in storage, it should set a more properly info here. Fixed.
| if err != nil { | ||
| return errors.Trace(infoschema.ErrTableNotExists.GenByArgs(ident.Schema, ident.Name)) | ||
| } | ||
| autoIncID, err := t.Allocator(ctx).NextGlobalAutoID(t.Meta().ID) |
There was a problem hiding this comment.
We'd can put this logical to onRebaseAutoID。Because now autoIncID is n，its value may change when you rebase the global auto ID in onRebaseAutoID。
There was a problem hiding this comment.
There is another check in storage node to guarantee the value be set properly. The comparison here is trying to set a more reasonable value in table info.
There was a problem hiding this comment.
Seems that the global auto-id is not the current max id, for example, if the current max id is 1, the new base is 1000, then the next id inserted will be 5001. Is it desirable?
There was a problem hiding this comment.
It is our expected result. I'll add a test for it.
| {"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}, |
There was a problem hiding this comment.
This SQL is different with SQL in 78. Should we add tests in ddl_db_test.go?
|
LGTM |
|
/run-all-tests |
|
/run-all-tests |
|
/run-sqllogic-test |
1 similar comment
|
/run-sqllogic-test |
Fix #5034 PTAL @zimulala