From 3fd8913f843613d58dc8a470363dae6517e7627e Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 7 Mar 2024 08:50:25 +0000 Subject: [PATCH 1/4] Replicate `onDelete` attribute of a FK --- pkg/migrations/duplicate.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/migrations/duplicate.go b/pkg/migrations/duplicate.go index a87a9c22..0173b026 100644 --- a/pkg/migrations/duplicate.go +++ b/pkg/migrations/duplicate.go @@ -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)` ) @@ -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, + ) } } From 11ccd6d040be5967931b99d0541af57bde614487 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 7 Mar 2024 09:29:30 +0000 Subject: [PATCH 2/4] Update assertion functions --- pkg/migrations/op_common_test.go | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/pkg/migrations/op_common_test.go b/pkg/migrations/op_common_test.go index 1862b5d2..993b1b5c 100644 --- a/pkg/migrations/op_common_test.go +++ b/pkg/migrations/op_common_test.go @@ -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) } } @@ -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 ( @@ -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) } From 4dd0c3d9437fb1a06f44af88ea9e53e35ddf2ecf Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 7 Mar 2024 09:48:03 +0000 Subject: [PATCH 3/4] Update usages of assertion functions --- pkg/migrations/op_add_column_test.go | 4 ++-- pkg/migrations/op_create_table_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/migrations/op_add_column_test.go b/pkg/migrations/op_add_column_test.go index d9e8cce5..1a152773 100644 --- a/pkg/migrations/op_add_column_test.go +++ b/pkg/migrations/op_add_column_test.go @@ -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{ @@ -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{ diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index f7886559..597cf99b 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -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{ @@ -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{ From 15e9fe824eb93c2540c7c74ff2ae757e2823b572 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 7 Mar 2024 10:13:30 +0000 Subject: [PATCH 4/4] Update existing testcases Ensure that the `ON DELETE` property of foreign keys is preserved when an FK column is duplicated for backfilling. --- pkg/migrations/op_change_type_test.go | 11 ++++++----- pkg/migrations/op_set_check_test.go | 11 ++++++----- pkg/migrations/op_set_fk_test.go | 11 ++++++----- pkg/migrations/op_set_notnull_test.go | 11 ++++++----- pkg/migrations/op_set_unique_test.go | 11 ++++++----- 5 files changed, 30 insertions(+), 25 deletions(-) diff --git a/pkg/migrations/op_change_type_test.go b/pkg/migrations/op_change_type_test.go index ab9b0f0f..ff7871f5 100644 --- a/pkg/migrations/op_change_type_test.go +++ b/pkg/migrations/op_change_type_test.go @@ -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, }, }, }, @@ -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()) }, }, { diff --git a/pkg/migrations/op_set_check_test.go b/pkg/migrations/op_set_check_test.go index 093b5e76..6c03434f 100644 --- a/pkg/migrations/op_set_check_test.go +++ b/pkg/migrations/op_set_check_test.go @@ -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, }, }, }, @@ -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()) }, }, { diff --git a/pkg/migrations/op_set_fk_test.go b/pkg/migrations/op_set_fk_test.go index 37b3d314..f8834c73 100644 --- a/pkg/migrations/op_set_fk_test.go +++ b/pkg/migrations/op_set_fk_test.go @@ -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"), @@ -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()) }, }, { diff --git a/pkg/migrations/op_set_notnull_test.go b/pkg/migrations/op_set_notnull_test.go index 7ff4eae8..e1902162 100644 --- a/pkg/migrations/op_set_notnull_test.go +++ b/pkg/migrations/op_set_notnull_test.go @@ -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, }, }, }, @@ -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()) }, }, { diff --git a/pkg/migrations/op_set_unique_test.go b/pkg/migrations/op_set_unique_test.go index b8ccadf0..b18d294b 100644 --- a/pkg/migrations/op_set_unique_test.go +++ b/pkg/migrations/op_set_unique_test.go @@ -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, }, }, }, @@ -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()) }, }, {