diff --git a/go/vt/schemadiff/onlineddl.go b/go/vt/schemadiff/onlineddl.go index 54bb9810e20..f02ccb1224d 100644 --- a/go/vt/schemadiff/onlineddl.go +++ b/go/vt/schemadiff/onlineddl.go @@ -737,3 +737,42 @@ func AddInstantAlgorithm(alterTable *sqlparser.AlterTable) { // append an algorithm alterTable.AlterOptions = append(alterTable.AlterOptions, instantOpt) } + +// DuplicateCreateTable parses the given `CREATE TABLE` statement, and returns: +// - The format CreateTable AST +// - A new CreateTable AST, with the table renamed as `newTableName`, and with constraints renamed deterministically +// - Map of renamed constraints +func DuplicateCreateTable(originalCreateTable *sqlparser.CreateTable, baseUUID string, newTableName string, allowForeignKeys bool) ( + newCreateTable *sqlparser.CreateTable, + constraintMap map[string]string, + err error, +) { + 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) + + // manipulate CreateTable statement: take care of constraints names which have to be + // unique across the schema + constraintMap, err = ValidateAndEditCreateTableStatement(originalCreateTable.Table.Name.String(), baseUUID, newCreateTable, allowForeignKeys) + if err != nil { + return nil, nil, err + } + return newCreateTable, constraintMap, nil +} diff --git a/go/vt/schemadiff/onlineddl_test.go b/go/vt/schemadiff/onlineddl_test.go index ce93a86f4f7..7cbcfc4aef4 100644 --- a/go/vt/schemadiff/onlineddl_test.go +++ b/go/vt/schemadiff/onlineddl_test.go @@ -1265,3 +1265,63 @@ func TestAddInstantAlgorithm(t *testing.T) { }) } } + +func TestDuplicateCreateTable(t *testing.T) { + baseUUID := "a5a563da_dc1a_11ec_a416_0a43f95f28a3" + allowForeignKeys := true + + 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_ewl7lthyax2xxocpib3hyyvxx 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_6tlv90d9gcf4h6roalfnkdhar 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_95jpox7sx4w0cv7dpspzmjbxu foreign key (i1) references mytable (id), + constraint f1_1fg002b1cuqoavgti5zp8pu91 foreign key (i2) references parent (id) +)`, + expectMapSize: 1, + }, + } + env := NewTestEnv() + for _, tcase := range tcases { + t.Run(tcase.sql, func(t *testing.T) { + stmt, err := env.Parser().ParseStrictDDL(tcase.sql) + require.NoError(t, err) + originalCreateTable, ok := stmt.(*sqlparser.CreateTable) + require.True(t, ok) + require.NotNil(t, originalCreateTable) + newCreateTable, constraintMap, err := DuplicateCreateTable(originalCreateTable, baseUUID, tcase.newName, allowForeignKeys) + assert.NoError(t, err) + 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)) + }) + } +} diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index 416feabada8..aad8417237e 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/go/vt/vttablet/onlineddl/executor.go @@ -1278,45 +1278,6 @@ func (e *Executor) initConnectionLockWaitTimeout(ctx context.Context, conn *conn return deferFunc, nil } -// duplicateCreateTable parses the given `CREATE TABLE` statement, and returns: -// - The format CreateTable AST -// - A new CreateTable AST, with the table renamed as `newTableName`, and with constraints renamed deterministically -// - Map of renamed constraints -func (e *Executor) duplicateCreateTable(ctx context.Context, onlineDDL *schema.OnlineDDL, originalCreateTable *sqlparser.CreateTable, newTableName string) ( - newCreateTable *sqlparser.CreateTable, - constraintMap map[string]string, - err error, -) { - 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) - - // manipulate CreateTable statement: take care of constraints names which have to be - // unique across the schema - constraintMap, err = schemadiff.ValidateAndEditCreateTableStatement(onlineDDL.Table, onlineDDL.UUID, newCreateTable, onlineDDL.StrategySetting().IsAllowForeignKeysFlag()) - if err != nil { - return nil, nil, err - } - return newCreateTable, constraintMap, nil -} - // createDuplicateTableLike creates the table named by `newTableName` in the likeness of onlineDDL.Table // This function emulates MySQL's `CREATE TABLE LIKE ...` statement. The difference is that this function takes control over the generated CONSTRAINT names, // if any, such that they are deterministic across shards, as well as preserve original names where possible. @@ -1329,7 +1290,7 @@ func (e *Executor) createDuplicateTableLike(ctx context.Context, newTableName st if err != nil { return nil, nil, err } - vreplCreateTable, constraintMap, err := e.duplicateCreateTable(ctx, onlineDDL, originalCreateTable, newTableName) + vreplCreateTable, constraintMap, err := schemadiff.DuplicateCreateTable(originalCreateTable, onlineDDL.UUID, newTableName, onlineDDL.StrategySetting().IsAllowForeignKeysFlag()) if err != nil { return nil, nil, err } diff --git a/go/vt/vttablet/onlineddl/executor_test.go b/go/vt/vttablet/onlineddl/executor_test.go index a789ff63cd3..2533f3a4b48 100644 --- a/go/vt/vttablet/onlineddl/executor_test.go +++ b/go/vt/vttablet/onlineddl/executor_test.go @@ -21,82 +21,12 @@ Functionality of this Executor is tested in go/test/endtoend/onlineddl/... package onlineddl import ( - "context" "testing" "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "vitess.io/vitess/go/vt/vtenv" - "vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv" - - "vitess.io/vitess/go/vt/schema" - "vitess.io/vitess/go/vt/sqlparser" ) -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) { - stmt, err := e.env.Environment().Parser().ParseStrictDDL(tcase.sql) - require.NoError(t, err) - originalCreateTable, ok := stmt.(*sqlparser.CreateTable) - require.True(t, ok) - require.NotNil(t, originalCreateTable) - newCreateTable, constraintMap, err := e.duplicateCreateTable(ctx, onlineDDL, originalCreateTable, tcase.newName) - assert.NoError(t, err) - 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