diff --git a/ddl/cancel_test.go b/ddl/cancel_test.go index 57f449713c2f4..199a75346e608 100644 --- a/ddl/cancel_test.go +++ b/ddl/cancel_test.go @@ -24,9 +24,7 @@ import ( "github.com/pingcap/tidb/errno" "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/testkit" - "github.com/pingcap/tidb/util/logutil" "github.com/stretchr/testify/require" - "go.uber.org/zap" ) type testCancelJob struct { @@ -49,8 +47,7 @@ var allTestCase = []testCancelJob{ {"alter table t add primary key idx_pc2 (c2)", true, model.StateWriteReorganization, true, true, nil}, {"alter table t add primary key idx_pc2 (c2)", false, model.StatePublic, false, true, nil}, // Drop primary key - // TODO: fix schema state. - {"alter table t drop primary key", true, model.StateNone, true, false, nil}, + {"alter table t drop primary key", true, model.StatePublic, true, false, nil}, {"alter table t drop primary key", false, model.StateWriteOnly, true, false, nil}, {"alter table t drop primary key", false, model.StateWriteOnly, true, false, []string{"alter table t add primary key idx_pc2 (c2)"}}, {"alter table t drop primary key", false, model.StateDeleteOnly, true, false, []string{"alter table t add primary key idx_pc2 (c2)"}}, @@ -75,36 +72,32 @@ var allTestCase = []testCancelJob{ {"create table test_create_table(a int)", true, model.StateNone, true, false, nil}, {"create table test_create_table(a int)", false, model.StatePublic, false, true, nil}, // Drop table. - // TODO: fix schema state. - {"drop table test_create_table", true, model.StateNone, true, false, nil}, + {"drop table test_create_table", true, model.StatePublic, true, false, nil}, {"drop table test_create_table", false, model.StateWriteOnly, true, true, []string{"create table if not exists test_create_table(a int)"}}, {"drop table test_create_table", false, model.StateDeleteOnly, true, true, []string{"create table if not exists test_create_table(a int)"}}, - {"drop table test_create_table", false, model.StatePublic, false, true, []string{"create table if not exists test_create_table(a int)"}}, + {"drop table test_create_table", false, model.StateNone, false, true, []string{"create table if not exists test_create_table(a int)"}}, // Create schema. {"create database test_create_db", true, model.StateNone, true, false, nil}, {"create database test_create_db", false, model.StatePublic, false, true, nil}, // Drop schema. - // TODO: fix schema state. - {"drop database test_create_db", true, model.StateNone, true, false, nil}, + {"drop database test_create_db", true, model.StatePublic, true, false, nil}, {"drop database test_create_db", false, model.StateWriteOnly, true, true, []string{"create database if not exists test_create_db"}}, {"drop database test_create_db", false, model.StateDeleteOnly, true, true, []string{"create database if not exists test_create_db"}}, - {"drop database test_create_db", false, model.StatePublic, false, true, []string{"create database if not exists test_create_db"}}, + {"drop database test_create_db", false, model.StateNone, false, true, []string{"create database if not exists test_create_db"}}, // Drop column. - // TODO: fix schema state. - {"alter table t drop column c3", true, model.StateNone, true, false, nil}, + {"alter table t drop column c3", true, model.StatePublic, true, false, nil}, {"alter table t drop column c3", false, model.StateDeleteOnly, true, false, nil}, {"alter table t drop column c3", false, model.StateDeleteOnly, false, true, []string{"alter table t add column c3 bigint"}}, {"alter table t drop column c3", false, model.StateWriteOnly, true, true, []string{"alter table t add column c3 bigint"}}, {"alter table t drop column c3", false, model.StateDeleteReorganization, true, true, []string{"alter table t add column c3 bigint"}}, - {"alter table t drop column c3", false, model.StatePublic, false, true, []string{"alter table t add column c3 bigint"}}, + {"alter table t drop column c3", false, model.StateNone, false, true, []string{"alter table t add column c3 bigint"}}, // Drop column with index. - // TODO: fix schema state. - {"alter table t drop column c3", true, model.StateNone, true, false, []string{"alter table t add column c3 bigint", "alter table t add index idx_c3(c3)"}}, + {"alter table t drop column c3", true, model.StatePublic, true, false, []string{"alter table t add column c3 bigint", "alter table t add index idx_c3(c3)"}}, {"alter table t drop column c3", false, model.StateDeleteOnly, true, false, nil}, {"alter table t drop column c3", false, model.StateDeleteOnly, false, true, []string{"alter table t add column c3 bigint", "alter table t add index idx_c3(c3)"}}, {"alter table t drop column c3", false, model.StateWriteOnly, true, true, []string{"alter table t add column c3 bigint", "alter table t add index idx_c3(c3)"}}, {"alter table t drop column c3", false, model.StateDeleteReorganization, true, true, []string{"alter table t add column c3 bigint", "alter table t add index idx_c3(c3)"}}, - {"alter table t drop column c3", false, model.StatePublic, false, true, []string{"alter table t add column c3 bigint", "alter table t add index idx_c3(c3)"}}, + {"alter table t drop column c3", false, model.StateNone, false, true, []string{"alter table t add column c3 bigint", "alter table t add index idx_c3(c3)"}}, // rebase auto ID. {"alter table t_rebase auto_increment = 6000", true, model.StateNone, true, false, []string{"create table t_rebase (c1 bigint auto_increment primary key, c2 bigint);"}}, {"alter table t_rebase auto_increment = 9000", false, model.StatePublic, false, true, nil}, @@ -128,9 +121,8 @@ var allTestCase = []testCancelJob{ {"alter table t add constraint fk foreign key a(c1) references t_ref(c1)", true, model.StateNone, true, false, []string{"create table t_ref (c1 int, c2 int, c3 int, c11 tinyint);"}}, {"alter table t add constraint fk foreign key a(c1) references t_ref(c1)", false, model.StatePublic, false, true, nil}, // Drop foreign key. - // TODO: fix schema state. - {"alter table t drop foreign key fk", true, model.StateNone, true, false, nil}, - {"alter table t drop foreign key fk", false, model.StatePublic, false, true, nil}, + {"alter table t drop foreign key fk", true, model.StatePublic, true, false, nil}, + {"alter table t drop foreign key fk", false, model.StateNone, false, true, nil}, // Rename table. {"rename table t_rename1 to t_rename11", true, model.StateNone, true, false, []string{"create table t_rename1 (c1 bigint , c2 bigint);", "create table t_rename2 (c1 bigint , c2 bigint);"}}, {"rename table t_rename1 to t_rename11", false, model.StatePublic, false, true, nil}, @@ -179,12 +171,11 @@ var allTestCase = []testCancelJob{ {"alter table t_partition add partition (partition p6 values less than (8192))", true, model.StateReplicaOnly, true, true, nil}, {"alter table t_partition add partition (partition p6 values less than (8192))", false, model.StatePublic, false, true, nil}, // Drop partition. - // TODO: fix schema state. - {"alter table t_partition drop partition p6", true, model.StateNone, true, false, nil}, + {"alter table t_partition drop partition p6", true, model.StatePublic, true, false, nil}, {"alter table t_partition drop partition p6", false, model.StateDeleteOnly, true, false, nil}, {"alter table t_partition drop partition p6", false, model.StateDeleteOnly, false, true, []string{"alter table t_partition add partition (partition p6 values less than (8192))"}}, {"alter table t_partition drop partition p6", false, model.StateDeleteReorganization, true, true, []string{"alter table t_partition add partition (partition p6 values less than (8192))"}}, - {"alter table t_partition drop partition p6", false, model.StatePublic, true, true, []string{"alter table t_partition add partition (partition p6 values less than (8192))"}}, + {"alter table t_partition drop partition p6", false, model.StateNone, true, true, []string{"alter table t_partition add partition (partition p6 values less than (8192))"}}, // Drop indexes. // TODO: fix schema state. {"alter table t drop index mul_idx1, drop index mul_idx2", true, model.StateNone, true, false, []string{"alter table t add index mul_idx1(c1)", "alter table t add index mul_idx2(c1)"}}, @@ -242,6 +233,7 @@ func TestCancel(t *testing.T) { hook := &ddl.TestDDLCallback{Do: dom} i := 0 cancel := false + cancelResult := false cancelWhenReorgNotStart := false hookFunc := func(job *model.Job) { @@ -250,7 +242,7 @@ func TestCancel(t *testing.T) { return } rs := tkCancel.MustQuery(fmt.Sprintf("admin cancel ddl jobs %d", job.ID)) - require.Equal(t, allTestCase[i].ok, cancelSuccess(rs)) + cancelResult = cancelSuccess(rs) cancel = true } } @@ -270,6 +262,7 @@ func TestCancel(t *testing.T) { for j, tc := range allTestCase { i = j + msg := fmt.Sprintf("sql: %s, state: %s", tc.sql, tc.cancelState) if tc.onJobBefore { restHook(hook) for _, prepareSQL := range tc.prepareSQL { @@ -279,28 +272,31 @@ func TestCancel(t *testing.T) { cancel = false cancelWhenReorgNotStart = true registHook(hook, true) - logutil.BgLogger().Info("test case", zap.Int("", i)) if tc.ok { tk.MustGetErrCode(tc.sql, errno.ErrCancelledDDLJob) } else { tk.MustExec(tc.sql) } + if cancel { + require.Equal(t, tc.ok, cancelResult, msg) + } } if tc.onJobUpdate { restHook(hook) for _, prepareSQL := range tc.prepareSQL { tk.MustExec(prepareSQL) } - cancel = false cancelWhenReorgNotStart = false registHook(hook, false) - logutil.BgLogger().Info("test case", zap.Int("", i)) if tc.ok { tk.MustGetErrCode(tc.sql, errno.ErrCancelledDDLJob) } else { tk.MustExec(tc.sql) } + if cancel { + require.Equal(t, tc.ok, cancelResult, msg) + } } } } diff --git a/ddl/column.go b/ddl/column.go index 28a3ff13abf17..61c55e0b2de33 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -556,7 +556,6 @@ func onDropColumn(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) if err != nil { return ver, errors.Trace(err) } - job.SchemaState = model.StateWriteOnly case model.StateWriteOnly: // write only -> delete only colInfo.State = model.StateDeleteOnly @@ -574,7 +573,6 @@ func onDropColumn(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) return ver, errors.Trace(err) } job.Args = append(job.Args, indexInfosToIDList(idxInfos)) - job.SchemaState = model.StateDeleteOnly case model.StateDeleteOnly: // delete only -> reorganization colInfo.State = model.StateDeleteReorganization @@ -582,7 +580,6 @@ func onDropColumn(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) if err != nil { return ver, errors.Trace(err) } - job.SchemaState = model.StateDeleteReorganization case model.StateDeleteReorganization: // reorganization -> absent // All reorganization jobs are done, drop this column. @@ -602,8 +599,9 @@ func onDropColumn(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) job.Args = append(job.Args, getPartitionIDs(tblInfo)) } default: - err = dbterror.ErrInvalidDDLJob.GenWithStackByArgs("table", tblInfo.State) + return ver, errors.Trace(dbterror.ErrInvalidDDLJob.GenWithStackByArgs("table", tblInfo.State)) } + job.SchemaState = colInfo.State return ver, errors.Trace(err) } diff --git a/ddl/column_change_test.go b/ddl/column_change_test.go index 7510fd96e3181..436559bd2fe3b 100644 --- a/ddl/column_change_test.go +++ b/ddl/column_change_test.go @@ -97,7 +97,7 @@ func TestColumnAdd(t *testing.T) { tc.OnJobUpdatedExported = func(job *model.Job) { jobID = job.ID tbl := external.GetTableByName(t, internal, "test", "t") - if job.SchemaState != model.StateNone { + if job.SchemaState != model.StatePublic { for _, col := range tbl.Cols() { require.NotEqualf(t, col.ID, dropCol.ID, "column is not dropped") } diff --git a/ddl/db_change_test.go b/ddl/db_change_test.go index 67be5b2de9331..889655231ba16 100644 --- a/ddl/db_change_test.go +++ b/ddl/db_change_test.go @@ -822,16 +822,15 @@ func (s *stateChangeSuite) runTestInSchemaState( callback := &ddl.TestDDLCallback{Do: s.dom} prevState := model.StateNone var checkErr error - times := 0 se, err := session.CreateSession(s.store) s.Require().NoError(err) _, err = se.Execute(context.Background(), "use test_db_state") s.Require().NoError(err) cbFunc := func(job *model.Job) { - if job.SchemaState == prevState || checkErr != nil || times >= 3 { + if job.SchemaState == prevState || checkErr != nil { return } - times++ + prevState = job.SchemaState if job.SchemaState != state { return } diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index f60ec61957f4e..6b8401caf3ef0 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -527,10 +527,11 @@ func (d *ddl) DropSchema(ctx sessionctx.Context, schema model.CIStr) (err error) return errors.Trace(infoschema.ErrDatabaseNotExists) } job := &model.Job{ - SchemaID: old.ID, - SchemaName: old.Name.L, - Type: model.ActionDropSchema, - BinlogInfo: &model.HistoryInfo{}, + SchemaID: old.ID, + SchemaName: old.Name.L, + SchemaState: old.State, + Type: model.ActionDropSchema, + BinlogInfo: &model.HistoryInfo{}, } err = d.DoDDLJob(ctx, job) @@ -3784,13 +3785,14 @@ func (d *ddl) DropTablePartition(ctx sessionctx.Context, ident ast.Ident, spec * } job := &model.Job{ - SchemaID: schema.ID, - TableID: meta.ID, - SchemaName: schema.Name.L, - TableName: meta.Name.L, - Type: model.ActionDropTablePartition, - BinlogInfo: &model.HistoryInfo{}, - Args: []interface{}{partNames}, + SchemaID: schema.ID, + TableID: meta.ID, + SchemaName: schema.Name.L, + SchemaState: model.StatePublic, + TableName: meta.Name.L, + Type: model.ActionDropTablePartition, + BinlogInfo: &model.HistoryInfo{}, + Args: []interface{}{partNames}, } err = d.DoDDLJob(ctx, job) @@ -4029,6 +4031,7 @@ func (d *ddl) DropColumn(ctx sessionctx.Context, ti ast.Ident, spec *ast.AlterTa Type: model.ActionDropColumn, BinlogInfo: &model.HistoryInfo{}, MultiSchemaInfo: multiSchemaInfo, + SchemaState: model.StatePublic, Args: []interface{}{colName}, } @@ -5267,12 +5270,13 @@ func (d *ddl) DropTable(ctx sessionctx.Context, ti ast.Ident) (err error) { } job := &model.Job{ - SchemaID: schema.ID, - TableID: tb.Meta().ID, - SchemaName: schema.Name.L, - TableName: tb.Meta().Name.L, - Type: model.ActionDropTable, - BinlogInfo: &model.HistoryInfo{}, + SchemaID: schema.ID, + TableID: tb.Meta().ID, + SchemaName: schema.Name.L, + SchemaState: schema.State, + TableName: tb.Meta().Name.L, + Type: model.ActionDropTable, + BinlogInfo: &model.HistoryInfo{}, } err = d.DoDDLJob(ctx, job) @@ -5301,12 +5305,13 @@ func (d *ddl) DropView(ctx sessionctx.Context, ti ast.Ident) (err error) { } job := &model.Job{ - SchemaID: schema.ID, - TableID: tb.Meta().ID, - SchemaName: schema.Name.L, - TableName: tb.Meta().Name.L, - Type: model.ActionDropView, - BinlogInfo: &model.HistoryInfo{}, + SchemaID: schema.ID, + TableID: tb.Meta().ID, + SchemaName: schema.Name.L, + SchemaState: tb.Meta().State, + TableName: tb.Meta().Name.L, + Type: model.ActionDropView, + BinlogInfo: &model.HistoryInfo{}, } err = d.DoDDLJob(ctx, job) @@ -5969,13 +5974,14 @@ func (d *ddl) DropForeignKey(ctx sessionctx.Context, ti ast.Ident, fkName model. } job := &model.Job{ - SchemaID: schema.ID, - TableID: t.Meta().ID, - SchemaName: schema.Name.L, - TableName: t.Meta().Name.L, - Type: model.ActionDropForeignKey, - BinlogInfo: &model.HistoryInfo{}, - Args: []interface{}{fkName}, + SchemaID: schema.ID, + TableID: t.Meta().ID, + SchemaName: schema.Name.L, + SchemaState: model.StatePublic, + TableName: t.Meta().Name.L, + Type: model.ActionDropForeignKey, + BinlogInfo: &model.HistoryInfo{}, + Args: []interface{}{fkName}, } err = d.DoDDLJob(ctx, job) @@ -6025,13 +6031,14 @@ func (d *ddl) DropIndex(ctx sessionctx.Context, ti ast.Ident, indexName model.CI } job := &model.Job{ - SchemaID: schema.ID, - TableID: t.Meta().ID, - SchemaName: schema.Name.L, - TableName: t.Meta().Name.L, - Type: jobTp, - BinlogInfo: &model.HistoryInfo{}, - Args: []interface{}{indexName}, + SchemaID: schema.ID, + TableID: t.Meta().ID, + SchemaName: schema.Name.L, + TableName: t.Meta().Name.L, + Type: jobTp, + BinlogInfo: &model.HistoryInfo{}, + SchemaState: indexInfo.State, + Args: []interface{}{indexName}, } err = d.DoDDLJob(ctx, job) @@ -6596,12 +6603,13 @@ func (d *ddl) DropSequence(ctx sessionctx.Context, ti ast.Ident, ifExists bool) } job := &model.Job{ - SchemaID: schema.ID, - TableID: tbl.Meta().ID, - SchemaName: schema.Name.L, - TableName: tbl.Meta().Name.L, - Type: model.ActionDropSequence, - BinlogInfo: &model.HistoryInfo{}, + SchemaID: schema.ID, + TableID: tbl.Meta().ID, + SchemaName: schema.Name.L, + SchemaState: tbl.Meta().State, + TableName: tbl.Meta().Name.L, + Type: model.ActionDropSequence, + BinlogInfo: &model.HistoryInfo{}, } err = d.DoDDLJob(ctx, job) diff --git a/ddl/foreign_key.go b/ddl/foreign_key.go index dce22609b8e7f..b81e2729af379 100644 --- a/ddl/foreign_key.go +++ b/ddl/foreign_key.go @@ -106,6 +106,7 @@ func onDropForeignKey(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ err } // Finish this job. job.FinishTableJob(model.JobStateDone, model.StateNone, ver, tblInfo) + job.SchemaState = fkInfo.State return ver, nil default: return ver, dbterror.ErrInvalidDDLState.GenWithStackByArgs("foreign key", fkInfo.State) diff --git a/ddl/index.go b/ddl/index.go index 9321da3eba7f7..f4d74028256d8 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -647,7 +647,6 @@ func onDropIndex(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) { if err != nil { return ver, errors.Trace(err) } - job.SchemaState = model.StateWriteOnly case model.StateWriteOnly: // write only -> delete only indexInfo.State = model.StateDeleteOnly @@ -655,7 +654,6 @@ func onDropIndex(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) { if err != nil { return ver, errors.Trace(err) } - job.SchemaState = model.StateDeleteOnly case model.StateDeleteOnly: // delete only -> reorganization indexInfo.State = model.StateDeleteReorganization @@ -663,9 +661,9 @@ func onDropIndex(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) { if err != nil { return ver, errors.Trace(err) } - job.SchemaState = model.StateDeleteReorganization case model.StateDeleteReorganization: // reorganization -> absent + indexInfo.State = model.StateNone if len(dependentHiddenCols) > 0 { firstHiddenOffset := dependentHiddenCols[0].Offset for i := 0; i < len(dependentHiddenCols); i++ { @@ -673,7 +671,6 @@ func onDropIndex(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) { adjustColumnInfoInDropColumn(tblInfo, firstHiddenOffset) } } - newIndices := make([]*model.IndexInfo, 0, len(tblInfo.Indices)) for _, idx := range tblInfo.Indices { if idx.Name.L != indexInfo.Name.L { @@ -707,8 +704,9 @@ func onDropIndex(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) { job.Args = append(job.Args, indexInfo.ID, getPartitionIDs(tblInfo)) } default: - err = dbterror.ErrInvalidDDLState.GenWithStackByArgs("index", indexInfo.State) + return ver, errors.Trace(dbterror.ErrInvalidDDLState.GenWithStackByArgs("index", indexInfo.State)) } + job.SchemaState = indexInfo.State return ver, errors.Trace(err) } diff --git a/ddl/partition.go b/ddl/partition.go index b8d75dbd9d8df..01adaca88e217 100644 --- a/ddl/partition.go +++ b/ddl/partition.go @@ -1115,10 +1115,6 @@ func (w *worker) onDropTablePartition(d *ddlCtx, t *meta.Meta, job *model.Job) ( } var physicalTableIDs []int64 - if job.State == model.JobStateRunning && job.SchemaState == model.StateNone { - // Manually set first state. - job.SchemaState = model.StatePublic - } // In order to skip maintaining the state check in partitionDefinition, TiDB use droppingDefinition instead of state field. // So here using `job.SchemaState` to judge what the stage of this job is. originalState := job.SchemaState @@ -1199,6 +1195,7 @@ func (w *worker) onDropTablePartition(d *ddlCtx, t *meta.Meta, job *model.Job) ( if err != nil { return ver, errors.Trace(err) } + job.SchemaState = model.StateNone job.FinishTableJob(model.JobStateDone, model.StateNone, ver, tblInfo) asyncNotifyEvent(d, &util.Event{Tp: model.ActionDropTablePartition, TableInfo: tblInfo, PartInfo: &model.PartitionInfo{Definitions: tblInfo.Partition.Definitions}}) // A background job will be created to delete old partition data. diff --git a/ddl/partition_test.go b/ddl/partition_test.go index 4484dcb07dde8..612b948e1f936 100644 --- a/ddl/partition_test.go +++ b/ddl/partition_test.go @@ -110,11 +110,12 @@ func buildTableInfoWithPartition(t *testing.T, d *ddl) (*model.TableInfo, []int6 func buildDropPartitionJob(dbInfo *model.DBInfo, tblInfo *model.TableInfo, partNames []string) *model.Job { return &model.Job{ - SchemaID: dbInfo.ID, - TableID: tblInfo.ID, - Type: model.ActionDropTablePartition, - BinlogInfo: &model.HistoryInfo{}, - Args: []interface{}{partNames}, + SchemaID: dbInfo.ID, + TableID: tblInfo.ID, + SchemaState: model.StatePublic, + Type: model.ActionDropTablePartition, + BinlogInfo: &model.HistoryInfo{}, + Args: []interface{}{partNames}, } } diff --git a/ddl/rollingback.go b/ddl/rollingback.go index dca0bdc823631..7ff3c6088970f 100644 --- a/ddl/rollingback.go +++ b/ddl/rollingback.go @@ -387,7 +387,7 @@ func rollingbackDropTablePartition(t *meta.Meta, job *model.Job) (ver int64, err if err != nil { return ver, errors.Trace(err) } - return cancelOnlyNotHandledJob(job) + return cancelOnlyNotHandledJob(job, model.StatePublic) } func rollingbackDropSchema(t *meta.Meta, job *model.Job) error { @@ -420,9 +420,9 @@ func rollingbackRenameIndex(t *meta.Meta, job *model.Job) (ver int64, err error) return ver, errors.Trace(err) } -func cancelOnlyNotHandledJob(job *model.Job) (ver int64, err error) { +func cancelOnlyNotHandledJob(job *model.Job, initialState model.SchemaState) (ver int64, err error) { // We can only cancel the not handled job. - if job.SchemaState == model.StateNone { + if job.SchemaState == initialState { job.State = model.JobStateCancelled return ver, dbterror.ErrCancelledDDLJob } @@ -437,7 +437,7 @@ func rollingbackTruncateTable(t *meta.Meta, job *model.Job) (ver int64, err erro if err != nil { return ver, errors.Trace(err) } - return cancelOnlyNotHandledJob(job) + return cancelOnlyNotHandledJob(job, model.StateNone) } func convertJob2RollbackJob(w *worker, d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) { @@ -472,13 +472,15 @@ func convertJob2RollbackJob(w *worker, d *ddlCtx, t *meta.Meta, job *model.Job) ver, err = rollingbackTruncateTable(t, job) case model.ActionModifyColumn: ver, err = rollingbackModifyColumn(w, d, t, job) + case model.ActionDropForeignKey: + ver, err = cancelOnlyNotHandledJob(job, model.StatePublic) case model.ActionRebaseAutoID, model.ActionShardRowID, model.ActionAddForeignKey, - model.ActionDropForeignKey, model.ActionRenameTable, model.ActionRenameTables, + model.ActionRenameTable, model.ActionRenameTables, model.ActionModifyTableCharsetAndCollate, model.ActionTruncateTablePartition, model.ActionModifySchemaCharsetAndCollate, model.ActionRepairTable, model.ActionModifyTableAutoIdCache, model.ActionAlterIndexVisibility, model.ActionExchangeTablePartition, model.ActionModifySchemaDefaultPlacement: - ver, err = cancelOnlyNotHandledJob(job) + ver, err = cancelOnlyNotHandledJob(job, model.StateNone) default: job.State = model.JobStateCancelled err = dbterror.ErrCancelledDDLJob diff --git a/ddl/schema.go b/ddl/schema.go index d9a9938ac6593..9fff2c5a77338 100644 --- a/ddl/schema.go +++ b/ddl/schema.go @@ -211,9 +211,6 @@ func onDropSchema(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) job.State = model.JobStateCancelled return ver, errors.Trace(err) } - - // Update the job state when all affairs done. - job.SchemaState = model.StateWriteOnly case model.StateWriteOnly: // write only -> delete only dbInfo.State = model.StateDeleteOnly @@ -221,8 +218,6 @@ func onDropSchema(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) if err != nil { return ver, errors.Trace(err) } - // Update the job state when all affairs done. - job.SchemaState = model.StateDeleteOnly case model.StateDeleteOnly: dbInfo.State = model.StateNone var tables []*model.TableInfo @@ -246,9 +241,9 @@ func onDropSchema(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) job.FinishDBJob(model.JobStateDone, model.StateNone, ver, dbInfo) default: // We can't enter here. - err = errors.Errorf("invalid db state %v", dbInfo.State) + return ver, errors.Trace(errors.Errorf("invalid db state %v", dbInfo.State)) } - + job.SchemaState = dbInfo.State return ver, errors.Trace(err) } diff --git a/ddl/table.go b/ddl/table.go index e9d8e05ce1154..53de0becd51ec 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -290,7 +290,6 @@ func onDropTableOrView(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ er if err != nil { return ver, errors.Trace(err) } - job.SchemaState = model.StateWriteOnly case model.StateWriteOnly: // write only -> delete only tblInfo.State = model.StateDeleteOnly @@ -298,7 +297,6 @@ func onDropTableOrView(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ er if err != nil { return ver, errors.Trace(err) } - job.SchemaState = model.StateDeleteOnly case model.StateDeleteOnly: tblInfo.State = model.StateNone oldIDs := getPartitionIDs(tblInfo) @@ -311,14 +309,14 @@ func onDropTableOrView(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ er } if tblInfo.IsSequence() { if err = t.DropSequence(job.SchemaID, job.TableID); err != nil { - break + return ver, errors.Trace(err) } } else { if err = t.DropTableOrView(job.SchemaID, job.TableID); err != nil { - break + return ver, errors.Trace(err) } if err = t.GetAutoIDAccessors(job.SchemaID, job.TableID).Del(); err != nil { - break + return ver, errors.Trace(err) } } // Placement rules cannot be removed immediately after drop table / truncate table, because the @@ -329,9 +327,9 @@ func onDropTableOrView(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ er startKey := tablecodec.EncodeTablePrefix(job.TableID) job.Args = append(job.Args, startKey, oldIDs, ruleIDs) default: - err = dbterror.ErrInvalidDDLState.GenWithStackByArgs("table", tblInfo.State) + return ver, errors.Trace(dbterror.ErrInvalidDDLState.GenWithStackByArgs("table", tblInfo.State)) } - + job.SchemaState = tblInfo.State return ver, errors.Trace(err) } diff --git a/parser/model/ddl.go b/parser/model/ddl.go index 2c5d7b33f5bf2..3b17a9bd69fe8 100644 --- a/parser/model/ddl.go +++ b/parser/model/ddl.go @@ -541,18 +541,13 @@ func (job *Job) IsRollbackable() bool { job.SchemaState == StateWriteOnly { return false } - case ActionDropSchema, ActionDropTable, ActionDropSequence: - // To simplify the rollback logic, cannot be canceled in the following states. - if job.SchemaState == StateWriteOnly || - job.SchemaState == StateDeleteOnly { - return false - } case ActionAddTablePartition: return job.SchemaState == StateNone || job.SchemaState == StateReplicaOnly - case ActionDropColumn, ActionDropColumns, ActionDropTablePartition, - ActionRebaseAutoID, ActionShardRowID, - ActionTruncateTable, ActionAddForeignKey, - ActionDropForeignKey, ActionRenameTable, + case ActionDropColumn, ActionDropSchema, ActionDropTable, ActionDropSequence, + ActionDropForeignKey, ActionDropTablePartition: + return job.SchemaState == StatePublic + case ActionDropColumns, ActionRebaseAutoID, ActionShardRowID, + ActionTruncateTable, ActionAddForeignKey, ActionRenameTable, ActionModifyTableCharsetAndCollate, ActionTruncateTablePartition, ActionModifySchemaCharsetAndCollate, ActionRepairTable, ActionModifyTableAutoIdCache, ActionModifySchemaDefaultPlacement: diff --git a/testkit/testkit.go b/testkit/testkit.go index 44a9ae59a5c16..4631861445b39 100644 --- a/testkit/testkit.go +++ b/testkit/testkit.go @@ -272,12 +272,12 @@ func (tk *TestKit) RefreshConnectionID() { // MustGetErrCode executes a sql statement and assert it's error code. func (tk *TestKit) MustGetErrCode(sql string, errCode int) { _, err := tk.Exec(sql) - tk.require.Error(err) + tk.require.Errorf(err, "sql: %s", sql) originErr := errors.Cause(err) tErr, ok := originErr.(*terror.Error) - tk.require.Truef(ok, "expect type 'terror.Error', but obtain '%T': %v", originErr, originErr) + tk.require.Truef(ok, "sql: %s, expect type 'terror.Error', but obtain '%T': %v", sql, originErr, originErr) sqlErr := terror.ToSQLError(tErr) - tk.require.Equalf(errCode, int(sqlErr.Code), "Assertion failed, origin err:\n %v", sqlErr) + tk.require.Equalf(errCode, int(sqlErr.Code), "sql: %s, Assertion failed, origin err:\n %v", sql, sqlErr) } // MustGetErrMsg executes a sql statement and assert its error message.