diff --git a/ddl/db_cache_test.go b/ddl/db_cache_test.go index 8343cad58f37b..67aab9bb40edb 100644 --- a/ddl/db_cache_test.go +++ b/ddl/db_cache_test.go @@ -273,6 +273,7 @@ func TestIssue34069(t *testing.T) { store, clean := testkit.CreateMockStore(t) defer clean() sem.Enable() + defer sem.Disable() tk := testkit.NewTestKit(t, store) tk.Session().Auth(&auth.UserIdentity{Username: "root", Hostname: "%"}, nil, nil) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index d5cf5ef2efd26..730673740a753 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -6623,7 +6623,7 @@ func (d *ddl) AlterIndexVisibility(ctx sessionctx.Context, ident ast.Ident, inde invisible = true } - skip, err := validateAlterIndexVisibility(indexName, invisible, tb.Meta()) + skip, err := validateAlterIndexVisibility(ctx, indexName, invisible, tb.Meta()) if err != nil { return errors.Trace(err) } diff --git a/ddl/index.go b/ddl/index.go index ac0f74a0e384d..40b8b3cf11ebc 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -338,11 +338,16 @@ func onRenameIndex(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) return ver, nil } -func validateAlterIndexVisibility(indexName model.CIStr, invisible bool, tbl *model.TableInfo) (bool, error) { - if idx := tbl.FindIndexByName(indexName.L); idx == nil { +func validateAlterIndexVisibility(ctx sessionctx.Context, indexName model.CIStr, invisible bool, tbl *model.TableInfo) (bool, error) { + var idx *model.IndexInfo + if idx = tbl.FindIndexByName(indexName.L); idx == nil || idx.State != model.StatePublic { return false, errors.Trace(infoschema.ErrKeyNotExists.GenWithStackByArgs(indexName.O, tbl.Name)) - } else if idx.Invisible == invisible { - return true, nil + } + if ctx == nil || ctx.GetSessionVars() == nil || ctx.GetSessionVars().StmtCtx.MultiSchemaInfo == nil { + // Early return. + if idx.Invisible == invisible { + return true, nil + } } return false, nil } @@ -352,8 +357,13 @@ func onAlterIndexVisibility(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, if err != nil || tblInfo == nil { return ver, errors.Trace(err) } - idx := tblInfo.FindIndexByName(from.L) - idx.Invisible = invisible + + if job.MultiSchemaInfo != nil && job.MultiSchemaInfo.Revertible { + job.MarkNonRevertible() + return updateVersionAndTableInfo(d, t, job, tblInfo, false) + } + + setIndexVisibility(tblInfo, from, invisible) if ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, true); err != nil { job.State = model.JobStateCancelled return ver, errors.Trace(err) @@ -362,6 +372,14 @@ func onAlterIndexVisibility(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, return ver, nil } +func setIndexVisibility(tblInfo *model.TableInfo, name model.CIStr, invisible bool) { + for _, idx := range tblInfo.Indices { + if idx.Name.L == name.L || (isTempIdxInfo(idx, tblInfo) && getChangingIndexOriginName(idx) == name.O) { + idx.Invisible = invisible + } + } +} + func getNullColInfos(tblInfo *model.TableInfo, indexInfo *model.IndexInfo) ([]*model.ColumnInfo, error) { nullCols := make([]*model.ColumnInfo, 0, len(indexInfo.Columns)) for _, colName := range indexInfo.Columns { @@ -915,12 +933,13 @@ func checkAlterIndexVisibility(t *meta.Meta, job *model.Job) (*model.TableInfo, return nil, indexName, invisible, errors.Trace(err) } - skip, err := validateAlterIndexVisibility(indexName, invisible, tblInfo) + skip, err := validateAlterIndexVisibility(nil, indexName, invisible, tblInfo) if err != nil { job.State = model.JobStateCancelled return nil, indexName, invisible, errors.Trace(err) } if skip { + job.State = model.JobStateDone return nil, indexName, invisible, nil } return tblInfo, indexName, invisible, nil diff --git a/ddl/multi_schema_change.go b/ddl/multi_schema_change.go index 8f5654bda0994..2f30a5428904c 100644 --- a/ddl/multi_schema_change.go +++ b/ddl/multi_schema_change.go @@ -247,6 +247,9 @@ func fillMultiSchemaInfo(info *model.MultiSchemaInfo, job *model.Job) (err error case model.ActionSetDefaultValue: col := job.Args[0].(*table.Column) info.ModifyColumns = append(info.ModifyColumns, col.Name) + case model.ActionAlterIndexVisibility: + idxName := job.Args[0].(model.CIStr) + info.AlterIndexes = append(info.AlterIndexes, idxName) default: return dbterror.ErrRunMultiSchemaChanges } diff --git a/ddl/multi_schema_change_test.go b/ddl/multi_schema_change_test.go index 8d688ea9ff483..0488f723d9382 100644 --- a/ddl/multi_schema_change_test.go +++ b/ddl/multi_schema_change_test.go @@ -859,6 +859,10 @@ func TestMultiSchemaChangeModifyColumns(t *testing.T) { tk.MustExec("create table t (a BIGINT NULL DEFAULT '-283977870758975838', b double);") tk.MustExec("insert into t values (-283977870758975838, 0);") tk.MustGetErrCode("alter table t change column a c tinyint null default '111' after b, modify column b time null default '13:51:02' FIRST;", errno.ErrDataOutOfRange) + rows := tk.MustQuery("admin show ddl jobs 1").Rows() + require.Equal(t, rows[0][11], "rollback done") + require.Equal(t, rows[1][11], "rollback done") + require.Equal(t, rows[2][11], "cancelled") tk.MustExec("drop table if exists t;") tk.MustExec("create table t(a int, b int);") @@ -891,6 +895,107 @@ func TestMultiSchemaChangeModifyColumnsCancelled(t *testing.T) { Check(testkit.Rows("int")) } +func TestMultiSchemaChangeAlterIndex(t *testing.T) { + store, dom, clean := testkit.CreateMockStoreAndDomain(t) + defer clean() + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test;") + + // unsupported ddl operations + { + // Test alter the same index + tk.MustExec("drop table if exists t;") + tk.MustExec("create table t (a int, b int, index idx(a, b));") + tk.MustGetErrCode("alter table t alter index idx visible, alter index idx invisible;", errno.ErrUnsupportedDDLOperation) + + // Test drop and alter the same index + tk.MustExec("drop table if exists t;") + tk.MustExec("create table t (a int, b int, index idx(a, b));") + tk.MustGetErrCode("alter table t drop index idx, alter index idx visible;", errno.ErrUnsupportedDDLOperation) + + // Test add and alter the same index + tk.MustExec("drop table if exists t;") + tk.MustExec("create table t (a int, b int);") + tk.MustGetErrCode("alter table t add index idx(a, b), alter index idx invisible", errno.ErrKeyDoesNotExist) + } + + tk.MustExec("drop table t;") + tk.MustExec("create table t (a int, b int, index i1(a, b), index i2(b));") + tk.MustExec("insert into t values (1, 2);") + tk.MustExec("alter table t modify column a tinyint, alter index i2 invisible, alter index i1 invisible;") + tk.MustGetErrCode("select * from t use index (i1);", errno.ErrKeyDoesNotExist) + tk.MustGetErrCode("select * from t use index (i2);", errno.ErrKeyDoesNotExist) + tk.MustQuery("select * from t;").Check(testkit.Rows("1 2")) + tk.MustExec("admin check table t;") + + tk.MustExec("drop table t;") + tk.MustExec("create table t (a int, b int, index i1(a, b), index i2(b));") + tk.MustExec("insert into t values (1, 2);") + originHook := dom.DDL().GetHook() + var checked bool + dom.DDL().SetHook(&ddl.TestDDLCallback{Do: dom, + OnJobUpdatedExported: func(job *model.Job) { + assert.NotNil(t, job.MultiSchemaInfo) + // "modify column a tinyint" in write-reorg. + if job.MultiSchemaInfo.SubJobs[1].SchemaState == model.StateWriteReorganization { + checked = true + rs, err := tk.Exec("select * from t use index(i1);") + assert.NoError(t, err) + assert.NoError(t, rs.Close()) + } + }}) + tk.MustExec("alter table t alter index i1 invisible, modify column a tinyint, alter index i2 invisible;") + dom.DDL().SetHook(originHook) + require.True(t, checked) + tk.MustGetErrCode("select * from t use index (i1);", errno.ErrKeyDoesNotExist) + tk.MustGetErrCode("select * from t use index (i2);", errno.ErrKeyDoesNotExist) + tk.MustQuery("select * from t;").Check(testkit.Rows("1 2")) + tk.MustExec("admin check table t;") +} + +func TestMultiSchemaChangeMix(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, c int, index i1(c), index i2(c));") + tk.MustExec("insert into t values (1, 2, 3);") + tk.MustExec("alter table t add column d int default 4, add index i3(c), " + + "drop column a, drop column if exists z, add column if not exists e int default 5, " + + "drop index i2, add column f int default 6, drop column b, drop index i1, add column if not exists c int;") + tk.MustQuery("select * from t;").Check(testkit.Rows("3 4 5 6")) + tk.MustGetErrCode("select * from t use index (i1);", errno.ErrKeyDoesNotExist) + tk.MustGetErrCode("select * from t use index (i2);", errno.ErrKeyDoesNotExist) + tk.MustQuery("select * from t use index (i3);").Check(testkit.Rows("3 4 5 6")) +} + +func TestMultiSchemaChangeMixCancelled(t *testing.T) { + store, dom, clean := testkit.CreateMockStoreAndDomain(t) + defer clean() + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test;") + + tk.MustExec("create table t (a int, b int, c int, index i1(c), index i2(c));") + tk.MustExec("insert into t values (1, 2, 3);") + origin := dom.DDL().GetHook() + cancelHook := newCancelJobHook(store, dom, func(job *model.Job) bool { + return job.MultiSchemaInfo != nil && + len(job.MultiSchemaInfo.SubJobs) > 8 && + job.MultiSchemaInfo.SubJobs[8].SchemaState == model.StateWriteReorganization + }) + dom.DDL().SetHook(cancelHook) + tk.MustGetErrCode("alter table t add column d int default 4, add index i3(c), "+ + "drop column a, drop column if exists z, add column if not exists e int default 5, "+ + "drop index i2, add column f int default 6, drop column b, drop index i1, add column if not exists g int;", + errno.ErrCancelledDDLJob) + dom.DDL().SetHook(origin) + cancelHook.MustCancelDone(t) + tk.MustQuery("select * from t;").Check(testkit.Rows("1 2 3")) + tk.MustQuery("select * from t use index(i1, i2);").Check(testkit.Rows("1 2 3")) + tk.MustExec("admin check table t;") +} + func TestMultiSchemaChangeAdminShowDDLJobs(t *testing.T) { store, dom, clean := testkit.CreateMockStoreAndDomain(t) defer clean() @@ -926,6 +1031,18 @@ func TestMultiSchemaChangeAdminShowDDLJobs(t *testing.T) { dom.DDL().SetHook(originHook) } +func TestMultiSchemaChangeAlterIndexVisibility(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, 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.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 */ )) +} + func TestMultiSchemaChangeWithExpressionIndex(t *testing.T) { store, dom, clean := testkit.CreateMockStoreAndDomain(t) defer clean() @@ -969,6 +1086,20 @@ func TestMultiSchemaChangeWithExpressionIndex(t *testing.T) { tk.MustQuery("select * from t use index(idx1, idx2);").Check(testkit.Rows("1 2 10", "2 1 10")) } +func TestMultiSchemaChangeNoSubJobs(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.MustExec("alter table t add column if not exists a int, add column if not exists b int;") + tk.MustQuery("show warnings;").Check(testkit.Rows( + "Note 1060 Duplicate column name 'a'", "Note 1060 Duplicate column name 'b'")) + rs := tk.MustQuery("admin show ddl jobs 1;").Rows() + require.Equal(t, "create table", rs[0][3]) +} + type cancelOnceHook struct { store kv.Storage triggered bool