have generated index and foreign key names match mysql

enginetest/queries/foreign_key_queries.go

@@ -2228,6 +2228,188 @@ var ForeignKeyTests = []ScriptTest{
 			},
 		},
 	},
+	{
+		Name: "foreign key naming",
+		SetUpScript: []string{
+			"create table theparent (i int primary key);",
+			"create table child1 (j int primary key);",
+			"create table child2 (j int primary key);",
+			"create table child3 (j int primary key);",
+			"create table child4 (j int primary key);",
+			"create table child5 (j int primary key);",
+			"create table child6 (j int primary key);",
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				Query: "alter table child1 add foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				Query: "alter table child1 add foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				Query: "alter table child1 add foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				Query: "show create table child1;",
+				Expected: []sql.Row{
+					{"child1", "CREATE TABLE `child1` (\n" +
+						"  `j` int NOT NULL,\n" +
+						"  PRIMARY KEY (`j`),\n" +
+						"  CONSTRAINT `child1_ibfk_1` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`),\n" +
+						"  CONSTRAINT `child1_ibfk_2` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`),\n" +
+						"  CONSTRAINT `child1_ibfk_3` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`)\n" +
+						") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"},
+				},
+			},
+
+			// If a key with a generated name already exists, the generated name will be one after
+			{
+				Query: "alter table child2 add constraint `child2_ibfk_1` foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				Query: "alter table child2 add foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				Query: "show create table child2;",
+				Expected: []sql.Row{
+					{"child2", "CREATE TABLE `child2` (\n" +
+						"  `j` int NOT NULL,\n" +
+						"  PRIMARY KEY (`j`),\n" +
+						"  CONSTRAINT `child2_ibfk_1` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`),\n" +
+						"  CONSTRAINT `child2_ibfk_2` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`)\n" +
+						") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"},
+				},
+			},
+
+			// For some reason, the generated name will always be the highest one
+			{
+				Query: "alter table child3 add constraint `child3_ibfk_100` foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				Query: "alter table child3 add foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				Query: "show create table child3;",
+				Expected: []sql.Row{
+					{"child3", "CREATE TABLE `child3` (\n" +
+						"  `j` int NOT NULL,\n" +
+						"  PRIMARY KEY (`j`),\n" +
+						"  CONSTRAINT `child3_ibfk_100` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`),\n" +
+						"  CONSTRAINT `child3_ibfk_101` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`)\n" +
+						") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"},
+				},
+			},
+
+			// Name generation is case-sensitive, but the name collision check is case-insensitive
+			{
+				Query: "alter table child4 add constraint `CHILD4_IBFK_1` foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				Query:       "alter table child4 add foreign key (j) references theparent (i);",
+				ExpectedErr: sql.ErrForeignKeyDuplicateName,
+			},
+			{
+				Query: "show create table child4;",
+				Expected: []sql.Row{
+					{"child4", "CREATE TABLE `child4` (\n" +
+						"  `j` int NOT NULL,\n" +
+						"  PRIMARY KEY (`j`),\n" +
+						"  CONSTRAINT `CHILD4_IBFK_1` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`)\n" +
+						") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"},
+				},
+			},
+
+			// Negatives behave weirdly
+			{
+				Query: "alter table child5 add constraint `child5_ibfk_-2` foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				// This adds -1, which is interpreted as 4294967295
+				Query: "alter table child5 add foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				// This adds 4294967296, which overflows back to 0
+				Query: "alter table child5 add foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				// This attempts to add 4294967296 again, which throws a duplicate name error
+				Query:       "alter table child5 add foreign key (j) references theparent (i);",
+				ExpectedErr: sql.ErrForeignKeyDuplicateName,
+			},
+			{
+				// foreign keys are sorted by name
+				Query: "show create table child5;",
+				Expected: []sql.Row{
+					{"child5", "CREATE TABLE `child5` (\n" +
+						"  `j` int NOT NULL,\n" +
+						"  PRIMARY KEY (`j`),\n" +
+						"  CONSTRAINT `child5_ibfk_-2` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`),\n" +
+						"  CONSTRAINT `child5_ibfk_0` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`),\n" +
+						"  CONSTRAINT `child5_ibfk_4294967295` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`)\n" +
+						") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"},
+				},
+			},
+
+			// empty string constraint names are allowed if specified explicitly
+			{
+				Query: "alter table child6 add constraint `` foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				Query: "alter table child6 add foreign key (j) references theparent (i);",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				Skip:  true, // we need parser changes to tell the difference between an empty string and a NULL
+				Query: "show create table child6;",
+				Expected: []sql.Row{
+					{"child6", "CREATE TABLE `child6` (\n" +
+						"  `j` int NOT NULL,\n" +
+						"  PRIMARY KEY (`j`),\n" +
+						"  CONSTRAINT `child6_ibfk_1` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`),\n" +
+						"  CONSTRAINT `child6_ibfk_2` FOREIGN KEY (`j`) REFERENCES `theparent` (`i`)\n" +
+						") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"},
+				},
+			},
+		},
+	},
 }
 
 var CreateForeignKeyTests = []ScriptTest{

enginetest/queries/script_queries.go

@@ -6330,6 +6330,46 @@ where
 			},
 		},
 	},
