schemadiff: remove table name from FK constraint name

Signed-off-by: Shlomi Noach <[email protected]>

go/vt/schemadiff/names.go

@@ -42,6 +42,9 @@ func ExtractConstraintOriginalName(tableName string, constraintName string) stri
 	if strings.HasPrefix(constraintName, fmt.Sprintf("%s_fk_", tableName)) {
 		return constraintName[len(tableName)+1:]
 	}
+	if strings.HasPrefix(constraintName, fmt.Sprintf("%s_ibfk_", tableName)) {
+		return constraintName[len(tableName)+1:]
+	}
 	if submatch := constraintVitessNameRegexp.FindStringSubmatch(constraintName); len(submatch) > 0 {
 		return submatch[1]
 	}

go/vt/schemadiff/names_test.go

@@ -23,7 +23,7 @@ import (
 )
 
 func TestConstraintOriginalName(t *testing.T) {
-	{
+	t.Run("check", func(t *testing.T) {
 		names := []string{
 			"check1",
 			"check1_7no794p1x6zw6je1gfqmt7bca",
@@ -36,8 +36,24 @@ func TestConstraintOriginalName(t *testing.T) {
 				assert.Equal(t, "check1", originalName)
 			})
 		}
-	}
-	{
+	})
+	t.Run("ibfk", func(t *testing.T) {
+		names := []string{
+			"ibfk_1",
+			"ibfk_1_7no794p1x6zw6je1gfqmt7bca",
+			"ibfk_1_etne0g9fvf3la2myjfsdgx9bx",
+			"mytable_ibfk_1",
+		}
+		for _, name := range names {
+			t.Run(name, func(t *testing.T) {
+				originalName := ExtractConstraintOriginalName("mytable", name)
+				assert.NotEmpty(t, originalName)
+				assert.Equal(t, "ibfk_1", originalName)
+			})
+		}
+	})
+
+	t.Run("chk", func(t *testing.T) {
 		names := []string{
 			"chk_1",
 			"chk_1_7no794p1x6zw6je1gfqmt7bca",
@@ -51,9 +67,9 @@ func TestConstraintOriginalName(t *testing.T) {
 				assert.Equal(t, "chk_1", originalName)
 			})
 		}
-	}
+	})
 
-	{
+	t.Run("no change", func(t *testing.T) {
 		names := []string{
 			"check1",
 			"check_991ek3m5g69vcule23s9vnayd_check1",
@@ -70,5 +86,5 @@ func TestConstraintOriginalName(t *testing.T) {
 				assert.Equal(t, name, originalName)
 			})
 		}
-	}
+	})
 }

go/vt/vttablet/onlineddl/executor_test.go

