diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index 66e81aef949..85642413302 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/go/vt/vttablet/onlineddl/executor.go @@ -876,7 +876,6 @@ func (e *Executor) cutOverVReplMigration(ctx context.Context, s *VReplStream) er } }() renameQuery := sqlparser.BuildParsedQuery(sqlSwapTables, onlineDDL.Table, sentryTableName, vreplTable, onlineDDL.Table, sentryTableName, vreplTable) - waitForRenameProcess := func() error { // This function waits until it finds the RENAME TABLE... query running in MySQL's PROCESSLIST, or until timeout // The function assumes that one of the renamed tables is locked, thus causing the RENAME to block. If nothing @@ -1262,7 +1261,31 @@ func (e *Executor) createTableLike(ctx context.Context, newTableName string, onl if !ok { return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "expected CreateTable statement, got: %v", sqlparser.CanonicalString(stmt)) } +<<<<<<< HEAD createTable.SetTable(createTable.GetTable().Qualifier.CompliantName(), newTableName) +======= + newCreateTable = sqlparser.Clone(originalCreateTable) + newCreateTable.SetTable(newCreateTable.GetTable().Qualifier.CompliantName(), newTableName) + + // If this table has a self-referencing foreign key constraint, ensure the referenced table gets renamed: + renameSelfFK := func(node sqlparser.SQLNode) (kontinue bool, err error) { + switch node := node.(type) { + case *sqlparser.ConstraintDefinition: + fk, ok := node.Details.(*sqlparser.ForeignKeyDefinition) + if !ok { + return true, nil + } + if referencedTableName := fk.ReferenceDefinition.ReferencedTable.Name.String(); referencedTableName == originalCreateTable.Table.Name.String() { + // This is a self-referencing foreign key + // We need to rename the referenced table as well + fk.ReferenceDefinition.ReferencedTable.Name = sqlparser.NewIdentifierCS(newTableName) + } + } + return true, nil + } + _ = sqlparser.Walk(renameSelfFK, newCreateTable) + +>>>>>>> 6f850892b1 (Online DDL shadow table: rename referenced table name in self referencing FK (#16205)) // manipulate CreateTable statement: take care of constraints names which have to be // unique across the schema constraintMap, err = e.validateAndEditCreateTableStatement(ctx, onlineDDL, createTable) diff --git a/go/vt/vttablet/onlineddl/executor_test.go b/go/vt/vttablet/onlineddl/executor_test.go index 08f363fc892..d29307236bf 100644 --- a/go/vt/vttablet/onlineddl/executor_test.go +++ b/go/vt/vttablet/onlineddl/executor_test.go @@ -271,3 +271,170 @@ func TestAddInstantAlgorithm(t *testing.T) { }) } } +<<<<<<< HEAD +======= + +func TestDuplicateCreateTable(t *testing.T) { + e := Executor{ + env: tabletenv.NewEnv(vtenv.NewTestEnv(), nil, "DuplicateCreateTableTest"), + } + ctx := context.Background() + onlineDDL := &schema.OnlineDDL{UUID: "a5a563da_dc1a_11ec_a416_0a43f95f28a3", Table: "something", Strategy: "vitess", Options: "--unsafe-allow-foreign-keys"} + + tcases := []struct { + sql string + newName string + expectSQL string + expectMapSize int + }{ + { + sql: "create table t (id int primary key)", + newName: "mytable", + expectSQL: "create table mytable (\n\tid int primary key\n)", + }, + { + sql: "create table t (id int primary key, i int, constraint f foreign key (i) references parent (id) on delete cascade)", + newName: "mytable", + expectSQL: "create table mytable (\n\tid int primary key,\n\ti int,\n\tconstraint f_bjj16562shq086ozik3zf6kjg foreign key (i) references parent (id) on delete cascade\n)", + expectMapSize: 1, + }, + { + sql: "create table self (id int primary key, i int, constraint f foreign key (i) references self (id))", + newName: "mytable", + expectSQL: "create table mytable (\n\tid int primary key,\n\ti int,\n\tconstraint f_8aymb58nzb78l5jhq600veg6y foreign key (i) references mytable (id)\n)", + expectMapSize: 1, + }, + { + sql: "create table self (id int primary key, i1 int, i2 int, constraint f1 foreign key (i1) references self (id), constraint f1 foreign key (i2) references parent (id))", + newName: "mytable", + expectSQL: `create table mytable ( + id int primary key, + i1 int, + i2 int, + constraint f1_1rlsg9yls1t91i35zq5gyeoq7 foreign key (i1) references mytable (id), + constraint f1_59t4lvb1ncti6fxy27drad4jp foreign key (i2) references parent (id) +)`, + expectMapSize: 1, + }, + } + for _, tcase := range tcases { + t.Run(tcase.sql, func(t *testing.T) { + originalCreateTable, newCreateTable, constraintMap, err := e.duplicateCreateTable(ctx, onlineDDL, tcase.sql, tcase.newName) + assert.NoError(t, err) + assert.NotNil(t, originalCreateTable) + assert.NotNil(t, newCreateTable) + assert.NotNil(t, constraintMap) + + newSQL := sqlparser.String(newCreateTable) + assert.Equal(t, tcase.expectSQL, newSQL) + assert.Equal(t, tcase.expectMapSize, len(constraintMap)) + }) + } +} + +func TestShouldCutOverAccordingToBackoff(t *testing.T) { + tcases := []struct { + name string + + shouldForceCutOverIndicator bool + forceCutOverAfter time.Duration + sinceReadyToComplete time.Duration + sinceLastCutoverAttempt time.Duration + cutoverAttempts int64 + + expectShouldCutOver bool + expectShouldForceCutOver bool + }{ + { + name: "no reason why not, normal cutover", + expectShouldCutOver: true, + }, + { + name: "backoff", + cutoverAttempts: 1, + expectShouldCutOver: false, + }, + { + name: "more backoff", + cutoverAttempts: 3, + expectShouldCutOver: false, + }, + { + name: "more backoff, since last cutover", + cutoverAttempts: 3, + sinceLastCutoverAttempt: time.Second, + expectShouldCutOver: false, + }, + { + name: "no backoff, long since last cutover", + cutoverAttempts: 3, + sinceLastCutoverAttempt: time.Hour, + expectShouldCutOver: true, + }, + { + name: "many attempts, long since last cutover", + cutoverAttempts: 3000, + sinceLastCutoverAttempt: time.Hour, + expectShouldCutOver: true, + }, + { + name: "force cutover", + shouldForceCutOverIndicator: true, + expectShouldCutOver: true, + expectShouldForceCutOver: true, + }, + { + name: "force cutover overrides backoff", + cutoverAttempts: 3, + shouldForceCutOverIndicator: true, + expectShouldCutOver: true, + expectShouldForceCutOver: true, + }, + { + name: "backoff; cutover-after not in effect yet", + cutoverAttempts: 3, + forceCutOverAfter: time.Second, + expectShouldCutOver: false, + expectShouldForceCutOver: false, + }, + { + name: "backoff; cutover-after still not in effect yet", + cutoverAttempts: 3, + forceCutOverAfter: time.Second, + sinceReadyToComplete: time.Millisecond, + expectShouldCutOver: false, + expectShouldForceCutOver: false, + }, + { + name: "cutover-after overrides backoff", + cutoverAttempts: 3, + forceCutOverAfter: time.Second, + sinceReadyToComplete: time.Second * 2, + expectShouldCutOver: true, + expectShouldForceCutOver: true, + }, + { + name: "cutover-after overrides backoff, realistic value", + cutoverAttempts: 300, + sinceLastCutoverAttempt: time.Minute, + forceCutOverAfter: time.Hour, + sinceReadyToComplete: time.Hour * 2, + expectShouldCutOver: true, + expectShouldForceCutOver: true, + }, + } + for _, tcase := range tcases { + t.Run(tcase.name, func(t *testing.T) { + shouldCutOver, shouldForceCutOver := shouldCutOverAccordingToBackoff( + tcase.shouldForceCutOverIndicator, + tcase.forceCutOverAfter, + tcase.sinceReadyToComplete, + tcase.sinceLastCutoverAttempt, + tcase.cutoverAttempts, + ) + assert.Equal(t, tcase.expectShouldCutOver, shouldCutOver) + assert.Equal(t, tcase.expectShouldForceCutOver, shouldForceCutOver) + }) + } +} +>>>>>>> 6f850892b1 (Online DDL shadow table: rename referenced table name in self referencing FK (#16205))