+	{
+		Name: "test index naming",
+		SetUpScript: []string{
+			"create table t (i int);",
+			"alter table t add index (i);",
+			"alter table t add index (i);",
+			"alter table t add index (i);",
+
+			"create table tt (i int);",
+			"alter table tt add index i_2(i);",
+			"alter table tt add index (i);",
+			"alter table tt add index (i);",
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				Query: "show create table t",
+				Expected: []sql.Row{
+					{"t", "CREATE TABLE `t` (\n" +
+						"  `i` int,\n" +
+						"  KEY `i` (`i`),\n" +
+						"  KEY `i_2` (`i`),\n" +
+						"  KEY `i_3` (`i`)\n" +
+						") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"},
+				},
+			},
+			{
+				// MySQL preserves the other that indexes are created
+				// We store them in a map, so we have to sort to have some consistency
+				Query: "show create table tt",
+				Expected: []sql.Row{
+					{"tt", "CREATE TABLE `tt` (\n" +
+						"  `i` int,\n" +
+						"  KEY `i` (`i`),\n" +
+						"  KEY `i_2` (`i`),\n" +
+						"  KEY `i_3` (`i`)\n" +
+						") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"},
+				},
+			},
+		},
+	},
 }
 
 var SpatialScriptTests = []ScriptTest{

memory/table.go

@@ -1769,6 +1769,9 @@ func (t *Table) GetDeclaredForeignKeys(ctx *sql.Context) ([]sql.ForeignKeyConstr
 			fks = append(fks, fk)
 		}
 	}
+	sort.Slice(fks, func(i, j int) bool {
+		return fks[i].Name < fks[j].Name
+	})
 	return fks, nil
 }
 
@@ -1795,7 +1798,7 @@ func (t *Table) AddForeignKey(ctx *sql.Context, fk sql.ForeignKeyConstraint) err
 	lowerName := strings.ToLower(fk.Name)
 	for _, key := range data.fkColl.Keys() {
 		if strings.ToLower(key.Name) == lowerName {
-			return fmt.Errorf("Constraint %s already exists", fk.Name)
+			return sql.ErrForeignKeyDuplicateName.New(fk.Name)
 		}
 	}
 	data.fkColl.AddFK(fk)
@@ -1824,7 +1827,7 @@ func (t *Table) UpdateForeignKey(ctx *sql.Context, fkName string, fk sql.Foreign
 	lowerName := strings.ToLower(fk.Name)
 	for _, key := range data.fkColl.Keys() {
 		if strings.ToLower(key.Name) == lowerName {
-			return fmt.Errorf("Constraint %s already exists", fk.Name)
+			return sql.ErrForeignKeyDuplicateName.New(fk.Name)
 		}
 	}
 	data.fkColl.AddFK(fk)
@@ -1877,7 +1880,7 @@ func (t *Table) CreateCheck(ctx *sql.Context, check *sql.CheckDefinition) error
 
 	for _, key := range data.checks {
 		if key.Name == toInsert.Name {
-			return fmt.Errorf("constraint %s already exists", toInsert.Name)
+			return sql.ErrForeignKeyDuplicateName.New(toInsert.Name)
 		}
 	}
 

sql/plan/alter_foreign_key.go

@@ -17,6 +17,7 @@ package plan
 import (
 	"fmt"
 	"sort"
+	"strconv"
 	"strings"
 
 	"github.com/dolthub/vitess/go/sqltypes"
@@ -232,6 +233,8 @@ func ResolveForeignKey(ctx *sql.Context, tbl sql.ForeignKeyTable, refTbl sql.For
 		}
 	}
 
+	// TODO: deal with empty foreign key names here
+	// TODO: remove dolt code handling this
 	// Check if the current foreign key name has already been used. Rather than checking the table first (which is the
 	// highest cost part of creating a foreign key), we'll check the name if it needs to be checked. If the foreign key
 	// was previously added, we don't need to check the name.
@@ -240,10 +243,31 @@ func ResolveForeignKey(ctx *sql.Context, tbl sql.ForeignKeyTable, refTbl sql.For
 		if err != nil {
 			return err
 		}
-		fkLowerName := strings.ToLower(fkDef.Name)
-		for _, existingFk := range existingFks {
-			if fkLowerName == strings.ToLower(existingFk.Name) {
-				return sql.ErrForeignKeyDuplicateName.New(fkDef.Name)
+
+		if len(fkDef.Name) == 0 {
+			// find the next available name
+			// negative numbers behave weirdly
+			fkNamePrefix := fmt.Sprintf("%s_ibfk_", strings.ToLower(tbl.Name()))
+			var highest uint32
+			for _, existingFk := range existingFks {
+				if strings.HasPrefix(existingFk.Name, fkNamePrefix) {
+					numStr := strings.TrimPrefix(existingFk.Name, fkNamePrefix)
+					num, err := strconv.Atoi(numStr)
+					if err != nil {
+						continue
+					}
+					if uint32(num) > highest {
+						highest = uint32(num)
+					}
+				}
+			}
+			fkDef.Name = fmt.Sprintf("%s%d", fkNamePrefix, uint32(highest)+1)
+		} else {
+			fkLowerName := strings.ToLower(fkDef.Name)
+			for _, existingFk := range existingFks {
+				if fkLowerName == strings.ToLower(existingFk.Name) {
+					return sql.ErrForeignKeyDuplicateName.New(fkDef.Name)
+				}
 			}
 		}
 	}

sql/rowexec/ddl_iters.go

@@ -1864,7 +1864,8 @@ func (b *BaseBuilder) executeAlterIndex(ctx *sql.Context, n *plan.AlterIndex) er
 			indexName = strings.Join(n.ColumnNames(), "")
 			if _, ok := indexMap[strings.ToLower(indexName)]; ok {
 				for i := 0; true; i++ {
-					newIndexName := fmt.Sprintf("%s_%d", indexName, i)
+					ii := i + 2
+					newIndexName := fmt.Sprintf("%s_%d", indexName, ii)
 					if _, ok = indexMap[strings.ToLower(newIndexName)]; !ok {
 						indexName = newIndexName
 						break