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.

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

ddl: add alter index visibility DDL support #16914

Merged
merged 18 commits into from
May 8, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
40 changes: 40 additions & 0 deletions ddl/db_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2257,3 +2257,43 @@ func (s *testIntegrationSuite3) TestCreateTableWithAutoIdCache(c *C) {
c.Assert(err, NotNil)
c.Assert(err.Error(), Equals, "table option auto_id_cache overflows int64")
}

func (s *testIntegrationSuite4) TestAlterIndexVisibility(c *C) {
config.GetGlobalConfig().Experimental.AllowsExpressionIndex = true
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("create database if not exists alter_index_test")
tk.MustExec("USE alter_index_test;")
tk.MustExec("drop table if exists t, t1, t2, t3;")

tk.MustExec("create table t(a int NOT NULL, b int, key(a), unique(b) invisible)")
queryIndexOnTable := func(tableName string) string{
return fmt.Sprintf("select index_name, is_visible from information_schema.statistics where table_schema = 'alter_index_test' and table_name = '%s' order by index_name", tableName)
}
query := queryIndexOnTable("t")
tk.MustQuery(query).Check(testkit.Rows("a YES", "b NO"))

tk.MustExec("alter table t alter index a invisible")
tk.MustQuery(query).Check(testkit.Rows("a NO", "b NO"))

tk.MustExec("alter table t alter index b visible")
tk.MustQuery(query).Check(testkit.Rows("a NO", "b YES"))

tk.MustExec("alter table t alter index b invisible")
tk.MustQuery(query).Check(testkit.Rows("a NO", "b NO"))

tk.MustGetErrMsg("alter table t alter index non_exists_idx visible", "[schema:1176]Key 'non_exists_idx' doesn't exist in table 't'")
Deardrops marked this conversation as resolved.
Show resolved Hide resolved

// Alter implicit primary key to invisible index should throw error
tk.MustExec("create table t1(a int NOT NULL, unique(a))")
// TODO: error here
Deardrops marked this conversation as resolved.
Show resolved Hide resolved
//tk.MustGetErrMsg("alter table t1 alter index a invisible", "")

// Alter expression index
tk.MustExec("create table t3(a int NOT NULL, b int)")
tk.MustExec("alter table t3 add index idx((a+b));")
query = queryIndexOnTable("t3")
tk.MustQuery(query).Check(testkit.Rows("idx YES"))

tk.MustExec("alter table t3 alter index idx invisible")
tk.MustQuery(query).Check(testkit.Rows("idx NO"))
}
Deardrops marked this conversation as resolved.
Show resolved Hide resolved
35 changes: 35 additions & 0 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -2203,6 +2203,8 @@ func (d *ddl) AlterTable(ctx sessionctx.Context, ident ast.Ident, specs []*ast.A
err = d.AlterTableSetTiFlashReplica(ctx, ident, spec.TiFlashReplica)
case ast.AlterTableOrderByColumns:
err = d.OrderByColumns(ctx, ident)
case ast.AlterTableIndexInvisible:
err = d.AlterIndexVisibility(ctx, ident, spec.KeyName, spec.Visibility)
default:
// Nothing to do now.
}
Expand Down Expand Up @@ -4723,3 +4725,36 @@ func (d *ddl) DropSequence(ctx sessionctx.Context, ti ast.Ident, ifExists bool)
err = d.callHookOnChanged(err)
return errors.Trace(err)
}

