From 734cbea10953dbe3747de44588400128112af335 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Tue, 17 Sep 2019 20:20:36 +0800 Subject: [PATCH 1/2] ddl: fix drop index failed when index contain auto_increment column and more than 2 index contain auto_increment_column --- ddl/db_change_test.go | 12 +++++++++++- ddl/ddl_api.go | 17 ++++++++++++++++- ddl/index.go | 11 +++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/ddl/db_change_test.go b/ddl/db_change_test.go index 0c8e8e435903b..fc9db586a6a69 100644 --- a/ddl/db_change_test.go +++ b/ddl/db_change_test.go @@ -754,6 +754,16 @@ func (s *testStateChangeSuite) TestParallelDropColumn(c *C) { s.testControlParallelExecSQL(c, sql, sql, f) } +func (s *testStateChangeSuite) TestParallelDropIndex(c *C) { + sql1 := "alter table t drop index idx1 ;" + sql2 := "alter table t drop index idx2 ;" + f := func(c *C, err1, err2 error) { + c.Assert(err1, IsNil) + c.Assert(err2.Error(), Equals, "[autoid:1075]Incorrect table definition; there can be only one auto column and it must be defined as a key") + } + s.testControlParallelExecSQL(c, sql1, sql2, f) +} + func (s *testStateChangeSuite) TestParallelCreateAndRename(c *C) { sql1 := "create table t_exists(c int);" sql2 := "alter table t rename to t_exists;" @@ -770,7 +780,7 @@ type checkRet func(c *C, err1, err2 error) func (s *testStateChangeSuite) testControlParallelExecSQL(c *C, sql1, sql2 string, f checkRet) { _, err := s.se.Execute(context.Background(), "use test_db_state") c.Assert(err, IsNil) - _, err = s.se.Execute(context.Background(), "create table t(a int, b int, c int)") + _, err = s.se.Execute(context.Background(), "create table t(a int, b int, c int, d int auto_increment,e int, index idx1(d), index idx2(d,e))") c.Assert(err, IsNil) defer s.se.Execute(context.Background(), "drop table t") diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 20d0d1a67f7f0..9fbada65df704 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -3396,9 +3396,10 @@ func (d *ddl) DropIndex(ctx sessionctx.Context, ti ast.Ident, indexName model.CI return err } + // Check for drop index on auto_increment column. cols := t.Cols() for _, idxCol := range indexInfo.Columns { - if mysql.HasAutoIncrementFlag(cols[idxCol.Offset].Flag) { + if mysql.HasAutoIncrementFlag(cols[idxCol.Offset].Flag) && countOfIndexContainColumn(t.Meta(), idxCol.Name.L) < 2 { return autoid.ErrWrongAutoKey } } @@ -3422,6 +3423,20 @@ func (d *ddl) DropIndex(ctx sessionctx.Context, ti ast.Ident, indexName model.CI return errors.Trace(err) } +func countOfIndexContainColumn(tblInfo *model.TableInfo, colName string) int { + count := 0 + for _, idx := range tblInfo.Indices { + for _, c := range idx.Columns { + if c.Name.L != colName { + continue + } + count++ + break + } + } + return count +} + func isDroppableColumn(tblInfo *model.TableInfo, colName model.CIStr) error { // Check whether there are other columns depend on this column or not. for _, col := range tblInfo.Columns { diff --git a/ddl/index.go b/ddl/index.go index 9316d053254b5..cc3ba880ff3d8 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -30,6 +30,7 @@ import ( "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/meta" + "github.com/pingcap/tidb/meta/autoid" "github.com/pingcap/tidb/metrics" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/variable" @@ -465,6 +466,16 @@ func checkDropIndex(t *meta.Meta, job *model.Job) (*model.TableInfo, *model.Inde job.State = model.JobStateCancelled return nil, nil, ErrCantDropFieldOrKey.GenWithStack("index %s doesn't exist", indexName) } + + // Double check for drop index on auto_increment column. + cols := tblInfo.Columns + for _, idxCol := range indexInfo.Columns { + if mysql.HasAutoIncrementFlag(cols[idxCol.Offset].Flag) && countOfIndexContainColumn(tblInfo, idxCol.Name.L) < 2 { + job.State = model.JobStateCancelled + return nil, nil, autoid.ErrWrongAutoKey + } + } + return tblInfo, indexInfo, nil } From 0847a0d6a08316ac71479a6bc9b2ec8379f3754f Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Wed, 18 Sep 2019 10:33:00 +0800 Subject: [PATCH 2/2] address comment --- ddl/ddl_api.go | 22 +++------------------- ddl/index.go | 31 ++++++++++++++++++++++++++----- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 9fbada65df704..d09ca0746b522 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -3397,11 +3397,9 @@ func (d *ddl) DropIndex(ctx sessionctx.Context, ti ast.Ident, indexName model.CI } // Check for drop index on auto_increment column. - cols := t.Cols() - for _, idxCol := range indexInfo.Columns { - if mysql.HasAutoIncrementFlag(cols[idxCol.Offset].Flag) && countOfIndexContainColumn(t.Meta(), idxCol.Name.L) < 2 { - return autoid.ErrWrongAutoKey - } + err = checkDropIndexOnAutoIncrementColumn(t.Meta(), indexInfo) + if err != nil { + return errors.Trace(err) } job := &model.Job{ @@ -3423,20 +3421,6 @@ func (d *ddl) DropIndex(ctx sessionctx.Context, ti ast.Ident, indexName model.CI return errors.Trace(err) } -func countOfIndexContainColumn(tblInfo *model.TableInfo, colName string) int { - count := 0 - for _, idx := range tblInfo.Indices { - for _, c := range idx.Columns { - if c.Name.L != colName { - continue - } - count++ - break - } - } - return count -} - func isDroppableColumn(tblInfo *model.TableInfo, colName model.CIStr) error { // Check whether there are other columns depend on this column or not. for _, col := range tblInfo.Columns { diff --git a/ddl/index.go b/ddl/index.go index cc3ba880ff3d8..e6aaeee58a060 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -468,15 +468,36 @@ func checkDropIndex(t *meta.Meta, job *model.Job) (*model.TableInfo, *model.Inde } // Double check for drop index on auto_increment column. + err = checkDropIndexOnAutoIncrementColumn(tblInfo, indexInfo) + if err != nil { + job.State = model.JobStateCancelled + return nil, nil, autoid.ErrWrongAutoKey + } + + return tblInfo, indexInfo, nil +} + +func checkDropIndexOnAutoIncrementColumn(tblInfo *model.TableInfo, indexInfo *model.IndexInfo) error { cols := tblInfo.Columns for _, idxCol := range indexInfo.Columns { - if mysql.HasAutoIncrementFlag(cols[idxCol.Offset].Flag) && countOfIndexContainColumn(tblInfo, idxCol.Name.L) < 2 { - job.State = model.JobStateCancelled - return nil, nil, autoid.ErrWrongAutoKey + if !mysql.HasAutoIncrementFlag(cols[idxCol.Offset].Flag) { + continue + } + // check the count of index on auto_increment column. + count := 0 + for _, idx := range tblInfo.Indices { + for _, c := range idx.Columns { + if c.Name.L == idxCol.Name.L { + count++ + break + } + } + } + if count < 2 { + return autoid.ErrWrongAutoKey } } - - return tblInfo, indexInfo, nil + return nil } func checkRenameIndex(t *meta.Meta, job *model.Job) (*model.TableInfo, model.CIStr, model.CIStr, error) {