From 71979917a6ea51a847cd25c9b8278e8b491a7104 Mon Sep 17 00:00:00 2001 From: tangenta Date: Mon, 22 Jul 2019 13:20:25 +0800 Subject: [PATCH 1/5] ddl: disallow droping index on auto_increment column --- ddl/db_integration_test.go | 18 ++++++++++++++++++ ddl/ddl_api.go | 10 +++++++++- meta/autoid/errors.go | 2 ++ planner/core/preprocess.go | 3 ++- planner/core/preprocess_test.go | 6 +++--- 5 files changed, 34 insertions(+), 5 deletions(-) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index 367549acc13cb..ddd130ea190ec 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -414,6 +414,8 @@ func (s *testIntegrationSuite5) TestMySQLErrorCode(c *C) { assertErrorCode(c, tk, sql, tmysql.ErrPrimaryCantHaveNull) sql = "create table t2 (id int null, age int, primary key(id));" assertErrorCode(c, tk, sql, tmysql.ErrPrimaryCantHaveNull) + sql = "create table t2 (id int auto_increment);" + assertErrorCode(c, tk, sql, tmysql.ErrWrongAutoKey) sql = "create table t2 (id int primary key , age int);" tk.MustExec(sql) @@ -1803,3 +1805,19 @@ func (s *testIntegrationSuite5) TestChangingDBCharset(c *C) { tk.MustExec("ALTER SCHEMA CHARACTER SET = 'utf8mb4' COLLATE = 'utf8mb4_general_ci'") verifyDBCharsetAndCollate("alterdb2", "utf8mb4", "utf8mb4_general_ci") } + +func (s *testIntegrationSuite4) TestDropAutoIncrementIndex(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("create database if not exists test") + tk.MustExec("use test") + + 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(c, tk, 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(c, tk, dropIndexSql, mysql.ErrWrongAutoKey) +} diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 5a930f6aafcf6..5ac435621783a 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -3263,7 +3263,8 @@ 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 := t.Meta().FindIndexByName(indexName.L); indexInfo == nil { + var indexInfo *model.IndexInfo + if indexInfo = t.Meta().FindIndexByName(indexName.L); indexInfo == nil { err = ErrCantDropFieldOrKey.GenWithStack("index %s doesn't exist", indexName) if ifExists { ctx.GetSessionVars().StmtCtx.AppendNote(err) @@ -3272,6 +3273,13 @@ func (d *ddl) DropIndex(ctx sessionctx.Context, ti ast.Ident, indexName model.CI return err } + cols := t.Cols() + for _, idxCol := range indexInfo.Columns { + if mysql.HasAutoIncrementFlag(cols[idxCol.Offset].Flag) { + return errors.Trace(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 770ba6f683e5a..81071fbd6899f 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -15,6 +15,7 @@ package core import ( "fmt" + "github.com/pingcap/tidb/meta/autoid" "math" "strings" @@ -295,7 +296,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 fc652943c3821..01d04b9261208 100644 --- a/planner/core/preprocess_test.go +++ b/planner/core/preprocess_test.go @@ -53,11 +53,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 af1a531dfe514c9a30cfe973c7bfe3b59df8da36 Mon Sep 17 00:00:00 2001 From: tangenta Date: Mon, 22 Jul 2019 13:27:54 +0800 Subject: [PATCH 2/5] fix naming problem --- ddl/db_integration_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index ddd130ea190ec..00fb782f76f39 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -1813,11 +1813,11 @@ func (s *testIntegrationSuite4) TestDropAutoIncrementIndex(c *C) { 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(c, tk, dropIndexSql, mysql.ErrWrongAutoKey) + dropIndexSQL := "alter table t1 drop index a" + assertErrorCode(c, tk, 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(c, tk, dropIndexSql, mysql.ErrWrongAutoKey) + dropIndexSQL = "alter table t1 drop index a" + assertErrorCode(c, tk, dropIndexSQL, mysql.ErrWrongAutoKey) } From 440df4ef7be24ed28b8f2c576d8166d000b3594f Mon Sep 17 00:00:00 2001 From: tangenta Date: Mon, 22 Jul 2019 14:22:37 +0800 Subject: [PATCH 3/5] update import --- planner/core/preprocess.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index 81071fbd6899f..d902ffba1f8e3 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -15,7 +15,6 @@ package core import ( "fmt" - "github.com/pingcap/tidb/meta/autoid" "math" "strings" @@ -27,6 +26,7 @@ import ( "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/expression" "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" From b815c1bd238d1892e2c30230179037be2cdd2747 Mon Sep 17 00:00:00 2001 From: tangenta Date: Tue, 23 Jul 2019 10:28:12 +0800 Subject: [PATCH 4/5] address comment --- ddl/ddl_api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 5ac435621783a..c6296d5c6eeac 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -3263,8 +3263,8 @@ func (d *ddl) DropIndex(ctx sessionctx.Context, ti ast.Ident, indexName model.CI return errors.Trace(infoschema.ErrTableNotExists.GenWithStackByArgs(ti.Schema, ti.Name)) } - var indexInfo *model.IndexInfo - if indexInfo = t.Meta().FindIndexByName(indexName.L); indexInfo == nil { + indexInfo := t.Meta().FindIndexByName(indexName.L) + if indexInfo == nil { err = ErrCantDropFieldOrKey.GenWithStack("index %s doesn't exist", indexName) if ifExists { ctx.GetSessionVars().StmtCtx.AppendNote(err) From 2aaf11720ef4a9f16942e7eb194701304904f929 Mon Sep 17 00:00:00 2001 From: tangenta Date: Tue, 23 Jul 2019 12:06:08 +0800 Subject: [PATCH 5/5] address comment --- ddl/ddl_api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index c6296d5c6eeac..90eec860e4bbc 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -3276,7 +3276,7 @@ func (d *ddl) DropIndex(ctx sessionctx.Context, ti ast.Ident, indexName model.CI cols := t.Cols() for _, idxCol := range indexInfo.Columns { if mysql.HasAutoIncrementFlag(cols[idxCol.Offset].Flag) { - return errors.Trace(autoid.ErrWrongAutoKey) + return autoid.ErrWrongAutoKey } }