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.

Sign up for GitHub

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

Jump to bottom

ddl: support multi-schema change for drop columns #16

Merged
merged 2 commits into from
Mar 10, 2022
Merged

ddl: support multi-schema change for drop columns #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34a7a16
ddl: support multi-schema change for drop columns
tangenta Mar 9, 2022
812523d
address comment
tangenta Mar 10, 2022
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
7 changes: 7 additions & 0 deletions ddl/column.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,13 @@ func onDropColumn(t *meta.Meta, job *model.Job) (ver int64, _ error) {
if err != nil {
return ver, errors.Trace(err)
}
if job.MultiSchemaInfo != nil && job.MultiSchemaInfo.Revertible {
job.MarkNonRevertible()
ver, err = updateVersionAndTableInfoWithCheck(t, job, tblInfo, false)
if err != nil {
return ver, errors.Trace(err)
}
}

originalState := colInfo.State
switch colInfo.State {
Expand Down
10 changes: 5 additions & 5 deletions ddl/ddl_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3777,7 +3777,7 @@ func (d *ddl) DropColumn(ctx sessionctx.Context, ti ast.Ident, spec *ast.AlterTa
return nil
}
colName := spec.OldColumnName.Name
err = checkDropVisibleColumnCnt(t, 1)
err = checkVisibleColumnCnt(t, 0, 1)
if err != nil {
return err
}
Expand Down Expand Up @@ -3850,7 +3850,7 @@ func (d *ddl) DropColumns(ctx sessionctx.Context, ti ast.Ident, specs []*ast.Alt
return ErrCantRemoveAllFields.GenWithStack("can't drop all columns in table %s",
tblInfo.Name)
}
err = checkDropVisibleColumnCnt(t, len(colNames))
err = checkVisibleColumnCnt(t, 0, len(colNames))
if err != nil {
return err
}
Expand Down Expand Up @@ -3904,14 +3904,14 @@ func checkIsDroppableColumn(ctx sessionctx.Context, t table.Table, spec *ast.Alt
return true, nil
}

func checkDropVisibleColumnCnt(t table.Table, columnCnt int) error {
func checkVisibleColumnCnt(t table.Table, addCnt, dropCnt int) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about combine checkDropVisibleColumnCnt and checkAddColumnTooManyColumns

Copy link
Owner Author

Choose a reason for hiding this comment

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

I prefer to separate them because they make different check.

tblInfo := t.Meta()
visibleColumCnt := 0
visibleColumCnt := addCnt
for _, column := range tblInfo.Columns {
if !column.Hidden {
visibleColumCnt++
}
if visibleColumCnt > columnCnt {
if visibleColumCnt > dropCnt {
return nil
}
}
Expand Down
2 changes: 1 addition & 1 deletion ddl/multi_schema_change.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func checkMultiSchemaInfo(info *model.MultiSchemaInfo, t table.Table) error {
return err
}

err = checkDropVisibleColumnCnt(t, len(info.DropColumns))
err = checkVisibleColumnCnt(t, len(info.AddColumns), len(info.DropColumns))
if err != nil {
return err
}
Expand Down
58 changes: 53 additions & 5 deletions ddl/multi_schema_change_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,20 @@ func TestMultiSchemaChangeAddColumns(t *testing.T) {
tk.MustExec("use test")
tk.MustExec("set @@global.tidb_enable_change_multi_schema = 1")

// Test add multiple columns in multiple specs.
tk.MustExec("create table t (a int);")
tk.MustExec("insert into t values (1);")
tk.MustExec("alter table t add column b int default 2, add column c int default 3;")
tk.MustQuery("select * from t;").Check(testkit.Rows("1 2 3"))

// Test add multiple columns in one spec.
tk.MustExec("drop table t;")
tk.MustExec("create table t (a int);")
tk.MustExec("insert into t values (1);")
tk.MustExec("alter table t add column (b int default 2, c int default 3);")
tk.MustQuery("select * from t;").Check(testkit.Rows("1 2 3"))

// Test referencing previous column in multi-schema change is not supported.
tk.MustExec("drop table t;")
tk.MustExec("create table t (a int);")
tk.MustGetErrCode("alter table t add column b int after a, add column c int after b", errno.ErrBadField)
Expand All @@ -54,17 +57,62 @@ func TestMultiSchemaChangeAddColumns(t *testing.T) {
add column e int default 5 after b,
add column f int default 6 after b;`)
tk.MustQuery("select * from t;").Check(testkit.Rows("4 1 2 6 5 3"))

// Test [if not exists] for adding columns.
tk.MustExec("drop table if exists t;")
tk.MustExec("create table t (a int default 1);")
tk.MustExec("insert into t values ();")
tk.MustExec("alter table t add column b int default 2, add column if not exists a int;")
tk.MustQuery("show warnings;").Check(testkit.Rows("Note 1060 Duplicate column name 'a'"))
tk.MustQuery("select * from t;").Check(testkit.Rows("1 2"))
}

func TestMultiSchemaDropColumns(t *testing.T) {
func TestMultiSchemaChangeDropColumns(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("set @@global.tidb_enable_change_multi_schema = 1")
tk.MustExec("use test;")
tk.MustExec("set @@global.tidb_enable_change_multi_schema = 1;")

tk.MustExec("create table t (a int, b int);")
tk.MustGetErrCode("alter table t drop column a, drop column b;", errno.ErrCantRemoveAllFields)

tk.MustExec("drop table if exists t;")
tk.MustExec("create table t (a int, b int, c int, d int, e int);")
tk.MustExec("insert into t values (1, 2, 3, 4, 5);")
tk.MustExec("alter table t drop column a, drop column d, drop column b;")
tk.MustQuery("select * from t;").Check(testkit.Rows("3 5"))
}

func TestMultiSchemaChangeAddDropColumns(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test;")
tk.MustExec("set @@global.tidb_enable_change_multi_schema = 1;")

// [a, b] -> [+c, -a, +d, -b] -> [c, d]
tk.MustExec("drop table if exists t;")
tk.MustExec("create table t (a int default 1, b int default 2);")
tk.MustExec("insert into t values ();")
tk.MustExec("alter table t add column c int default 3, drop column a, add column d int default 4, drop column b;")
tk.MustQuery("select * from t;").Check(testkit.Rows("3 4"))

// [a, b] -> [-a, -b, +c, +d] -> [c, d]
tk.MustExec("drop table if exists t;")
tk.MustExec("create table t (a int default 1, b int default 2);")
tk.MustExec("insert into t values ();")
tk.MustExec("alter table t drop column a, drop column b, add column c int default 3, add column d int default 4;")
tk.MustQuery("select * from t;").Check(testkit.Rows("3 4"))

tk.MustExec("create table t (a int, b int)")
tk.MustGetErrCode("alter table t drop column a, drop column b", errno.ErrCantRemoveAllFields)
// [a, b] -> [+c after a, +d first, -a, -b] -> [d, c]
tk.MustExec("drop table if exists t;")
tk.MustExec("create table t (a int default 1, b int default 2);")
tk.MustExec("insert into t values ();")
// Note that MariaDB does not support this: Unknown column 'a' in 't'.
// Since TiDB's implementation is snapshot + reasonable cascading, this is supported.
tk.MustExec("alter table t add column c int default 3 after a, add column d int default 4 first, drop column a, drop column b;")
tk.MustQuery("select * from t;").Check(testkit.Rows("4 3"))
}

func TestMultiSchemaChangeOperateSameColumn(t *testing.T) {
Expand Down