Skip to content

Commit

Permalink
Preserve foreign key ON DELETE attributes when FK columns are duplica…
Browse files Browse the repository at this point in the history
…ted (#314)

Ensure that a foreign key's `ON DELETE` attribute is preserved when a
foreign key column is duplicated by a migration.

Fixes #313
  • Loading branch information
andrew-farries committed Mar 7, 2024
1 parent 297dd38 commit 547ac3e
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 37 deletions.
6 changes: 4 additions & 2 deletions pkg/migrations/duplicate.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (d *Duplicator) Duplicate(ctx context.Context) error {
const (
cAlterTableSQL = `ALTER TABLE %s ADD COLUMN %s %s`
cSetDefaultSQL = `ALTER COLUMN %s SET DEFAULT %s`
cAddForeignKeySQL = `ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s)`
cAddForeignKeySQL = `ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s) ON DELETE %s`
cAddCheckConstraintSQL = `ADD CONSTRAINT %s %s NOT VALID`
cCreateUniqueIndexSQL = `CREATE UNIQUE INDEX CONCURRENTLY %s ON %s (%s)`
)
Expand Down Expand Up @@ -91,7 +91,9 @@ func (d *Duplicator) Duplicate(ctx context.Context) error {
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), ", "))
strings.Join(quoteColumnNames(fk.ReferencedColumns), ", "),
fk.OnDelete,
)
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/migrations/op_add_column_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ func TestAddForeignKeyColumn(t *testing.T) {
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// The foreign key constraint exists on the new table.
ValidatedForeignKeyMustExist(t, db, schema, "orders", "fk_users_id")
ValidatedForeignKeyMustExist(t, db, schema, "orders", "fk_users_id", withOnDeleteCascade())

// Inserting a row into the referenced table succeeds.
MustInsert(t, db, schema, "01_create_table", "users", map[string]string{
Expand Down Expand Up @@ -563,7 +563,7 @@ func TestAddForeignKeyColumn(t *testing.T) {
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// The foreign key constraint still exists on the new table
ValidatedForeignKeyMustExist(t, db, schema, "orders", "fk_users_id")
ValidatedForeignKeyMustExist(t, db, schema, "orders", "fk_users_id", withOnDeleteCascade())

// Inserting a row into the referenced table succeeds.
MustInsert(t, db, schema, "02_add_column", "users", map[string]string{
Expand Down
11 changes: 6 additions & 5 deletions pkg/migrations/op_change_type_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,10 @@ func TestChangeColumnType(t *testing.T) {
Name: "department_id",
Type: "integer",
References: &migrations.ForeignKeyReference{
Name: "fk_employee_department",
Table: "departments",
Column: "id",
Name: "fk_employee_department",
Table: "departments",
Column: "id",
OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE,
},
},
},
Expand All @@ -214,13 +215,13 @@ func TestChangeColumnType(t *testing.T) {
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// A temporary FK constraint has been created on the temporary column
ValidatedForeignKeyMustExist(t, db, schema, "employees", migrations.DuplicationName("fk_employee_department"))
ValidatedForeignKeyMustExist(t, db, schema, "employees", migrations.DuplicationName("fk_employee_department"), withOnDeleteCascade())
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// The foreign key constraint still exists on the column
ValidatedForeignKeyMustExist(t, db, schema, "employees", "fk_employee_department")
ValidatedForeignKeyMustExist(t, db, schema, "employees", "fk_employee_department", withOnDeleteCascade())
},
},
{
Expand Down
28 changes: 22 additions & 6 deletions pkg/migrations/op_common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,16 +204,16 @@ func UniqueConstraintMustExist(t *testing.T, db *sql.DB, schema, table, constrai
}
}

func ValidatedForeignKeyMustExist(t *testing.T, db *sql.DB, schema, table, constraint string) {
func ValidatedForeignKeyMustExist(t *testing.T, db *sql.DB, schema, table, constraint string, opts ...foreignKeyOnDeleteOpt) {
t.Helper()
if !foreignKeyExists(t, db, schema, table, constraint, true) {
if !foreignKeyExists(t, db, schema, table, constraint, true, opts...) {
t.Fatalf("Expected validated foreign key %q to exist", constraint)
}
}

func NotValidatedForeignKeyMustExist(t *testing.T, db *sql.DB, schema, table, constraint string) {
func NotValidatedForeignKeyMustExist(t *testing.T, db *sql.DB, schema, table, constraint string, opts ...foreignKeyOnDeleteOpt) {
t.Helper()
if !foreignKeyExists(t, db, schema, table, constraint, false) {
if !foreignKeyExists(t, db, schema, table, constraint, false, opts...) {
t.Fatalf("Expected not validated foreign key %q to exist", constraint)
}
}
Expand Down Expand Up @@ -313,9 +313,24 @@ func uniqueConstraintExists(t *testing.T, db *sql.DB, schema, table, constraint
return exists
}

func foreignKeyExists(t *testing.T, db *sql.DB, schema, table, constraint string, validated bool) bool {
type foreignKeyOnDeleteOpt func() string

func withOnDeleteCascade() foreignKeyOnDeleteOpt {
return func() string { return "c" }
}

func withOnDeleteSetNull() foreignKeyOnDeleteOpt {
return func() string { return "n" }
}

func foreignKeyExists(t *testing.T, db *sql.DB, schema, table, constraint string, validated bool, opts ...foreignKeyOnDeleteOpt) bool {
t.Helper()

confDelType := "a"
for _, opt := range opts {
confDelType = opt()
}

var exists bool
err := db.QueryRow(`
SELECT EXISTS (
Expand All @@ -325,8 +340,9 @@ func foreignKeyExists(t *testing.T, db *sql.DB, schema, table, constraint string
AND conname = $2
AND contype = 'f'
AND convalidated = $3
AND confdeltype = $4
)`,
fmt.Sprintf("%s.%s", schema, table), constraint, validated).Scan(&exists)
fmt.Sprintf("%s.%s", schema, table), constraint, validated, confDelType).Scan(&exists)
if err != nil {
t.Fatal(err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/migrations/op_create_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func TestCreateTable(t *testing.T) {
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// The foreign key constraint exists on the new table.
ValidatedForeignKeyMustExist(t, db, schema, migrations.TemporaryName("orders"), "fk_users_id")
ValidatedForeignKeyMustExist(t, db, schema, migrations.TemporaryName("orders"), "fk_users_id", withOnDeleteCascade())

// Inserting a row into the referenced table succeeds.
MustInsert(t, db, schema, "01_create_table", "users", map[string]string{
Expand Down Expand Up @@ -269,7 +269,7 @@ func TestCreateTable(t *testing.T) {
// The table has been dropped, so the foreign key constraint is gone.
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
ValidatedForeignKeyMustExist(t, db, schema, "orders", "fk_users_id")
ValidatedForeignKeyMustExist(t, db, schema, "orders", "fk_users_id", withOnDeleteCascade())

// Inserting a row into the referenced table succeeds.
MustInsert(t, db, schema, "02_create_table_with_fk", "users", map[string]string{
Expand Down
11 changes: 6 additions & 5 deletions pkg/migrations/op_set_check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,10 @@ func TestSetCheckConstraint(t *testing.T) {
Type: "integer",
Nullable: ptr(true),
References: &migrations.ForeignKeyReference{
Name: "fk_employee_department",
Table: "departments",
Column: "id",
Name: "fk_employee_department",
Table: "departments",
Column: "id",
OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE,
},
},
},
Expand All @@ -285,13 +286,13 @@ func TestSetCheckConstraint(t *testing.T) {
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// A temporary FK constraint has been created on the temporary column
ValidatedForeignKeyMustExist(t, db, schema, "employees", migrations.DuplicationName("fk_employee_department"))
ValidatedForeignKeyMustExist(t, db, schema, "employees", migrations.DuplicationName("fk_employee_department"), withOnDeleteCascade())
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// The foreign key constraint still exists on the column
ValidatedForeignKeyMustExist(t, db, schema, "employees", "fk_employee_department")
ValidatedForeignKeyMustExist(t, db, schema, "employees", "fk_employee_department", withOnDeleteCascade())
},
},
{
Expand Down
11 changes: 6 additions & 5 deletions pkg/migrations/op_set_fk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -738,9 +738,10 @@ func TestSetForeignKey(t *testing.T) {
Table: "posts",
Column: "user_id",
References: &migrations.ForeignKeyReference{
Name: "fk_users_id_1",
Table: "users",
Column: "id",
Name: "fk_users_id_1",
Table: "users",
Column: "id",
OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE,
},
Up: ptr("(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)"),
Down: ptr("user_id"),
Expand All @@ -766,13 +767,13 @@ func TestSetForeignKey(t *testing.T) {
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// A temporary FK constraint has been created on the temporary column
ValidatedForeignKeyMustExist(t, db, schema, "posts", migrations.DuplicationName("fk_users_id_1"))
ValidatedForeignKeyMustExist(t, db, schema, "posts", migrations.DuplicationName("fk_users_id_1"), withOnDeleteCascade())
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// The foreign key constraint still exists on the column
ValidatedForeignKeyMustExist(t, db, schema, "posts", "fk_users_id_1")
ValidatedForeignKeyMustExist(t, db, schema, "posts", "fk_users_id_1", withOnDeleteCascade())
},
},
{
Expand Down
11 changes: 6 additions & 5 deletions pkg/migrations/op_set_notnull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,10 @@ func TestSetNotNull(t *testing.T) {
Type: "integer",
Nullable: ptr(true),
References: &migrations.ForeignKeyReference{
Name: "fk_employee_department",
Table: "departments",
Column: "id",
Name: "fk_employee_department",
Table: "departments",
Column: "id",
OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE,
},
},
},
Expand All @@ -291,13 +292,13 @@ func TestSetNotNull(t *testing.T) {
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// A temporary FK constraint has been created on the temporary column
ValidatedForeignKeyMustExist(t, db, schema, "employees", migrations.DuplicationName("fk_employee_department"))
ValidatedForeignKeyMustExist(t, db, schema, "employees", migrations.DuplicationName("fk_employee_department"), withOnDeleteCascade())
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// The foreign key constraint still exists on the column
ValidatedForeignKeyMustExist(t, db, schema, "employees", "fk_employee_department")
ValidatedForeignKeyMustExist(t, db, schema, "employees", "fk_employee_department", withOnDeleteCascade())
},
},
{
Expand Down
11 changes: 6 additions & 5 deletions pkg/migrations/op_set_unique_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,10 @@ func TestSetColumnUnique(t *testing.T) {
Type: "integer",
Nullable: ptr(true),
References: &migrations.ForeignKeyReference{
Name: "fk_employee_department",
Table: "departments",
Column: "id",
Name: "fk_employee_department",
Table: "departments",
Column: "id",
OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL,
},
},
},
Expand All @@ -330,13 +331,13 @@ func TestSetColumnUnique(t *testing.T) {
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// A temporary FK constraint has been created on the temporary column
ValidatedForeignKeyMustExist(t, db, schema, "employees", migrations.DuplicationName("fk_employee_department"))
ValidatedForeignKeyMustExist(t, db, schema, "employees", migrations.DuplicationName("fk_employee_department"), withOnDeleteSetNull())
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// The foreign key constraint still exists on the column
ValidatedForeignKeyMustExist(t, db, schema, "employees", "fk_employee_department")
ValidatedForeignKeyMustExist(t, db, schema, "employees", "fk_employee_department", withOnDeleteSetNull())
},
},
{
Expand Down

0 comments on commit 547ac3e

Please sign in to comment.