From c088b9b2ec02e81ed661b0737798611fc86e363d Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 18 Jan 2024 15:29:12 +0000 Subject: [PATCH 1/4] Create duplicated FKs with a special prefix When duplicating a column for backfilling, ensure that any FK constraints on the column are duplicated with a special prefix to identify them as duplicated constraints. Naming the duplicated FK constraint makes it easy for such constraints to be renamed on migration completion. --- pkg/migrations/duplicate.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/migrations/duplicate.go b/pkg/migrations/duplicate.go index 39c0af4f..627c397a 100644 --- a/pkg/migrations/duplicate.go +++ b/pkg/migrations/duplicate.go @@ -61,7 +61,7 @@ func (d *Duplicator) Duplicate(ctx context.Context) error { for _, fk := range d.table.ForeignKeys { if slices.Contains(fk.Columns, d.column.Name) { sql += fmt.Sprintf(", "+cAddForeignKeySQL, - pq.QuoteIdentifier(TemporaryName(fk.Name)), + pq.QuoteIdentifier(DuplicationName(fk.Name)), strings.Join(quoteColumnNames(copyAndReplace(fk.Columns, d.column.Name, d.asName)), ", "), pq.QuoteIdentifier(fk.ReferencedTable), strings.Join(quoteColumnNames(fk.ReferencedColumns), ", ")) @@ -73,6 +73,18 @@ func (d *Duplicator) Duplicate(ctx context.Context) error { return err } +func DuplicationName(name string) string { + return "_pgroll_dup_" + name +} + +func IsDuplicatedName(name string) bool { + return strings.HasPrefix(name, "_pgroll_dup_") +} + +func StripDuplicationPrefix(name string) string { + return strings.TrimPrefix(name, "_pgroll_dup_") +} + func copyAndReplace(xs []string, oldValue, newValue string) []string { ys := slices.Clone(xs) From d915966693c5fd6885c911fc3d262d201c20a0e8 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 18 Jan 2024 15:31:03 +0000 Subject: [PATCH 2/4] Ensure that duplicated FKs are renamed On migration completion, `RenameDuplicatedColumn` is called to rename a duplicated column along with any FK constraints that were created on the column. Ensure that the rename process only attempts to handle duplicated FK constraints by looking to the special name prefix. --- pkg/migrations/rename.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/migrations/rename.go b/pkg/migrations/rename.go index 200d96ca..7ac9c2be 100644 --- a/pkg/migrations/rename.go +++ b/pkg/migrations/rename.go @@ -35,11 +35,15 @@ func RenameDuplicatedColumn(ctx context.Context, conn *sql.DB, table *schema.Tab // to their original name var renameConstraintSQL string for _, fk := range table.ForeignKeys { + if !IsDuplicatedName(fk.Name) { + continue + } + if slices.Contains(fk.Columns, TemporaryName(column.Name)) { renameConstraintSQL = fmt.Sprintf(cRenameConstraintSQL, pq.QuoteIdentifier(table.Name), pq.QuoteIdentifier(fk.Name), - pq.QuoteIdentifier(StripTemporaryPrefix(fk.Name)), + pq.QuoteIdentifier(StripDuplicationPrefix(fk.Name)), ) _, err = conn.ExecContext(ctx, renameConstraintSQL) From 1c8503ac045728b38858a8f4f10d28fddc2cc185 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 18 Jan 2024 15:33:11 +0000 Subject: [PATCH 3/4] Stop giving FKs temporary names on creation There is no need to create FK constraints with a temporary name prefix - they only need to be validated on migration completion, not renamed. --- pkg/migrations/op_set_fk.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/migrations/op_set_fk.go b/pkg/migrations/op_set_fk.go index 53b76cb7..0308c9f8 100644 --- a/pkg/migrations/op_set_fk.go +++ b/pkg/migrations/op_set_fk.go @@ -85,7 +85,7 @@ func (o *OpSetForeignKey) Complete(ctx context.Context, conn *sql.DB, s *schema. // Validate the foreign key constraint _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s VALIDATE CONSTRAINT %s", pq.QuoteIdentifier(o.Table), - pq.QuoteIdentifier(TemporaryName(o.References.Name)))) + pq.QuoteIdentifier(o.References.Name))) if err != nil { return err } @@ -173,7 +173,7 @@ func (o *OpSetForeignKey) addForeignKeyConstraint(ctx context.Context, conn *sql _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s) NOT VALID", pq.QuoteIdentifier(o.Table), - pq.QuoteIdentifier(TemporaryName(o.References.Name)), + pq.QuoteIdentifier(o.References.Name), pq.QuoteIdentifier(tempColumnName), pq.QuoteIdentifier(o.References.Table), pq.QuoteIdentifier(o.References.Column), From 845ead316cd08603dc9c53815763a5bfa3cc6e11 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 18 Jan 2024 15:34:23 +0000 Subject: [PATCH 4/4] Update test expectations Update test expectations now that FK constraints on duplicated columns have a special prefix. --- pkg/migrations/op_change_type_test.go | 2 +- pkg/migrations/op_set_check_test.go | 2 +- pkg/migrations/op_set_fk_test.go | 4 ++-- pkg/migrations/op_set_notnull_test.go | 2 +- pkg/migrations/op_set_unique_test.go | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/migrations/op_change_type_test.go b/pkg/migrations/op_change_type_test.go index 3b51aa3f..4e6baf7c 100644 --- a/pkg/migrations/op_change_type_test.go +++ b/pkg/migrations/op_change_type_test.go @@ -213,7 +213,7 @@ func TestChangeColumnType(t *testing.T) { }, afterStart: func(t *testing.T, db *sql.DB) { // A temporary FK constraint has been created on the temporary column - ValidatedForeignKeyMustExist(t, db, "public", "employees", migrations.TemporaryName("fk_employee_department")) + ValidatedForeignKeyMustExist(t, db, "public", "employees", migrations.DuplicationName("fk_employee_department")) }, afterRollback: func(t *testing.T, db *sql.DB) { }, diff --git a/pkg/migrations/op_set_check_test.go b/pkg/migrations/op_set_check_test.go index 777e781d..7bf4b724 100644 --- a/pkg/migrations/op_set_check_test.go +++ b/pkg/migrations/op_set_check_test.go @@ -285,7 +285,7 @@ func TestSetCheckConstraint(t *testing.T) { }, afterStart: func(t *testing.T, db *sql.DB) { // A temporary FK constraint has been created on the temporary column - ValidatedForeignKeyMustExist(t, db, "public", "employees", migrations.TemporaryName("fk_employee_department")) + ValidatedForeignKeyMustExist(t, db, "public", "employees", migrations.DuplicationName("fk_employee_department")) }, afterRollback: func(t *testing.T, db *sql.DB) { }, diff --git a/pkg/migrations/op_set_fk_test.go b/pkg/migrations/op_set_fk_test.go index 73db8cf5..709a4daa 100644 --- a/pkg/migrations/op_set_fk_test.go +++ b/pkg/migrations/op_set_fk_test.go @@ -77,7 +77,7 @@ func TestSetForeignKey(t *testing.T) { ColumnMustExist(t, db, "public", "posts", migrations.TemporaryName("user_id")) // A temporary FK constraint has been created on the temporary column - NotValidatedForeignKeyMustExist(t, db, "public", "posts", migrations.TemporaryName("fk_users_id")) + NotValidatedForeignKeyMustExist(t, db, "public", "posts", "fk_users_id") // Inserting some data into the `users` table works. MustInsert(t, db, "public", "02_add_fk_constraint", "users", map[string]string{ @@ -348,7 +348,7 @@ func TestSetForeignKey(t *testing.T) { }, afterStart: func(t *testing.T, db *sql.DB) { // A temporary FK constraint has been created on the temporary column - ValidatedForeignKeyMustExist(t, db, "public", "posts", migrations.TemporaryName("fk_users_id_1")) + ValidatedForeignKeyMustExist(t, db, "public", "posts", migrations.DuplicationName("fk_users_id_1")) }, afterRollback: func(t *testing.T, db *sql.DB) { }, diff --git a/pkg/migrations/op_set_notnull_test.go b/pkg/migrations/op_set_notnull_test.go index 82ed0514..d619386c 100644 --- a/pkg/migrations/op_set_notnull_test.go +++ b/pkg/migrations/op_set_notnull_test.go @@ -291,7 +291,7 @@ func TestSetNotNull(t *testing.T) { }, afterStart: func(t *testing.T, db *sql.DB) { // A temporary FK constraint has been created on the temporary column - ValidatedForeignKeyMustExist(t, db, "public", "employees", migrations.TemporaryName("fk_employee_department")) + ValidatedForeignKeyMustExist(t, db, "public", "employees", migrations.DuplicationName("fk_employee_department")) }, afterRollback: func(t *testing.T, db *sql.DB) { }, diff --git a/pkg/migrations/op_set_unique_test.go b/pkg/migrations/op_set_unique_test.go index 459ed934..aca893db 100644 --- a/pkg/migrations/op_set_unique_test.go +++ b/pkg/migrations/op_set_unique_test.go @@ -330,7 +330,7 @@ func TestSetColumnUnique(t *testing.T) { }, afterStart: func(t *testing.T, db *sql.DB) { // A temporary FK constraint has been created on the temporary column - ValidatedForeignKeyMustExist(t, db, "public", "employees", migrations.TemporaryName("fk_employee_department")) + ValidatedForeignKeyMustExist(t, db, "public", "employees", migrations.DuplicationName("fk_employee_department")) }, afterRollback: func(t *testing.T, db *sql.DB) { },