From 0ec31eae84fa825c36df17ed3e4aa4011b7e90d2 Mon Sep 17 00:00:00 2001 From: Tanner Date: Tue, 23 Jul 2019 15:13:24 +0800 Subject: [PATCH 1/2] ddl: disallow dropping index on auto_increment column (#11360) --- ddl/db_integration_test.go | 24 ++++++++++++++++++++++++ ddl/ddl_api.go | 10 +++++++++- meta/autoid/errors.go | 2 ++ planner/core/preprocess.go | 3 ++- planner/core/preprocess_test.go | 6 +++--- 5 files changed, 40 insertions(+), 5 deletions(-) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index c55617358bdca..77d67fb6f8d52 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -16,6 +16,7 @@ package ddl_test import ( "context" "fmt" + "strconv" "strings" . "github.com/pingcap/check" @@ -29,6 +30,7 @@ import ( "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/meta" + "github.com/pingcap/tidb/mysql" "github.com/pingcap/tidb/session" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/stmtctx" @@ -803,3 +805,25 @@ func (s *testIntegrationSuite) TestChangingDBCharset(c *C) { tk.MustExec("ALTER SCHEMA CHARACTER SET = 'utf8mb4' COLLATE = 'utf8mb4_general_ci'") verifyDBCharsetAndCollate("alterdb2", "utf8mb4", "utf8mb4_general_ci") } + +func (s *testIntegrationSuite) TestDropAutoIncrementIndex(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("create database if not exists test") + tk.MustExec("use test") + + assertErrorCode := func(sql string, errCodeStr int) { + _, err := tk.Exec(sql) + c.Assert(err, NotNil) + c.Assert(err.Error()[:len(strconv.Itoa(errCodeStr))], Equals, errCodeStr) + } + + tk.MustExec("drop table if exists t1") + tk.MustExec("create table t1 (a int auto_increment, unique key (a))") + dropIndexSQL := "alter table t1 drop index a" + assertErrorCode(dropIndexSQL, mysql.ErrWrongAutoKey) + + tk.MustExec("drop table if exists t1") + tk.MustExec("create table t1 (a int(11) not null auto_increment, b int(11), c bigint, unique key (a, b, c))") + dropIndexSQL = "alter table t1 drop index a" + assertErrorCode(dropIndexSQL, mysql.ErrWrongAutoKey) +} diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 66c5707b30cdb..14b5000df8eb4 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -2585,10 +2585,18 @@ func (d *ddl) DropIndex(ctx sessionctx.Context, ti ast.Ident, indexName model.CI return errors.Trace(infoschema.ErrTableNotExists.GenWithStackByArgs(ti.Schema, ti.Name)) } - if indexInfo := schemautil.FindIndexByName(indexName.L, t.Meta().Indices); indexInfo == nil { + indexInfo := schemautil.FindIndexByName(indexName.L, t.Meta().Indices) + if indexInfo == nil { return ErrCantDropFieldOrKey.GenWithStack("index %s doesn't exist", indexName) } + cols := t.Cols() + for _, idxCol := range indexInfo.Columns { + if mysql.HasAutoIncrementFlag(cols[idxCol.Offset].Flag) { + return autoid.ErrWrongAutoKey + } + } + job := &model.Job{ SchemaID: schema.ID, TableID: t.Meta().ID, diff --git a/meta/autoid/errors.go b/meta/autoid/errors.go index 44ef83650a202..e9b0b0fa6fae4 100644 --- a/meta/autoid/errors.go +++ b/meta/autoid/errors.go @@ -21,12 +21,14 @@ import ( // Error instances. var ( ErrAutoincReadFailed = terror.ClassAutoid.New(mysql.ErrAutoincReadFailed, mysql.MySQLErrName[mysql.ErrAutoincReadFailed]) + ErrWrongAutoKey = terror.ClassAutoid.New(mysql.ErrWrongAutoKey, mysql.MySQLErrName[mysql.ErrWrongAutoKey]) ) func init() { // Map error codes to mysql error codes. tableMySQLErrCodes := map[terror.ErrCode]uint16{ mysql.ErrAutoincReadFailed: mysql.ErrAutoincReadFailed, + mysql.ErrWrongAutoKey: mysql.ErrWrongAutoKey, } terror.ErrClassToMySQLCodes[terror.ClassAutoid] = tableMySQLErrCodes } diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index 410c5c79e08b3..d157550d36ebe 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -24,6 +24,7 @@ import ( "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/infoschema" + "github.com/pingcap/tidb/meta/autoid" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/types/parser_driver" @@ -220,7 +221,7 @@ func (p *preprocessor) checkAutoIncrement(stmt *ast.CreateTableStmt) { } } if (autoIncrementMustBeKey && !isKey) || count > 1 { - p.err = errors.New("Incorrect table definition; there can be only one auto column and it must be defined as a key") + p.err = autoid.ErrWrongAutoKey.GenWithStackByArgs() } switch autoIncrementCol.Tp.Tp { diff --git a/planner/core/preprocess_test.go b/planner/core/preprocess_test.go index 3c8f7c6561955..7f15f23b29e69 100644 --- a/planner/core/preprocess_test.go +++ b/planner/core/preprocess_test.go @@ -52,11 +52,11 @@ func (s *testValidatorSuite) TestValidator(c *C) { {"create table t(id int auto_increment default null, primary key (id))", true, nil}, {"create table t(id int default null auto_increment, primary key (id))", true, nil}, {"create table t(id int not null auto_increment)", true, - errors.New("Incorrect table definition; there can be only one auto column and it must be defined as a key")}, + errors.New("[autoid:1075]Incorrect table definition; there can be only one auto column and it must be defined as a key")}, {"create table t(id int not null auto_increment, c int auto_increment, key (id, c))", true, - errors.New("Incorrect table definition; there can be only one auto column and it must be defined as a key")}, + errors.New("[autoid:1075]Incorrect table definition; there can be only one auto column and it must be defined as a key")}, {"create table t(id int not null auto_increment, c int, key (c, id))", true, - errors.New("Incorrect table definition; there can be only one auto column and it must be defined as a key")}, + errors.New("[autoid:1075]Incorrect table definition; there can be only one auto column and it must be defined as a key")}, {"create table t(id decimal auto_increment, key (id))", true, errors.New("Incorrect column specifier for column 'id'")}, {"create table t(id float auto_increment, key (id))", true, nil}, From 0f2d478f83bb7321e543b3ba305017ed4cd97682 Mon Sep 17 00:00:00 2001 From: tangenta Date: Wed, 24 Jul 2019 14:03:20 +0800 Subject: [PATCH 2/2] update test --- ddl/db_integration_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index 77d67fb6f8d52..41b69f9e094cd 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -16,7 +16,6 @@ package ddl_test import ( "context" "fmt" - "strconv" "strings" . "github.com/pingcap/check" @@ -30,7 +29,6 @@ import ( "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/meta" - "github.com/pingcap/tidb/mysql" "github.com/pingcap/tidb/session" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/stmtctx" @@ -811,19 +809,20 @@ func (s *testIntegrationSuite) TestDropAutoIncrementIndex(c *C) { tk.MustExec("create database if not exists test") tk.MustExec("use test") - assertErrorCode := func(sql string, errCodeStr int) { + assertErrMsg := func(sql string) { _, err := tk.Exec(sql) c.Assert(err, NotNil) - c.Assert(err.Error()[:len(strconv.Itoa(errCodeStr))], Equals, errCodeStr) + errMsg := "[autoid:1075]Incorrect table definition; there can be only one auto column and it must be defined as a key" + c.Assert(err.Error(), Equals, errMsg) } tk.MustExec("drop table if exists t1") tk.MustExec("create table t1 (a int auto_increment, unique key (a))") dropIndexSQL := "alter table t1 drop index a" - assertErrorCode(dropIndexSQL, mysql.ErrWrongAutoKey) + assertErrMsg(dropIndexSQL) tk.MustExec("drop table if exists t1") tk.MustExec("create table t1 (a int(11) not null auto_increment, b int(11), c bigint, unique key (a, b, c))") dropIndexSQL = "alter table t1 drop index a" - assertErrorCode(dropIndexSQL, mysql.ErrWrongAutoKey) + assertErrMsg(dropIndexSQL) }