func (d *ddl) AlterIndexVisibility(ctx sessionctx.Context, ident ast.Ident, indexName model.CIStr, visibility ast.IndexVisibility) error {
schema, tb, err := d.getSchemaAndTableByIdent(ctx, ident)
if err != nil {
return err
}
Deardrops marked this conversation as resolved.
Show resolved Hide resolved

invisible := false
if visibility == ast.IndexVisibilityInvisible {
invisible = true
}

skip, err := validateAlterIndexVisibility(indexName, invisible, tb.Meta())
if skip {
return nil
}
if err != nil {
return errors.Trace(err)
}
Deardrops marked this conversation as resolved.
Show resolved Hide resolved

job := &model.Job{
SchemaID: schema.ID,
TableID: tb.Meta().ID,
SchemaName: schema.Name.L,
Type: model.ActionAlterIndexVisibility,
BinlogInfo: &model.HistoryInfo{},
Args: []interface{}{indexName, invisible},
}

err = d.doDDLJob(ctx, job)
err = d.callHookOnChanged(err)
return errors.Trace(err)
}
2 changes: 2 additions & 0 deletions ddl/ddl_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,8 @@ func (w *worker) runDDLJob(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64,
ver, err = onUpdateFlashReplicaStatus(t, job)
case model.ActionCreateSequence:
ver, err = onCreateSequence(d, t, job)
case model.ActionAlterIndexVisibility:
ver, err = onAlterIndexVisibility(t, job)
default:
// Invalid job, cancel it.
job.State = model.JobStateCancelled
Expand Down
39 changes: 38 additions & 1 deletion ddl/ddl_worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,9 @@ func buildCancelJobTests(firstID int64) []testCancelJob {
{act: model.ActionDropColumns, jobIDs: []int64{firstID + 42}, cancelRetErrs: []error{admin.ErrCannotCancelDDLJob.GenWithStackByArgs(firstID + 42)}, cancelState: model.StateDeleteOnly},
{act: model.ActionDropColumns, jobIDs: []int64{firstID + 43}, cancelRetErrs: []error{admin.ErrCannotCancelDDLJob.GenWithStackByArgs(firstID + 43)}, cancelState: model.StateWriteOnly},
{act: model.ActionDropColumns, jobIDs: []int64{firstID + 44}, cancelRetErrs: []error{admin.ErrCannotCancelDDLJob.GenWithStackByArgs(firstID + 44)}, cancelState: model.StateWriteReorganization},

{act: model.ActionAlterIndexVisibility, jobIDs: []int64{firstID + 46}, cancelRetErrs: noErrs, cancelState: model.StateNone},
{act: model.ActionAlterIndexVisibility, jobIDs: []int64{firstID + 47}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob.GenWithStackByArgs(firstID + 47)}, cancelState: model.StatePublic},
}

return tests
Expand Down Expand Up @@ -524,7 +527,17 @@ func checkColumnsNotFound(t table.Table, colNames []string) bool {
return notFound
}

