diff --git a/docs/README.md b/docs/README.md index 89abb760..1f1e12d7 100644 --- a/docs/README.md +++ b/docs/README.md @@ -816,7 +816,8 @@ Add foreign key operations add a foreign key constraint to a column. "references": { "name": "name of foreign key reference", "table": "name of referenced table", - "column": "name of referenced column" + "column": "name of referenced column", + "on_delete": "ON DELETE behaviour, can be CASCADE, SET NULL, RESTRICT, or NO ACTION. Default is NO ACTION", }, "up": "SQL expression", "down": "SQL expression" diff --git a/examples/21_add_foreign_key_constraint.json b/examples/21_add_foreign_key_constraint.json index a3eadedf..a2f6f1d3 100644 --- a/examples/21_add_foreign_key_constraint.json +++ b/examples/21_add_foreign_key_constraint.json @@ -8,7 +8,8 @@ "references": { "name": "fk_users_id", "table": "users", - "column": "id" + "column": "id", + "on_delete": "CASCADE" }, "up": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", "down": "user_id" diff --git a/pkg/migrations/errors.go b/pkg/migrations/errors.go index 96d77095..e0c7b66f 100644 --- a/pkg/migrations/errors.go +++ b/pkg/migrations/errors.go @@ -173,3 +173,22 @@ type InvalidReplicaIdentityError struct { func (e InvalidReplicaIdentityError) Error() string { return fmt.Sprintf("replica identity on table %q must be one of 'NOTHING', 'DEFAULT', 'INDEX' or 'FULL', found %q", e.Table, e.Identity) } + +type InvalidOnDeleteSettingError struct { + Table string + Column string + Setting string +} + +func (e InvalidOnDeleteSettingError) Error() string { + return fmt.Sprintf("foreign key on_delete setting on column %q, table %q must be one of: %q, %q, %q, %q or %q, not %q", + e.Column, + e.Table, + ForeignKeyReferenceOnDeleteNOACTION, + ForeignKeyReferenceOnDeleteRESTRICT, + ForeignKeyReferenceOnDeleteSETDEFAULT, + ForeignKeyReferenceOnDeleteSETNULL, + ForeignKeyReferenceOnDeleteCASCADE, + e.Setting, + ) +} diff --git a/pkg/migrations/op_common_test.go b/pkg/migrations/op_common_test.go index 289a9a26..1862b5d2 100644 --- a/pkg/migrations/op_common_test.go +++ b/pkg/migrations/op_common_test.go @@ -565,6 +565,24 @@ func MustDelete(t *testing.T, db *sql.DB, schema, version, table string, record } } +func MustNotDelete(t *testing.T, db *sql.DB, schema, version, table string, record map[string]string, errorCode string) { + t.Helper() + + err := delete(t, db, schema, version, table, record) + if err == nil { + t.Fatal("Expected DELETE to fail") + } + + var pqErr *pq.Error + if ok := errors.As(err, &pqErr); ok { + if pqErr.Code.Name() != errorCode { + t.Fatalf("Expected DELETE to fail with %q, got %q", errorCode, pqErr.Code.Name()) + } + } else { + t.Fatalf("DELETE failed with unknown error: %v", err) + } +} + func delete(t *testing.T, db *sql.DB, schema, version, table string, record map[string]string) error { t.Helper() versionSchema := roll.VersionedSchemaName(schema, version) diff --git a/pkg/migrations/op_set_fk.go b/pkg/migrations/op_set_fk.go index 1137ad96..2f700dc2 100644 --- a/pkg/migrations/op_set_fk.go +++ b/pkg/migrations/op_set_fk.go @@ -6,6 +6,7 @@ import ( "context" "database/sql" "fmt" + "strings" "github.com/lib/pq" "github.com/xataio/pgroll/pkg/schema" @@ -160,19 +161,38 @@ func (o *OpSetForeignKey) Validate(ctx context.Context, s *schema.Schema) error return FieldRequiredError{Name: "down"} } + switch strings.ToUpper(string(o.References.OnDelete)) { + case string(ForeignKeyReferenceOnDeleteNOACTION): + case string(ForeignKeyReferenceOnDeleteRESTRICT): + case string(ForeignKeyReferenceOnDeleteSETDEFAULT): + case string(ForeignKeyReferenceOnDeleteSETNULL): + case string(ForeignKeyReferenceOnDeleteCASCADE): + case "": + break + default: + return InvalidOnDeleteSettingError{Table: o.Table, Column: o.Column, Setting: string(o.References.OnDelete)} + } + return nil } func (o *OpSetForeignKey) addForeignKeyConstraint(ctx context.Context, conn *sql.DB) error { tempColumnName := TemporaryName(o.Column) - _, 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(o.References.Name), - pq.QuoteIdentifier(tempColumnName), - pq.QuoteIdentifier(o.References.Table), - pq.QuoteIdentifier(o.References.Column), - )) + onDelete := "NO ACTION" + if o.References.OnDelete != "" { + onDelete = strings.ToUpper(string(o.References.OnDelete)) + } + + _, err := conn.ExecContext(ctx, + fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s) ON DELETE %s NOT VALID", + pq.QuoteIdentifier(o.Table), + pq.QuoteIdentifier(o.References.Name), + pq.QuoteIdentifier(tempColumnName), + pq.QuoteIdentifier(o.References.Table), + pq.QuoteIdentifier(o.References.Column), + onDelete, + )) return err } diff --git a/pkg/migrations/op_set_fk_test.go b/pkg/migrations/op_set_fk_test.go index 45f7b092..7148bce2 100644 --- a/pkg/migrations/op_set_fk_test.go +++ b/pkg/migrations/op_set_fk_test.go @@ -180,6 +180,423 @@ func TestSetForeignKey(t *testing.T) { TriggerMustNotExist(t, db, schema, "posts", migrations.TriggerName("posts", migrations.TemporaryName("user_id"))) }, }, + { + name: "on delete NO ACTION is the default behavior when removing referenced rows", + migrations: []migrations.Migration{ + { + Name: "01_add_tables", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "text", + }, + }, + }, + &migrations.OpCreateTable{ + Name: "posts", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "title", + Type: "text", + }, + { + Name: "user_id", + Type: "integer", + Nullable: ptr(true), + }, + }, + }, + }, + }, + { + Name: "02_add_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "posts", + Column: "user_id", + References: &migrations.ForeignKeyReference{ + Name: "fk_users_id", + Table: "users", + Column: "id", + }, + 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"), + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Inserting some data into the `users` table works. + MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "alice", + }) + MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "bob", + }) + + // Inserting data into the new `posts` view with a valid user reference works. + MustInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ + "title": "post by alice", + "user_id": "1", + }) + + // Attempting to delete a row from the `users` table that is referenced + // by a row in the `posts` table fails. + MustNotDelete(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "alice", + }, testutils.FKViolationErrorCode) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Attempting to delete a row from the `users` table that is referenced + // by a row in the `posts` table fails. + MustNotDelete(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "alice", + }, testutils.FKViolationErrorCode) + }, + }, + { + name: "on delete CASCADE allows referenced rows to be removed", + migrations: []migrations.Migration{ + { + Name: "01_add_tables", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "text", + }, + }, + }, + &migrations.OpCreateTable{ + Name: "posts", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "title", + Type: "text", + }, + { + Name: "user_id", + Type: "integer", + Nullable: ptr(true), + }, + }, + }, + }, + }, + { + Name: "02_add_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "posts", + Column: "user_id", + References: &migrations.ForeignKeyReference{ + Name: "fk_users_id", + 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"), + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Inserting some data into the `users` table works. + MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "alice", + }) + MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "bob", + }) + + // Inserting data into the new `posts` view with a valid user reference works. + MustInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ + "title": "post by alice", + "user_id": "1", + }) + + // Attempting to delete a row from the `users` table that is referenced + // by a row in the `posts` table succeeds. + MustDelete(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "alice", + }) + + // The row in the `posts` table that referenced the deleted row in the + // `users` table has been removed. + rows := MustSelect(t, db, schema, "02_add_fk_constraint", "posts") + assert.Empty(t, rows) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Inserting data into the new `posts` view with a valid user reference works. + MustInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ + "title": "post by bob", + "user_id": "2", + }) + + // Attempting to delete a row from the `users` table that is referenced + // by a row in the `posts` table succeeds. + MustDelete(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "bob", + }) + + // The row in the `posts` table that referenced the deleted row in the + // `users` table has been removed. + rows := MustSelect(t, db, schema, "02_add_fk_constraint", "posts") + assert.Empty(t, rows) + }, + }, + { + name: "on delete SET NULL allows referenced rows to be removed", + migrations: []migrations.Migration{ + { + Name: "01_add_tables", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "text", + }, + }, + }, + &migrations.OpCreateTable{ + Name: "posts", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "title", + Type: "text", + }, + { + Name: "user_id", + Type: "integer", + Nullable: ptr(true), + }, + }, + }, + }, + }, + { + Name: "02_add_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "posts", + Column: "user_id", + References: &migrations.ForeignKeyReference{ + Name: "fk_users_id", + Table: "users", + Column: "id", + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, + }, + 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"), + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Inserting some data into the `users` table works. + MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "alice", + }) + MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "bob", + }) + + // Inserting data into the new `posts` view with a valid user reference works. + MustInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ + "title": "post by alice", + "user_id": "1", + }) + + // Attempting to delete a row from the `users` table that is referenced + // by a row in the `posts` table succeeds. + MustDelete(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "alice", + }) + + // The user_id of the deleted row in the `posts` table is set to NULL. + rows := MustSelect(t, db, schema, "02_add_fk_constraint", "posts") + assert.Equal(t, []map[string]any{ + {"id": 1, "title": "post by alice", "user_id": nil}, + }, rows) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Inserting data into the new `posts` view with a valid user reference works. + MustInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ + "title": "post by bob", + "user_id": "2", + }) + + // Attempting to delete a row from the `users` table that is referenced + // by a row in the `posts` table succeeds. + MustDelete(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "bob", + }) + + // The user_id of the deleted row in the `posts` table is set to NULL. + rows := MustSelect(t, db, schema, "02_add_fk_constraint", "posts") + assert.Equal(t, []map[string]any{ + {"id": 1, "title": "post by alice", "user_id": nil}, + {"id": 2, "title": "post by bob", "user_id": nil}, + }, rows) + }, + }, + { + name: "on delete SET DEFAULT allows referenced rows to be removed", + migrations: []migrations.Migration{ + { + Name: "01_add_tables", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "text", + }, + }, + }, + &migrations.OpCreateTable{ + Name: "posts", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "title", + Type: "text", + }, + { + Name: "user_id", + Type: "integer", + Nullable: ptr(true), + Default: ptr("3"), + }, + }, + }, + }, + }, + { + Name: "02_add_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "posts", + Column: "user_id", + References: &migrations.ForeignKeyReference{ + Name: "fk_users_id", + Table: "users", + Column: "id", + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETDEFAULT, + }, + 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"), + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Inserting some data into the `users` table works. + MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "alice", + }) + MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "bob", + }) + MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "carl", + }) + + // Inserting data into the new `posts` view with a valid user reference works. + MustInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ + "title": "post by alice", + "user_id": "1", + }) + + // Attempting to delete a row from the `users` table that is referenced + // by a row in the `posts` table succeeds. + MustDelete(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "alice", + }) + + // The user_id of the deleted row in the `posts` table is set to its default value. + rows := MustSelect(t, db, schema, "02_add_fk_constraint", "posts") + assert.Equal(t, []map[string]any{ + {"id": 1, "title": "post by alice", "user_id": 3}, + }, rows) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Inserting data into the new `posts` view with a valid user reference works. + MustInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ + "title": "post by bob", + "user_id": "2", + }) + + // Attempting to delete a row from the `users` table that is referenced + // by a row in the `posts` table succeeds. + MustDelete(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "bob", + }) + + // The user_id of the deleted row in the `posts` table is set to its default value. + rows := MustSelect(t, db, schema, "02_add_fk_constraint", "posts") + assert.Equal(t, []map[string]any{ + {"id": 1, "title": "post by alice", "user_id": 3}, + {"id": 2, "title": "post by bob", "user_id": 3}, + }, rows) + }, + }, { name: "column defaults are preserved when adding a foreign key constraint", migrations: []migrations.Migration{ @@ -757,5 +1174,81 @@ func TestSetForeignKeyValidation(t *testing.T) { Err: migrations.ColumnDoesNotExistError{Table: "users", Name: "doesntexist"}, }, }, + { + name: "on_delete must be a valid value", + migrations: []migrations.Migration{ + createTablesMigration, + { + Name: "02_add_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "posts", + Column: "user_id", + References: &migrations.ForeignKeyReference{ + Name: "fk_users_doesntexist", + Table: "users", + Column: "id", + OnDelete: "invalid", + }, + 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"), + }, + }, + }, + }, + wantStartErr: migrations.InvalidOnDeleteSettingError{ + Table: "posts", + Column: "user_id", + Setting: "invalid", + }, + }, + { + name: "on_delete can be specified as lowercase", + migrations: []migrations.Migration{ + createTablesMigration, + { + Name: "02_add_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "posts", + Column: "user_id", + References: &migrations.ForeignKeyReference{ + Name: "fk_users_doesntexist", + Table: "users", + Column: "id", + OnDelete: "no action", + }, + 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"), + }, + }, + }, + }, + wantStartErr: nil, + }, + { + name: "on_delete can be specified as uppercase", + migrations: []migrations.Migration{ + createTablesMigration, + { + Name: "02_add_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "posts", + Column: "user_id", + References: &migrations.ForeignKeyReference{ + Name: "fk_users_doesntexist", + Table: "users", + Column: "id", + OnDelete: "SET NULL", + }, + 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"), + }, + }, + }, + }, + wantStartErr: nil, + }, }) } diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 4e047ec7..5613b75c 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -50,10 +50,21 @@ type ForeignKeyReference struct { // Name of the foreign key constraint Name string `json:"name"` + // On delete behavior of the foreign key constraint + OnDelete ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` + // Name of the referenced table Table string `json:"table"` } +type ForeignKeyReferenceOnDelete string + +const ForeignKeyReferenceOnDeleteCASCADE ForeignKeyReferenceOnDelete = "CASCADE" +const ForeignKeyReferenceOnDeleteNOACTION ForeignKeyReferenceOnDelete = "NO ACTION" +const ForeignKeyReferenceOnDeleteRESTRICT ForeignKeyReferenceOnDelete = "RESTRICT" +const ForeignKeyReferenceOnDeleteSETDEFAULT ForeignKeyReferenceOnDelete = "SET DEFAULT" +const ForeignKeyReferenceOnDeleteSETNULL ForeignKeyReferenceOnDelete = "SET NULL" + // Add column operation type OpAddColumn struct { // Column to add diff --git a/schema.json b/schema.json index 4f310d50..203f42b3 100644 --- a/schema.json +++ b/schema.json @@ -80,6 +80,12 @@ "table": { "description": "Name of the referenced table", "type": "string" + }, + "on_delete": { + "description": "On delete behavior of the foreign key constraint", + "type": "string", + "enum": ["NO ACTION", "RESTRICT", "CASCADE", "SET NULL", "SET DEFAULT"], + "default": "NO ACTION" } }, "required": ["column", "name", "table"],