@@ -45,11 +45,12 @@ func TestGetConstraintType(t *testing.T) {
 func TestValidateAndEditCreateTableStatement(t *testing.T) {
 	e := Executor{}
 	tt := []struct {
-		name             string
-		query            string
-		strategyOptions  string
-		expectError      string
-		countConstraints int
+		name                string
+		query               string
+		strategyOptions     string
+		expectError         string
+		countConstraints    int
+		expectConstraintMap map[string]string
 	}{
 		{
 			name: "table with FK, not allowed",
@@ -62,8 +63,7 @@ func TestValidateAndEditCreateTableStatement(t *testing.T) {
 						constraint test_fk foreign key (parent_id) references onlineddl_test_parent (id) on delete no action
 					)
 				`,
-			countConstraints: 1,
-			expectError:      schema.ErrForeignKeyFound.Error(),
+			expectError: schema.ErrForeignKeyFound.Error(),
 		},
 		{
 			name: "table with FK, allowed",
@@ -76,8 +76,25 @@ func TestValidateAndEditCreateTableStatement(t *testing.T) {
 						constraint test_fk foreign key (parent_id) references onlineddl_test_parent (id) on delete no action
 					)
 				`,
+			strategyOptions:     "--unsafe-allow-foreign-keys",
+			countConstraints:    1,
+			expectConstraintMap: map[string]string{"test_fk": "test_fk_2wtivm6zk4lthpz14g9uoyaqk"},
+		},
+		{
+			name: "table with default FK name, strip table name",
+			query: `
+			create table onlineddl_test (
+					id int auto_increment,
+					i int not null,
+					parent_id int not null,
+					primary key(id),
+					constraint onlineddl_test_ibfk_1 foreign key (parent_id) references onlineddl_test_parent (id) on delete no action
+				)
+			`,
 			strategyOptions:  "--unsafe-allow-foreign-keys",
 			countConstraints: 1,
+			// we want 'onlineddl_test_' to be stripped out:
+			expectConstraintMap: map[string]string{"onlineddl_test_ibfk_1": "ibfk_1_2wtivm6zk4lthpz14g9uoyaqk"},
 		},
 		{
 			name: "table with anonymous FK, allowed",
@@ -90,8 +107,9 @@ func TestValidateAndEditCreateTableStatement(t *testing.T) {
 						foreign key (parent_id) references onlineddl_test_parent (id) on delete no action
 					)
 				`,
-			strategyOptions:  "--unsafe-allow-foreign-keys",
-			countConstraints: 1,
+			strategyOptions:     "--unsafe-allow-foreign-keys",
+			countConstraints:    1,
+			expectConstraintMap: map[string]string{"": "fk_2wtivm6zk4lthpz14g9uoyaqk"},
 		},
 		{
 			name: "table with CHECK constraints",
@@ -107,6 +125,12 @@ func TestValidateAndEditCreateTableStatement(t *testing.T) {
 					)
 				`,
 			countConstraints: 4,
+			expectConstraintMap: map[string]string{
+				"check_1": "check_1_7dbssrkwdaxhdunwi5zj53q83",
+				"check_2": "check_2_ehg3rtk6ejvbxpucimeess30o",
+				"check_3": "check_3_0se0t8x98mf8v7lqmj2la8j9u",
+				"chk_1111033c1d2d5908bf1f956ba900b192_check_4": "chk_1111033c1d2d5908bf1f956ba900b192_c_0c2c3bxi9jp4evqrct44wg3xh",
+			},
 		},
 		{
 			name: "table with both FOREIGN and CHECK constraints",
@@ -122,6 +146,11 @@ func TestValidateAndEditCreateTableStatement(t *testing.T) {
 				`,
 			strategyOptions:  "--unsafe-allow-foreign-keys",
 			countConstraints: 3,
+			expectConstraintMap: map[string]string{
+				"check_1": "check_1_7dbssrkwdaxhdunwi5zj53q83",
+				"chk_1111033c1d2d5908bf1f956ba900b192_check_4": "chk_1111033c1d2d5908bf1f956ba900b192_c_0se0t8x98mf8v7lqmj2la8j9u",
+				"test_fk": "test_fk_2wtivm6zk4lthpz14g9uoyaqk",
+			},
 		},
 	}
 	for _, tc := range tt {
@@ -134,11 +163,12 @@ func TestValidateAndEditCreateTableStatement(t *testing.T) {
 			onlineDDL := &schema.OnlineDDL{UUID: "a5a563da_dc1a_11ec_a416_0a43f95f28a3", Table: "onlineddl_test", Options: tc.strategyOptions}
 			constraintMap, err := e.validateAndEditCreateTableStatement(context.Background(), onlineDDL, createTable)
 			if tc.expectError != "" {
-				require.Error(t, err)
 				assert.ErrorContains(t, err, tc.expectError)
-			} else {
-				assert.NoError(t, err)
+				return
 			}
+			assert.NoError(t, err)
+			assert.Equal(t, tc.expectConstraintMap, constraintMap)
+
 			uniqueConstraintNames := map[string]bool{}
 			err = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) {
 				switch node := node.(type) {
@@ -202,6 +232,12 @@ func TestValidateAndEditAlterTableStatement(t *testing.T) {
 			expect: []string{"alter table t add constraint myfk_6fmhzdlya89128u5j3xapq34i foreign key (parent_id) references onlineddl_test_parent (id) on delete no action, algorithm = copy"},
 		},
 		{
+			// strip out table name
+			alter:  "alter table t add constraint t_ibfk_1 foreign key (parent_id) references onlineddl_test_parent (id) on delete no action",
+			expect: []string{"alter table t add constraint ibfk_1_6fmhzdlya89128u5j3xapq34i foreign key (parent_id) references onlineddl_test_parent (id) on delete no action, algorithm = copy"},
+		},
+		{
+			// stript out table name
 			alter:  "alter table t add constraint t_fk_1 foreign key (parent_id) references onlineddl_test_parent (id) on delete no action",
 			expect: []string{"alter table t add constraint fk_1_6fmhzdlya89128u5j3xapq34i foreign key (parent_id) references onlineddl_test_parent (id) on delete no action, algorithm = copy"},
 		},