match mysql fk name generation (#7711)

go/go.mod

@@ -57,7 +57,7 @@ require (
 	github.com/cespare/xxhash v1.1.0
 	github.com/creasty/defaults v1.6.0
 	github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
-	github.com/dolthub/go-mysql-server v0.18.2-0.20240409193903-6b139dee1c42
+	github.com/dolthub/go-mysql-server v0.18.2-0.20240409224550-bfde35b66568
 	github.com/dolthub/swiss v0.1.0
 	github.com/goccy/go-json v0.10.2
 	github.com/google/go-github/v57 v57.0.0

go/go.sum

@@ -183,8 +183,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U=
 github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0=
 github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw1+y/N5SSCkma7FhAPw7KeGmD6c9PBZW9Y=
 github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168=
-github.com/dolthub/go-mysql-server v0.18.2-0.20240409193903-6b139dee1c42 h1:IrKAURAyI9SQt0VVm0U2PkagYsNK9c88CGHGKdSmthE=
-github.com/dolthub/go-mysql-server v0.18.2-0.20240409193903-6b139dee1c42/go.mod h1:uTqIti1oPqKEUS9N1zcT43a/QQ8n0PuBdZUMZziBaGU=
+github.com/dolthub/go-mysql-server v0.18.2-0.20240409224550-bfde35b66568 h1:H2VdUQThLeXRdfKma4Llh9YjB6Ba76twe9MvHlDj8NM=
+github.com/dolthub/go-mysql-server v0.18.2-0.20240409224550-bfde35b66568/go.mod h1:uTqIti1oPqKEUS9N1zcT43a/QQ8n0PuBdZUMZziBaGU=
 github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514=
 github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto=
 github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71 h1:bMGS25NWAGTEtT5tOBsCuCrlYnLRKpbJVJkDbrTRhwQ=

go/libraries/doltcore/doltdb/foreign_key_coll.go

@@ -28,6 +28,8 @@ import (
 	"github.com/dolthub/dolt/go/libraries/utils/set"
 	"github.com/dolthub/dolt/go/store/hash"
 	"github.com/dolthub/dolt/go/store/types"
+
+	"github.com/dolthub/go-mysql-server/sql"
 )
 
 // ForeignKeyCollection represents the collection of foreign keys for a root value.
@@ -343,31 +345,12 @@ func NewForeignKeyCollection(keys ...ForeignKey) (*ForeignKeyCollection, error)
 // both column counts are equal. All other validation should occur before being added to the collection.
 func (fkc *ForeignKeyCollection) AddKeys(fks ...ForeignKey) error {
 	for _, key := range fks {
-		if key.Name == "" {
-			// assign a name based on the hash
-			// 8 char = 5 base32 bytes, should be collision resistant
-			// TODO: constraint names should be unique, and this isn't guaranteed to be.
-			//  This logic needs to live at the table / DB level.
-			fkHash, err := key.HashOf()
-			if err != nil {
-				return err
-			}
-			key.Name = fkHash.String()[:8]
-		}
-
 		if _, ok := fkc.GetByNameCaseInsensitive(key.Name); ok {
-			return fmt.Errorf("a foreign key with the name `%s` already exists", key.Name)
+			return sql.ErrForeignKeyDuplicateName.New(key.Name)
 		}
 		if len(key.TableColumns) != len(key.ReferencedTableColumns) {
 			return fmt.Errorf("foreign keys must have the same number of columns declared and referenced")
 		}
-		if key.IsResolved() {
-			if _, ok := fkc.GetByTags(key.TableColumns, key.ReferencedTableColumns); ok {
-				// this differs from MySQL's logic
-				return fmt.Errorf("a foreign key over columns %v and referenced columns %v already exists",
-					key.TableColumns, key.ReferencedTableColumns)
-			}
-		}
 
 		fkHash, err := key.HashOf()
 		if err != nil {

go/libraries/doltcore/doltdb/foreign_key_test.go

@@ -504,7 +504,7 @@ var foreignKeyTests = []foreignKeyTest{
 		},
 		fks: []doltdb.ForeignKey{
 			{
-				Name:                   "1ncba7pr",
+				Name:                   "child_ibfk_1",
 				TableName:              "child",
 				TableIndex:             "v1_idx",
 				TableColumns:           []uint64{1215},
@@ -529,7 +529,7 @@ var foreignKeyTests = []foreignKeyTest{
 		},
 		fks: []doltdb.ForeignKey{
 			{
-				Name:      "7nt5f9b0",
+				Name:      "new_table_ibfk_1",
 				TableName: "new_table",
 				// unnamed indexes take the column name
 				TableIndex:             "v1",
@@ -555,7 +555,7 @@ var foreignKeyTests = []foreignKeyTest{
 		},
 		fks: []doltdb.ForeignKey{
 			{
-				Name:                   "k48mbatd",
+				Name:                   "child_ibfk_1",
 				TableName:              "child",
 				TableIndex:             "v1v2_idx",
 				TableColumns:           []uint64{1215, 8734},
@@ -581,7 +581,7 @@ var foreignKeyTests = []foreignKeyTest{
 		},
 		fks: []doltdb.ForeignKey{
 			{
-				Name:                   "1ncba7pr",
+				Name:                   "child_ibfk_1",
 				TableName:              "child",
 				TableIndex:             "v1_idx",
 				TableColumns:           []uint64{1215},
@@ -594,7 +594,7 @@ var foreignKeyTests = []foreignKeyTest{
 				},
 			},
 			{
-				Name:                   "8geddp18",
+				Name:                   "child_ibfk_2",
 				TableName:              "child",
 				TableIndex:             "v2_idx",
 				TableColumns:           []uint64{8734},

go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go

@@ -176,7 +176,7 @@ var MergeScripts = []queries.ScriptTest{
 			{
 				Query: "SELECT from_root_ish, violation_type, hex(dolt_row_hash), aColumn, bColumn, CAST(violation_info as CHAR) FROM dolt_constraint_violations_aTable;",
 				Expected: []sql.Row{
-					{doltCommit, "foreign key", "13F8480978D0556FA9AE6DF5745A7ACA", 2, -1, `{"Columns":["bColumn"],"ForeignKey":"ml92huct","Index":"bColumn","OnDelete":"RESTRICT","OnUpdate":"RESTRICT","ReferencedColumns":["pk"],"ReferencedIndex":"","ReferencedTable":"parent","Table":"aTable"}`},
+					{doltCommit, "foreign key", "13F8480978D0556FA9AE6DF5745A7ACA", 2, -1, `{"Columns":["bColumn"],"ForeignKey":"atable_ibfk_1","Index":"bColumn","OnDelete":"RESTRICT","OnUpdate":"RESTRICT","ReferencedColumns":["pk"],"ReferencedIndex":"","ReferencedTable":"parent","Table":"aTable"}`},
 				},
 			},
 			{
@@ -3474,7 +3474,7 @@ var MergeArtifactsScripts = []queries.ScriptTest{
 		Assertions: []queries.ScriptTestAssertion{
 			{
 				Query:          "CALL DOLT_MERGE('right');",
-				ExpectedErrStr: "error storing constraint violation for primary key (( 1 )): another violation already exists\nnew violation: {\"Columns\":[\"col2\"],\"ForeignKey\":\"r8l98srn\",\"Index\":\"col2\",\"OnDelete\":\"RESTRICT\",\"OnUpdate\":\"RESTRICT\",\"ReferencedColumns\":[\"col2\"],\"ReferencedIndex\":\"par_col2_idx\",\"ReferencedTable\":\"parent\",\"Table\":\"child\"} old violation: ({\"Columns\":[\"col1\"],\"ForeignKey\":\"ut564qa1\",\"Index\":\"col1\",\"OnDelete\":\"RESTRICT\",\"OnUpdate\":\"RESTRICT\",\"ReferencedColumns\":[\"col1\"],\"ReferencedIndex\":\"par_col1_idx\",\"ReferencedTable\":\"parent\",\"Table\":\"child\"})",
+				ExpectedErrStr: "error storing constraint violation for primary key (( 1 )): another violation already exists\nnew violation: {\"Columns\":[\"col1\"],\"ForeignKey\":\"child_ibfk_1\",\"Index\":\"col1\",\"OnDelete\":\"RESTRICT\",\"OnUpdate\":\"RESTRICT\",\"ReferencedColumns\":[\"col1\"],\"ReferencedIndex\":\"par_col1_idx\",\"ReferencedTable\":\"parent\",\"Table\":\"child\"} old violation: ({\"Columns\":[\"col2\"],\"ForeignKey\":\"child_ibfk_2\",\"Index\":\"col2\",\"OnDelete\":\"RESTRICT\",\"OnUpdate\":\"RESTRICT\",\"ReferencedColumns\":[\"col2\"],\"ReferencedIndex\":\"par_col2_idx\",\"ReferencedTable\":\"parent\",\"Table\":\"child\"})",
 			},
 			{
 				Query:    "SELECT * from parent;",

go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go

@@ -1998,15 +1998,15 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{
 					"To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.\n" +
 					"Constraint violations: \n" +
 					"Type: Foreign Key Constraint Violation\n" +
-					"\tForeignKey: r4cf97vs,\n" +
+					"\tForeignKey: child_ibfk_1,\n" +
 					"\tTable: child,\n" +
 					"\tReferencedTable: ,\n" +
 					"\tIndex: parent_fk,\n" +
 					"\tReferencedIndex: ",
 			},
 			{
 				Query:          "/* client b */ INSERT INTO child VALUES (1, 1);",
-				ExpectedErrStr: "cannot add or update a child row - Foreign key violation on fk: `r4cf97vs`, table: `child`, referenced table: `parent`, key: `[1]`",
+				ExpectedErrStr: "cannot add or update a child row - Foreign key violation on fk: `child_ibfk_1`, table: `child`, referenced table: `parent`, key: `[1]`",
 			},
 		},
 	},

integration-tests/bats/diff.bats

@@ -681,7 +681,7 @@ SQL
 
     run dolt diff
     [ "$status" -eq 0 ]
-    [[ "$output" =~ "+  CONSTRAINT \`qvvgfpe7\` FOREIGN KEY (\`j\`) REFERENCES \`parent\` (\`i\`)" ]] || false
+    [[ "$output" =~ "+  CONSTRAINT \`child_ibfk_1\` FOREIGN KEY (\`j\`) REFERENCES \`parent\` (\`i\`)" ]] || false
 }
 
 @test "diff: new foreign key added without foreign key check, and does not resolve" {
@@ -695,7 +695,7 @@ SQL
 
     run dolt diff
     [ "$status" -eq 0 ]
-    [[ ! "$output" =~ "+  CONSTRAINT \`npsbmr30\` FOREIGN KEY (\`j\`) REFERENCES \`parent\` (\`i\`)" ]] || false
+    [[ ! "$output" =~ "+  CONSTRAINT \`child_ibfk_1\` FOREIGN KEY (\`j\`) REFERENCES \`parent\` (\`i\`)" ]] || false
 }
 
 @test "diff: existing foreign key that was resolved is deleted" {

integration-tests/bats/foreign-keys.bats

@@ -134,17 +134,17 @@ SQL
     run dolt sql -q "select constraint_name, table_name, column_name, ordinal_position, position_in_unique_constraint, referenced_table_name, referenced_column_name from information_schema.KEY_COLUMN_USAGE;" -r csv
     [[ "$output" =~ "PRIMARY,colors,id,1,,," ]] || false
     [[ "$output" =~ "PRIMARY,materials,id,1,,," ]] || false
-    [[ "$output" =~ "npjt0g99,materials,color,1,1,colors,color" ]] || false
+    [[ "$output" =~ "materials_ibfk_1,materials,color,1,1,colors,color" ]] || false
     [[ "$output" =~ "PRIMARY,objects,id,1,,," ]] || false
-    [[ "$output" =~ "ootftvit,objects,color,1,1,materials,color" ]] || false
-    [[ "$output" =~ "ootftvit,objects,material,2,2,materials,material" ]] || false
+    [[ "$output" =~ "objects_ibfk_1,objects,color,1,1,materials,color" ]] || false
+    [[ "$output" =~ "objects_ibfk_1,objects,material,2,2,materials,material" ]] || false
     [[ "$output" =~ "PRIMARY,child,id,1,,," ]] || false
     [[ "$output" =~ "PRIMARY,parent,id,1,,," ]] || false
 
     # check information_schema.TABLE_CONSTRAINTS table
     run dolt sql -q "select * from information_schema.TABLE_CONSTRAINTS where table_name = 'materials';" -r csv
     [[ "$output" =~ "def,dolt-repo-$$,PRIMARY,dolt-repo-$$,materials,PRIMARY KEY,YES" ]] || false
-    [[ "$output" =~ "def,dolt-repo-$$,npjt0g99,dolt-repo-$$,materials,FOREIGN KEY,YES" ]] || false
+    [[ "$output" =~ "def,dolt-repo-$$,materials_ibfk_1,dolt-repo-$$,materials,FOREIGN KEY,YES" ]] || false
 
     # check information_schema.TABLE_CONSTRAINTS_EXTENSIONS table
     run dolt sql -q "select constraint_name from information_schema.TABLE_CONSTRAINTS_EXTENSIONS where table_name = 'materials';" -r csv
@@ -1289,6 +1289,45 @@ SQL
     [[ "$output" =~ "child,1" ]] || false
 }
 
+@test "foreign-keys: different foreign keys with same name is schema conflict" {
+    dolt commit -Am "initial commit"
+
+    dolt checkout -b other
+    dolt sql -q "alter table child add foreign key (id) references parent (v1)"
+    dolt commit -Am "other"
+
+    dolt checkout main
+    dolt sql -q "alter table child add foreign key (id) references parent (v2)"
+    dolt commit -Am "main"
+
+    run dolt merge other
+    [ "$status" -eq "1" ]
+    [[ "$output" =~ "duplicate foreign key constraint name" ]] || false
+
+    run dolt sql -q "alter table child rename constraint foreign key child_ibfk_1 to child_ibfk_2"
+    [ "$status" -eq "0" ]
+
+    dolt commit -Am "rename"
+
+    run dolt merge other
+    [ "$status" -eq "0" ]
+}
+
+@test "foreign-keys: same foreign keys with same name is ok" {
+    dolt commit -Am "initial commit"
+
+    dolt checkout -b other
+    dolt sql -q "alter table child add foreign key (id) references parent (id)"
+    dolt commit -Am "other"
+
+    dolt checkout main
+    dolt sql -q "alter table child add foreign key (id) references parent (id)"
+    dolt commit -Am "main"
+
+    run dolt merge other
+    [ "$status" -eq "0" ]
+}
+
 @test "foreign-keys: Resolve catches violations" {
     dolt sql <<SQL
 ALTER TABLE child ADD CONSTRAINT fk_v1 FOREIGN KEY (v1) REFERENCES parent(v1);

integration-tests/mysql-client-tests/node/workbenchTests/diffs.js

@@ -272,7 +272,7 @@ export const diffTests = [
           "  `test_pk` int,\n" +
           "  PRIMARY KEY (`id`),\n" +
           "  KEY `test_pk` (`test_pk`),\n" +
-          "  CONSTRAINT `lfbtivir` FOREIGN KEY (`test_pk`) REFERENCES `test` (`pk`)\n" +
+          "  CONSTRAINT `test_info_ibfk_1` FOREIGN KEY (`test_pk`) REFERENCES `test` (`pk`)\n" +
           ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;",
         to_create_statement: "",
       },
@@ -306,7 +306,7 @@ export const diffTests = [
           "  `test_pk` int,\n" +
           "  PRIMARY KEY (`id`),\n" +
           "  KEY `test_pk` (`test_pk`),\n" +
-          "  CONSTRAINT `lfbtivir` FOREIGN KEY (`test_pk`) REFERENCES `test` (`pk`)\n" +
+          "  CONSTRAINT `test_info_ibfk_1` FOREIGN KEY (`test_pk`) REFERENCES `test` (`pk`)\n" +
           ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;",
         to_create_statement: "",
       },

integration-tests/mysql-client-tests/node/workbenchTests/table.js

@@ -127,7 +127,7 @@ export const tableTests = [
       {
         CONSTRAINT_CATALOG: "def",
         CONSTRAINT_SCHEMA: `${dbName}/main`,
-        CONSTRAINT_NAME: "lfbtivir",
+        CONSTRAINT_NAME: "test_info_ibfk_1",
         TABLE_CATALOG: "def",
         TABLE_SCHEMA: `${dbName}/main`,
         TABLE_NAME: "test_info",