func (s *testDDLSuite) TestCancelJob(c *C) {
func checkIdxVisibility(changedTable table.Table, idxName string, expected bool) (flag bool){
for _, idxInfo := range changedTable.Meta().Indices {
if idxInfo.Name.O == idxName && idxInfo.Invisible == expected {
flag = true
break
Deardrops marked this conversation as resolved.
Show resolved Hide resolved
}
}
return
}

func (s *testDDLSuite) TestCancelJob1(c *C) {
zimulala marked this conversation as resolved.
Show resolved Hide resolved
store := testCreateStore(c, "test_cancel_job")
defer store.Close()
d := newDDL(
Expand Down Expand Up @@ -939,6 +952,30 @@ func (s *testDDLSuite) TestCancelJob(c *C) {
testDropColumns(c, ctx, d, dbInfo, tblInfo, dropColNames, false)
c.Check(errors.ErrorStack(checkErr), Equals, "")
s.checkCancelDropColumns(c, d, dbInfo.ID, tblInfo.ID, dropColNames, true)

// test alter index visibility failed caused by canceled.
indexName := "idx_c3"
testCreateIndex(c, ctx, d, dbInfo, tblInfo, false, indexName, "c3")
c.Check(errors.ErrorStack(checkErr), Equals, "")
txn, err = ctx.Txn(true)
c.Assert(err, IsNil)
c.Assert(txn.Commit(context.Background()), IsNil)
s.checkAddIdx(c, d, dbInfo.ID, tblInfo.ID, indexName, true)

updateTest(&tests[41])
alterIndexVisibility := []interface{}{model.NewCIStr(indexName), true}
doDDLJobErrWithSchemaState(ctx, d, c, dbInfo.ID, tblInfo.ID, test.act, alterIndexVisibility, &test.cancelState)
c.Check(checkErr, IsNil)
changedTable = testGetTable(c, d, dbInfo.ID, tblInfo.ID)
c.Assert(checkIdxVisibility(changedTable, indexName, false), IsTrue)

// cancel alter index visibility successfully
updateTest(&tests[42])
alterIndexVisibility = []interface{}{model.NewCIStr(indexName), true}
doDDLJobSuccess(ctx, d, c, dbInfo.ID, tblInfo.ID, test.act, alterIndexVisibility)
c.Check(checkErr, IsNil)
changedTable = testGetTable(c, d, dbInfo.ID, tblInfo.ID)
c.Assert(checkIdxVisibility(changedTable, indexName, true), IsTrue)
}

func (s *testDDLSuite) TestIgnorableSpec(c *C) {
Expand Down
49 changes: 49 additions & 0 deletions ddl/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,30 @@ func onRenameIndex(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 {
return false, errors.Trace(infoschema.ErrKeyNotExists.GenWithStackByArgs(indexName.O, tbl.Name))
} else if idx.Invisible == invisible {
return true, nil
}
return false, nil
}

func onAlterIndexVisibility(t *meta.Meta, job *model.Job) (ver int64, _ error) {
tblInfo, from, invisible, err := checkAlterIndexVisibility(t, job)
if err != nil {
return ver, errors.Trace(err)
}
idx := tblInfo.FindIndexByName(from.L)
idx.Invisible = invisible
if ver, err = updateVersionAndTableInfo(t, job, tblInfo, true); err != nil {
job.State = model.JobStateCancelled
return ver, errors.Trace(err)
}
job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tblInfo)
return ver, nil
}

func getNullColInfos(tblInfo *model.TableInfo, indexInfo *model.IndexInfo) ([]*model.ColumnInfo, error) {
nullCols := make([]*model.ColumnInfo, 0, len(indexInfo.Columns))
for _, colName := range indexInfo.Columns {
Expand Down Expand Up @@ -712,6 +736,31 @@ func checkRenameIndex(t *meta.Meta, job *model.Job) (*model.TableInfo, model.CIS
return tblInfo, from, to, errors.Trace(err)
}

func checkAlterIndexVisibility(t *meta.Meta, job *model.Job) (*model.TableInfo, model.CIStr, bool, error) {
var (
indexName model.CIStr
invisible bool
)

schemaID := job.SchemaID
tblInfo, err := getTableInfoAndCancelFaultJob(t, job, schemaID)
if err != nil {
return nil, indexName, invisible, errors.Trace(err)
}

if err := job.DecodeArgs(&indexName, &invisible); err != nil {
job.State = model.JobStateCancelled
return nil, indexName, invisible, errors.Trace(err)
}

_, err = validateAlterIndexVisibility(indexName, invisible, tblInfo)
if err != nil {
job.State = model.JobStateCancelled
return nil, indexName, invisible, errors.Trace(err)
}
Deardrops marked this conversation as resolved.
Show resolved Hide resolved
return tblInfo, indexName, invisible, nil
}

const (
// DefaultTaskHandleCnt is default batch size of adding indices.
DefaultTaskHandleCnt = 128
Expand Down
3 changes: 2 additions & 1 deletion ddl/rollingback.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,8 @@ func convertJob2RollbackJob(w *worker, d *ddlCtx, t *meta.Meta, job *model.Job)
model.ActionModifyColumn, model.ActionAddForeignKey,
model.ActionDropForeignKey, model.ActionRenameTable,
model.ActionModifyTableCharsetAndCollate, model.ActionTruncateTablePartition,
model.ActionModifySchemaCharsetAndCollate, model.ActionRepairTable, model.ActionModifyTableAutoIdCache:
model.ActionModifySchemaCharsetAndCollate, model.ActionRepairTable,
model.ActionModifyTableAutoIdCache, model.ActionAlterIndexVisibility:
Deardrops marked this conversation as resolved.
Show resolved Hide resolved
ver, err = cancelOnlyNotHandledJob(job)
default:
job.State = model.JobStateCancelled
Expand Down