refactor DuplicateCreateTable into schemadiff

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

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
+}

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))
+ })
+ }
+}

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
  }

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