Online DDL: edit CONSTRAINT names in CREATE TABLE (#14517)

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

go/mysql/flavor.go

@@ -55,6 +55,7 @@ const (
 	MySQLUpgradeInServerFlavorCapability
 	DynamicRedoLogCapacityFlavorCapability // supported in MySQL 8.0.30 and above: https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-30.html
 	DisableRedoLogFlavorCapability         // supported in MySQL 8.0.21 and above: https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-21.html
+	CheckConstraintsCapability             // supported in MySQL 8.0.16 and above: https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-16.html
 )
 
 const (

go/mysql/flavor_mysql.go

@@ -397,6 +397,8 @@ func (mysqlFlavor80) supportsCapability(serverVersion string, capability FlavorC
 		return ServerVersionAtLeast(serverVersion, 8, 0, 30)
 	case DisableRedoLogFlavorCapability:
 		return ServerVersionAtLeast(serverVersion, 8, 0, 21)
+	case CheckConstraintsCapability:
+		return ServerVersionAtLeast(serverVersion, 8, 0, 16)
 	default:
 		return false, nil
 	}

go/mysql/flavor_test.go

@@ -160,6 +160,16 @@ func TestGetFlavor(t *testing.T) {
 			capability: DisableRedoLogFlavorCapability,
 			isCapable:  false,
 		},
+		{
+			version:    "8.0.15",
+			capability: CheckConstraintsCapability,
+			isCapable:  false,
+		},
+		{
+			version:    "8.0.20",
+			capability: CheckConstraintsCapability,
+			isCapable:  true,
+		},
 	}
 	for _, tc := range testcases {
 		name := fmt.Sprintf("%s %v", tc.version, tc.capability)

go/test/endtoend/onlineddl/revert/onlineddl_revert_test.go

@@ -428,7 +428,20 @@ func testRevertible(t *testing.T) {
 					droppedNoDefaultColumnNames := row.AsString("dropped_no_default_column_names", "")
 					expandedColumnNames := row.AsString("expanded_column_names", "")
 
-					assert.Equal(t, testcase.removedForeignKeyNames, removeBackticks(removedForeignKeyNames))
+					// Online DDL renames constraint names, and keeps the original name as a prefix.
+					// The name of e.g. "some_fk_2_" might turn into "some_fk_2_518ubnm034rel35l1m0u1dc7m"
+					expectRemovedForeignKeyNames := strings.Split(testcase.removedForeignKeyNames, ",")
+					actualRemovedForeignKeyNames := strings.Split(removeBackticks(removedForeignKeyNames), ",")
+					assert.Equal(t, len(expectRemovedForeignKeyNames), len(actualRemovedForeignKeyNames))
+					for _, actualRemovedForeignKeyName := range actualRemovedForeignKeyNames {
+						found := false
+						for _, expectRemovedForeignKeyName := range expectRemovedForeignKeyNames {
+							if strings.HasPrefix(actualRemovedForeignKeyName, expectRemovedForeignKeyName) {
+								found = true
+							}
+						}
+						assert.Truef(t, found, "unexpected FK name", "%s", actualRemovedForeignKeyName)
+					}
 					assert.Equal(t, testcase.removedUniqueKeyNames, removeBackticks(removedUniqueKeyNames))
 					assert.Equal(t, testcase.droppedNoDefaultColumnNames, removeBackticks(droppedNoDefaultColumnNames))
 					assert.Equal(t, testcase.expandedColumnNames, removeBackticks(expandedColumnNames))
@@ -466,7 +479,8 @@ func testRevertible(t *testing.T) {
 				droppedNoDefaultColumnNames := row.AsString("dropped_no_default_column_names", "")
 				expandedColumnNames := row.AsString("expanded_column_names", "")
 
-				assert.Equal(t, "some_fk_2", removeBackticks(removedForeignKeyNames))
+				// Online DDL renames constraint names, and keeps the original name as a prefix. The name will be e.g. some_fk_2_518ubnm034rel35l1m0u1dc7m
+				assert.Contains(t, removeBackticks(removedForeignKeyNames), "some_fk_2")
 				assert.Equal(t, "", removeBackticks(removedUniqueKeyNames))
 				assert.Equal(t, "", removeBackticks(droppedNoDefaultColumnNames))
 				assert.Equal(t, "", removeBackticks(expandedColumnNames))

go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go

@@ -941,6 +941,25 @@ func testScheduler(t *testing.T) {
 		})
 	})
 
+	checkConstraintCapable, err := capableOf(mysql.CheckConstraintsCapability) // 8.0.16 and above
+	require.NoError(t, err)
+	if checkConstraintCapable {
+		// Constraints
+		t.Run("CREATE TABLE with CHECK constraint", func(t *testing.T) {
+			query := `create table with_constraint (id int primary key, check ((id >= 0)))`
+			uuid := testOnlineDDLStatement(t, createParams(query, ddlStrategy, "vtgate", "chk_", "", false))
+			onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
+			t.Run("ensure constraint name is rewritten", func(t *testing.T) {
+				// Since we did not provide a name for the CHECK constraint, MySQL will
+				// name it `with_constraint_chk_1`. But we expect Online DDL to explicitly
+				// modify the constraint name, specifically to get rid of the <table-name> prefix,
+				// so that we don't get into https://bugs.mysql.com/bug.php?id=107772 situation.
+				createStatement := getCreateTableStatement(t, shards[0].Vttablets[0], "with_constraint")
+				assert.NotContains(t, createStatement, "with_constraint_chk")
+			})
+		})
+	}
+
 	// INSTANT DDL
 	instantDDLCapable, err := capableOf(mysql.InstantAddLastColumnFlavorCapability)
 	require.NoError(t, err)
@@ -2328,7 +2347,7 @@ func testRevertMigration(t *testing.T, params *testRevertMigrationParams) (uuid
 	return uuid
 }
 
-// checkTable checks the number of tables in the first two shards.
+// checkTable checks the number of tables in all shards
 func checkTable(t *testing.T, showTableName string, expectExists bool) bool {
 	expectCount := 0
 	if expectExists {

go/vt/vttablet/onlineddl/executor.go

@@ -2863,6 +2863,17 @@ func (e *Executor) executeCreateDDLActionMigration(ctx context.Context, onlineDD
 			}
 		}
 	}
+	if originalCreateTable, ok := ddlStmt.(*sqlparser.CreateTable); ok {
+		newCreateTable := sqlparser.CloneRefOfCreateTable(originalCreateTable)
+		// Rewrite this CREATE TABLE statement such that CONSTRAINT names are edited,
+		// specifically removing any <tablename> prefix.
+		if _, err := e.validateAndEditCreateTableStatement(ctx, onlineDDL, newCreateTable); err != nil {
+			return failMigration(err)
+		}
+		ddlStmt = newCreateTable
+		onlineDDL.SQL = sqlparser.String(newCreateTable)
+	}
+
 	// from now on, whether a VIEW or a TABLE, they get the same treatment
 
 	sentryArtifactTableName, err := schema.GenerateGCTableName(schema.HoldTableGCState, newGCTableRetainTime())