Skip to content

Commit

Permalink
ddl: refine multi-schema change error msg
Browse files Browse the repository at this point in the history
  • Loading branch information
tangenta committed Jul 18, 2022
1 parent 8c380e3 commit 0261465
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 10 deletions.
2 changes: 1 addition & 1 deletion ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ func checkMultiSchemaSpecs(_sctx sessionctx.Context, specs []*ast.DatabaseOption
for _, spec := range specs {
if spec.Tp == ast.DatabaseSetTiFlashReplica {
if hasSetTiFlashReplica {
return dbterror.ErrRunMultiSchemaChanges
return dbterror.ErrRunMultiSchemaChanges.FastGenByArgs(model.ActionSetTiFlashReplica.String())
}
hasSetTiFlashReplica = true
}
Expand Down
9 changes: 8 additions & 1 deletion ddl/multi_schema_change.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,21 @@ func onMultiSchemaChange(w *worker, d *ddlCtx, t *meta.Meta, job *model.Job) (ve
if err != nil {
return ver, err
}
var schemaVersionGenerated = false
subJobs := make([]model.SubJob, len(job.MultiSchemaInfo.SubJobs))
// Step the sub-jobs to the non-revertible states all at once.
// We only generate 1 schema version for these sub-job.
for i, sub := range job.MultiSchemaInfo.SubJobs {
if sub.IsFinished() {
continue
}
subJobs[i] = *sub
proxyJob := sub.ToProxyJob(job)
if schemaVersionGenerated {
proxyJob.MultiSchemaInfo.SkipVersion = true
} else {
schemaVersionGenerated = true
}
ver, err = w.runDDLJob(d, t, &proxyJob)
sub.FromProxyJob(&proxyJob)
if err != nil || proxyJob.Error != nil {
Expand Down Expand Up @@ -252,7 +259,7 @@ func fillMultiSchemaInfo(info *model.MultiSchemaInfo, job *model.Job) (err error
info.AlterIndexes = append(info.AlterIndexes, idxName)
case model.ActionRebaseAutoID, model.ActionModifyTableComment, model.ActionModifyTableCharsetAndCollate:
default:
return dbterror.ErrRunMultiSchemaChanges
return dbterror.ErrRunMultiSchemaChanges.FastGenByArgs(job.Type.String())
}
return nil
}
Expand Down
23 changes: 17 additions & 6 deletions ddl/multi_schema_change_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,22 +528,22 @@ func TestMultiSchemaChangeAddIndexes(t *testing.T) {
tk.MustExec("drop table if exists t")
tk.MustExec("create table t (a int, b int, c int)")
tk.MustGetErrCode("alter table t add index t(a), add index t(b)", errno.ErrUnsupportedDDLOperation)
tk.MustQuery("show index from t;").Check(testkit.Rows( /* no index */ ))
tk.MustQuery("show index from t;").Check(testkit.Rows( /* no index */))

// Test add indexes with drop column.
tk.MustExec("drop table if exists t")
tk.MustExec("create table t (a int, b int, c int)")
tk.MustGetErrCode("alter table t add index t(a), drop column a", errno.ErrUnsupportedDDLOperation)
tk.MustGetErrCode("alter table t add index t(a, b), drop column a", errno.ErrUnsupportedDDLOperation)
tk.MustQuery("show index from t;").Check(testkit.Rows( /* no index */ ))
tk.MustQuery("show index from t;").Check(testkit.Rows( /* no index */))

// Test add index failed.
tk.MustExec("drop table if exists t;")
tk.MustExec("create table t (a int, b int, c int);")
tk.MustExec("insert into t values (1, 1, 1), (2, 2, 2), (3, 3, 1);")
tk.MustGetErrCode("alter table t add unique index i1(a), add unique index i2(a, b), add unique index i3(c);",
errno.ErrDupEntry)
tk.MustQuery("show index from t;").Check(testkit.Rows( /* no index */ ))
tk.MustQuery("show index from t;").Check(testkit.Rows( /* no index */))
tk.MustExec("alter table t add index i1(a), add index i2(a, b), add index i3(c);")
}

Expand All @@ -568,7 +568,7 @@ func TestMultiSchemaChangeAddIndexesCancelled(t *testing.T) {
"add index t2(a), add index t3(a, b);", errno.ErrCancelledDDLJob)
dom.DDL().SetHook(originHook)
cancelHook.MustCancelDone(t)
tk.MustQuery("show index from t;").Check(testkit.Rows( /* no index */ ))
tk.MustQuery("show index from t;").Check(testkit.Rows( /* no index */))
tk.MustQuery("select * from t;").Check(testkit.Rows("1 2 3"))
tk.MustExec("admin check table t;")

Expand Down Expand Up @@ -1075,9 +1075,9 @@ func TestMultiSchemaChangeAlterIndexVisibility(t *testing.T) {
tk.MustExec("use test;")
tk.MustExec("create table t (a int, b int, index idx(b));")
tk.MustExec("alter table t add index idx2(a), alter index idx visible;")
tk.MustQuery("select * from t use index (idx, idx2);").Check(testkit.Rows( /* no rows */ ))
tk.MustQuery("select * from t use index (idx, idx2);").Check(testkit.Rows( /* no rows */))
tk.MustGetErrCode("alter table t drop column b, alter index idx invisible;", errno.ErrKeyDoesNotExist)
tk.MustQuery("select a, b from t;").Check(testkit.Rows( /* no rows */ ))
tk.MustQuery("select a, b from t;").Check(testkit.Rows( /* no rows */))
}

func TestMultiSchemaChangeWithExpressionIndex(t *testing.T) {
Expand Down Expand Up @@ -1137,6 +1137,17 @@ func TestMultiSchemaChangeNoSubJobs(t *testing.T) {
require.Equal(t, "create table", rs[0][3])
}

func TestMultiSchemaChangeUnsupportedType(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test;")

tk.MustExec("create table t (a int, b int);")
tk.MustGetErrMsg("alter table t add column c int, auto_id_cache = 1;",
"[ddl:8200]Unsupported multi schema change for modify auto id cache")
}

type cancelOnceHook struct {
store kv.Storage
triggered bool
Expand Down
2 changes: 1 addition & 1 deletion ddl/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1334,7 +1334,7 @@ func updateVersionAndTableInfo(d *ddlCtx, t *meta.Meta, job *model.Job, tblInfo
default:
}
})
if shouldUpdateVer {
if shouldUpdateVer && (job.MultiSchemaInfo == nil || !job.MultiSchemaInfo.SkipVersion) {
ver, err = updateSchemaVersion(d, t, job)
if err != nil {
return 0, errors.Trace(err)
Expand Down
2 changes: 2 additions & 0 deletions parser/model/ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ type MultiSchemaInfo struct {
SubJobs []*SubJob `json:"sub_jobs"`
Revertible bool `json:"revertible"`

SkipVersion bool `json:"-"`

AddColumns []CIStr `json:"-"`
DropColumns []CIStr `json:"-"`
ModifyColumns []CIStr `json:"-"`
Expand Down
2 changes: 1 addition & 1 deletion util/dbterror/ddl_terror.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var (
// ErrCancelledDDLJob means the DDL job is cancelled.
ErrCancelledDDLJob = ClassDDL.NewStd(mysql.ErrCancelledDDLJob)
// ErrRunMultiSchemaChanges means we run multi schema changes.
ErrRunMultiSchemaChanges = ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrUnsupportedDDLOperation].Raw, "multi schema change"), nil))
ErrRunMultiSchemaChanges = ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrUnsupportedDDLOperation].Raw, "multi schema change for %s"), nil))
// ErrOperateSameColumn means we change the same columns multiple times in a DDL.
ErrOperateSameColumn = ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrUnsupportedDDLOperation].Raw, "operate same column '%s'"), nil))
// ErrOperateSameIndex means we change the same indexes multiple times in a DDL.
Expand Down

0 comments on commit 0261465

Please sign in to comment.