From 7a38e1f15934949f7fbe0e2b0aeb8d62efdcb44b Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Fri, 19 Jan 2024 15:08:29 +0000 Subject: [PATCH] Identify duplicated foreign key constraints by prefix (#245) 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 constraints in this way allows the rename process to ensure that it only attempts to rename duplicated FK constraints. --- pkg/migrations/duplicate.go | 14 +++++++++++++- pkg/migrations/op_change_type_test.go | 2 +- pkg/migrations/op_set_check_test.go | 2 +- pkg/migrations/op_set_fk.go | 4 ++-- pkg/migrations/op_set_fk_test.go | 4 ++-- pkg/migrations/op_set_notnull_test.go | 2 +- pkg/migrations/op_set_unique_test.go | 2 +- pkg/migrations/rename.go | 6 +++++- 8 files changed, 26 insertions(+), 10 deletions(-) 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) 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.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), 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) { }, 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)