ddl: index name should not be primary (pingcap#3834) (pingcap#4030)

crazycs520 · Aug 6, 2017 · bae8fe4 · bae8fe4
1 parent e9785fd
commit bae8fe4
Show file tree

Hide file tree

Showing 8 changed files with 43 additions and 12 deletions.
diff --git a/ddl/ddl.go b/ddl/ddl.go
@@ -116,6 +116,8 @@ var (
 	ErrWrongTableName = terror.ClassDDL.New(codeWrongTableName, mysql.MySQLErrName[mysql.ErrWrongTableName])
 	// ErrWrongColumnName returns for wrong column name.
 	ErrWrongColumnName = terror.ClassDDL.New(codeWrongColumnName, mysql.MySQLErrName[mysql.ErrWrongColumnName])
+	// ErrWrongNameForIndex returns for wrong index name.
+	ErrWrongNameForIndex = terror.ClassDDL.New(codeWrongNameForIndex, mysql.MySQLErrName[mysql.ErrWrongNameForIndex])
 )
 
 // DDL is responsible for updating schema in data store and maintaining in-memory InfoSchema cache.
@@ -527,6 +529,7 @@ const (
 	codeGeneratedColumnNonPrior      = 3107
 	codeDependentByGeneratedColumn   = 3108
 	codeJSONUsedAsKey                = 3152
+	codeWrongNameForIndex            = terror.ErrCode(mysql.ErrWrongNameForIndex)
 )
 
 func init() {
@@ -555,6 +558,7 @@ func init() {
 		codeBlobCantHaveDefault:          mysql.ErrBlobCantHaveDefault,
 		codeWrongColumnName:              mysql.ErrWrongColumnName,
 		codeWrongKeyColumn:               mysql.ErrWrongKeyColumn,
+		codeWrongNameForIndex:            mysql.ErrWrongNameForIndex,
 	}
 	terror.ErrClassToMySQLCodes[terror.ClassDDL] = ddlMySQLErrCodes
 }
diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go
@@ -498,6 +498,10 @@ func setEmptyConstraintName(namesMap map[string]bool, constr *ast.Constraint, fo
 		colName := constr.Keys[0].Column.Name.L
 		constrName := colName
 		i := 2
+		if strings.EqualFold(constrName, mysql.PrimaryKeyName) {
+			constrName = fmt.Sprintf("%s_%d", constrName, 2)
+			i = 3
+		}
 		for namesMap[constrName] {
 			// We loop forever until we find constrName that haven't been used.
 			if foreign {
@@ -620,7 +624,7 @@ func (d *ddl) buildTableInfo(tableName model.CIStr, cols []*table.Column, constr
 		case ast.ConstraintPrimaryKey:
 			idxInfo.Primary = true
 			idxInfo.Unique = true
-			idxInfo.Name = model.NewCIStr(table.PrimaryKeyName)
+			idxInfo.Name = model.NewCIStr(mysql.PrimaryKeyName)
 		case ast.ConstraintUniq, ast.ConstraintUniqKey, ast.ConstraintUniqIndex:
 			idxInfo.Unique = true
 		}

diff --git a/ddl/ddl_db_test.go b/ddl/ddl_db_test.go
@@ -323,6 +323,18 @@ func (s *testDBSuite) testAddAnonymousIndex(c *C) {
 	s.mustExec(c, "alter table t_anonymous_index drop index c3")
 	s.mustExec(c, "alter table t_anonymous_index add index c3 (C3)")
 	s.mustExec(c, "alter table t_anonymous_index drop index C3")
+	// for anonymous index with column name `primary`
+	s.mustExec(c, "create table t_primary (`primary` int, key (`primary`))")
+	t = s.testGetTable(c, "t_primary")
+	c.Assert(t.Indices()[0].Meta().Name.String(), Equals, "primary_2")
+	s.mustExec(c, "create table t_primary_2 (`primary` int, key primary_2 (`primary`), key (`primary`))")
+	t = s.testGetTable(c, "t_primary_2")
+	c.Assert(t.Indices()[0].Meta().Name.String(), Equals, "primary_2")
+	c.Assert(t.Indices()[1].Meta().Name.String(), Equals, "primary_3")
+	s.mustExec(c, "create table t_primary_3 (`primary_2` int, key(`primary_2`), `primary` int, key(`primary`));")
+	t = s.testGetTable(c, "t_primary_3")
+	c.Assert(t.Indices()[0].Meta().Name.String(), Equals, "primary_2")
+	c.Assert(t.Indices()[1].Meta().Name.String(), Equals, "primary_3")
 }
 
 func (s *testDBSuite) testAlterLock(c *C) {

diff --git a/infoschema/tables.go b/infoschema/tables.go
@@ -813,7 +813,7 @@ func dataForTableConstraints(schemas []*model.DBInfo) [][]types.Datum {
 				record := types.MakeDatums(
 					catalogVal,           // CONSTRAINT_CATALOG
 					schema.Name.O,        // CONSTRAINT_SCHEMA
-					table.PrimaryKeyName, // CONSTRAINT_NAME
+					mysql.PrimaryKeyName, // CONSTRAINT_NAME
 					schema.Name.O,        // TABLE_SCHEMA
 					tbl.Name.O,           // TABLE_NAME
 					primaryKeyType,       // CONSTRAINT_TYPE
@@ -824,7 +824,7 @@ func dataForTableConstraints(schemas []*model.DBInfo) [][]types.Datum {
 			for _, idx := range tbl.Indices {
 				var cname, ctype string
 				if idx.Primary {
-					cname = table.PrimaryKeyName
+					cname = mysql.PrimaryKeyName
 					ctype = primaryKeyType
 				} else if idx.Unique {
 					cname = idx.Name.O

diff --git a/mysql/const.go b/mysql/const.go
@@ -482,3 +482,8 @@ const (
 	HighPriority
 	DelayedPriority
 )
+
+// PrimaryKeyName defines primary key name.
+const (
+	PrimaryKeyName = "PRIMARY"
+)
diff --git a/plan/validator.go b/plan/validator.go
@@ -261,7 +261,7 @@ func (v *validator) checkCreateTableGrammar(stmt *ast.CreateTableStmt) {
 	for _, constraint := range stmt.Constraints {
 		switch tp := constraint.Tp; tp {
 		case ast.ConstraintKey, ast.ConstraintIndex, ast.ConstraintUniq, ast.ConstraintUniqKey, ast.ConstraintUniqIndex:
-			err := checkDuplicateColumnName(constraint.Keys)
+			err := checkIndexInfo(constraint.Name, constraint.Keys)
 			if err != nil {
 				v.err = err
 				return
@@ -272,7 +272,7 @@ func (v *validator) checkCreateTableGrammar(stmt *ast.CreateTableStmt) {
 				return
 			}
 			countPrimaryKey++
-			err := checkDuplicateColumnName(constraint.Keys)
+			err := checkIndexInfo(constraint.Name, constraint.Keys)
 			if err != nil {
 				v.err = err
 				return
@@ -310,9 +310,7 @@ func (v *validator) checkCreateIndexGrammar(stmt *ast.CreateIndexStmt) {
 		v.err = ddl.ErrWrongTableName.GenByArgs(tName)
 		return
 	}
-	// We do not check column name here, due to MySQL returns "ERROR 1072 (42000): Key column 'a ' doesn't exist in table",
-	// if column name is `` or contains space at the end.
-	v.err = checkDuplicateColumnName(stmt.IndexColNames)
+	v.err = checkIndexInfo(stmt.IndexName, stmt.IndexColNames)
 	return
 }
 
@@ -347,7 +345,7 @@ func (v *validator) checkAlterTableGrammar(stmt *ast.AlterTableStmt) {
 			switch spec.Constraint.Tp {
 			case ast.ConstraintKey, ast.ConstraintIndex, ast.ConstraintUniq, ast.ConstraintUniqIndex,
 				ast.ConstraintUniqKey:
-				v.err = checkDuplicateColumnName(spec.Constraint.Keys)
+				v.err = checkIndexInfo(spec.Constraint.Name, spec.Constraint.Keys)
 				if v.err != nil {
 					return
 				}
@@ -381,6 +379,14 @@ func checkDuplicateColumnName(indexColNames []*ast.IndexColName) error {
 	return nil
 }
 
+// checkIndexInfo checks index name and index column names.
+func checkIndexInfo(indexName string, indexColNames []*ast.IndexColName) error {
+	if strings.EqualFold(indexName, mysql.PrimaryKeyName) {
+		return ddl.ErrWrongNameForIndex.GenByArgs(indexName)
+	}
+	return checkDuplicateColumnName(indexColNames)
+}
+
 // checkColumn checks if the column definition is valid.
 // See https://dev.mysql.com/doc/refman/5.7/en/storage-requirements.html
 func checkColumn(colDef *ast.ColumnDef) error {

diff --git a/plan/validator_test.go b/plan/validator_test.go
@@ -139,6 +139,9 @@ func (s *testValidatorSuite) TestValidator(c *C) {
 		// issue 3844
 		{`create table t (a set("a, b", "c, d"))`, true, errors.New("[types:1367]Illegal set 'a, b' value found during parsing")},
 		{`alter table t add column a set("a, b", "c, d")`, true, errors.New("[types:1367]Illegal set 'a, b' value found during parsing")},
+		// issue 3843
+		{"create index `primary` on t (i)", true, errors.New("[ddl:1280]Incorrect index name 'primary'")},
+		{"alter table t add index `primary` (i)", true, errors.New("[ddl:1280]Incorrect index name 'primary'")},
 	}
 
 	store, err := tidb.NewStore(tidb.EngineGoLevelDBMemory)

diff --git a/table/column.go b/table/column.go
@@ -39,9 +39,6 @@ type Column struct {
 	GeneratedExpr ast.ExprNode
 }
 
-// PrimaryKeyName defines primary key name.
-const PrimaryKeyName = "PRIMARY"
-
 // String implements fmt.Stringer interface.
 func (c *Column) String() string {
 	ans := []string{c.Name.O, types.TypeToStr(c.Tp, c.Charset)}