From 9c9518b2708b1889719b5376e7536ebb0031605c Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Tue, 26 Nov 2024 16:50:30 +0000 Subject: [PATCH] Add a `drop_multicolumn_constraint` operation (#487) Add a new `drop_multicolumn_constraint` operation type. The new operation is used to drop multi-column (and single-column) `CHECK`, `UNIQUE`, and `FOREIGN KEY` constraints: The following migration drops a multi-column constraint called `check_zip_name`, and defines `up` and `down` SQL data migrations for each column covered by the constraint: ```json { "name": "48_drop_tickets_check", "operations": [ { "drop_multicolumn_constraint": { "table": "tickets", "name": "check_zip_name", "up": { "sellers_name": "sellers_name", "sellers_zip": "sellers_zip" }, "down": { "sellers_name": "sellers_name", "sellers_zip": "sellers_zip" } } } ] } ``` * On operation `Start` each column covered by the constraint is duplicated, ignoring the constraint to be dropped. Triggers for data migrations are created for each column covered by the constraint using the SQL for each column from the migration file. * On `Complete`, the duplicated column is renamed to the original name and triggers are removed. * `Validate` ensures that all columns covered by the constraint have data migrations defined, and only columns covered by the constraint have data migrations defined. The new operation is in addition to the existing `drop_constraint` operation which only drops single column constraints. This old operation type is preserved for backwards compatibility but will be removed as a breaking change before a v1 release. https://github.com/xataio/pgroll/issues/489 tracks the removal of the `drop_constraint` operation. --- docs/README.md | 33 +- examples/.ledger | 1 + examples/48_drop_tickets_check.json | 19 + pkg/migrations/errors.go | 9 + pkg/migrations/op_common.go | 33 +- .../op_drop_multicolumn_constraint.go | 196 ++++++ .../op_drop_multicolumn_constraint_test.go | 657 ++++++++++++++++++ pkg/migrations/types.go | 35 +- schema.json | 55 +- 9 files changed, 1008 insertions(+), 30 deletions(-) create mode 100644 examples/48_drop_tickets_check.json create mode 100644 pkg/migrations/op_drop_multicolumn_constraint.go create mode 100644 pkg/migrations/op_drop_multicolumn_constraint_test.go diff --git a/docs/README.md b/docs/README.md index 3e98a860..2d9fd3ec 100644 --- a/docs/README.md +++ b/docs/README.md @@ -33,6 +33,7 @@ * [Create constraint](#create-constraint) * [Drop column](#drop-column) * [Drop constraint](#drop-constraint) + * [Drop multi-column constraint](#drop-multi-column-constraint) * [Drop index](#drop-index) * [Drop table](#drop-table) * [Raw SQL](#raw-sql) @@ -793,6 +794,7 @@ See the [examples](../examples) directory for examples of each kind of operation * [Create constraint](#create-constraint) * [Drop column](#drop-column) * [Drop constraint](#drop-constraint) +* [Drop multi-column constraint](#drop-multi-column-constraint) * [Drop index](#drop-index) * [Drop table](#drop-table) * [Raw SQL](#raw-sql) @@ -1208,7 +1210,7 @@ Example **drop column** migrations: ### Drop constraint -A drop constraint operation drops a constraint from an existing table. +A drop constraint operation drops a single-column constraint from an existing table. Only `CHECK`, `FOREIGN KEY`, and `UNIQUE` constraints can be dropped. @@ -1231,6 +1233,35 @@ Example **drop constraint** migrations: * [24_drop_foreign_key_constraint.json](../examples/24_drop_foreign_key_constraint.json) * [27_drop_unique_constraint.json](../examples/27_drop_unique_constraint.json) +### Drop multi-column constraint + +A drop constraint operation drops a multi-column constraint from an existing table. + +Only `CHECK`, `FOREIGN KEY`, and `UNIQUE` constraints can be dropped. + +**drop multi-column constraint** operations have this structure: + +```json +{ + "drop_multicolumn_constraint": { + "table": "name of table", + "name": "name of constraint to drop", + "up": { + "column1": "up SQL expressions for each column covered by the constraint", + ... + }, + "down": { + "column1": "down SQL expressions for each column covered by the constraint", + ... + } + } +} +``` + +Example **drop multi-column constraint** migrations: + +* [48_drop_tickets_check.json](../examples/48_drop_tickets_check.json) + ### Drop index A drop index operation drops an index from a table. diff --git a/examples/.ledger b/examples/.ledger index 689b88b3..33826846 100644 --- a/examples/.ledger +++ b/examples/.ledger @@ -45,3 +45,4 @@ 45_add_table_check_constraint.json 46_alter_column_drop_default.json 47_add_table_foreign_key_constraint.json +48_drop_tickets_check.json diff --git a/examples/48_drop_tickets_check.json b/examples/48_drop_tickets_check.json new file mode 100644 index 00000000..a35c7c6b --- /dev/null +++ b/examples/48_drop_tickets_check.json @@ -0,0 +1,19 @@ +{ + "name": "48_drop_tickets_check", + "operations": [ + { + "drop_multicolumn_constraint": { + "table": "tickets", + "name": "check_zip_name", + "up": { + "sellers_name": "sellers_name", + "sellers_zip": "sellers_zip" + }, + "down": { + "sellers_name": "sellers_name", + "sellers_zip": "sellers_zip" + } + } + } + ] +} diff --git a/pkg/migrations/errors.go b/pkg/migrations/errors.go index 344d9fab..fb803a9a 100644 --- a/pkg/migrations/errors.go +++ b/pkg/migrations/errors.go @@ -63,6 +63,15 @@ func (e ColumnMigrationMissingError) Error() string { return fmt.Sprintf("migration for column %q in %q is missing", e.Name, e.Table) } +type ColumnMigrationRedundantError struct { + Table string + Name string +} + +func (e ColumnMigrationRedundantError) Error() string { + return fmt.Sprintf("migration for column %q in %q is redundant", e.Name, e.Table) +} + type ColumnIsNotNullableError struct { Table string Name string diff --git a/pkg/migrations/op_common.go b/pkg/migrations/op_common.go index 97d73703..0d6a144b 100644 --- a/pkg/migrations/op_common.go +++ b/pkg/migrations/op_common.go @@ -12,19 +12,20 @@ import ( type OpName string const ( - OpNameCreateTable OpName = "create_table" - OpNameRenameTable OpName = "rename_table" - OpNameDropTable OpName = "drop_table" - OpNameAddColumn OpName = "add_column" - OpNameDropColumn OpName = "drop_column" - OpNameAlterColumn OpName = "alter_column" - OpNameCreateIndex OpName = "create_index" - OpNameDropIndex OpName = "drop_index" - OpNameRenameConstraint OpName = "rename_constraint" - OpNameDropConstraint OpName = "drop_constraint" - OpNameSetReplicaIdentity OpName = "set_replica_identity" - OpRawSQLName OpName = "sql" - OpCreateConstraintName OpName = "create_constraint" + OpNameCreateTable OpName = "create_table" + OpNameRenameTable OpName = "rename_table" + OpNameDropTable OpName = "drop_table" + OpNameAddColumn OpName = "add_column" + OpNameDropColumn OpName = "drop_column" + OpNameAlterColumn OpName = "alter_column" + OpNameCreateIndex OpName = "create_index" + OpNameDropIndex OpName = "drop_index" + OpNameRenameConstraint OpName = "rename_constraint" + OpNameDropConstraint OpName = "drop_constraint" + OpNameSetReplicaIdentity OpName = "set_replica_identity" + OpNameDropMultiColumnConstraint OpName = "drop_multicolumn_constraint" + OpRawSQLName OpName = "sql" + OpCreateConstraintName OpName = "create_constraint" // Internal operation types used by `alter_column` @@ -129,6 +130,9 @@ func (v *Operations) UnmarshalJSON(data []byte) error { case OpCreateConstraintName: item = &OpCreateConstraint{} + case OpNameDropMultiColumnConstraint: + item = &OpDropMultiColumnConstraint{} + default: return fmt.Errorf("unknown migration type: %v", opName) } @@ -218,6 +222,9 @@ func OperationName(op Operation) OpName { case *OpCreateConstraint: return OpCreateConstraintName + case *OpDropMultiColumnConstraint: + return OpNameDropMultiColumnConstraint + } panic(fmt.Errorf("unknown operation for %T", op)) diff --git a/pkg/migrations/op_drop_multicolumn_constraint.go b/pkg/migrations/op_drop_multicolumn_constraint.go new file mode 100644 index 00000000..c7acafeb --- /dev/null +++ b/pkg/migrations/op_drop_multicolumn_constraint.go @@ -0,0 +1,196 @@ +// SPDX-License-Identifier: Apache-2.0 + +package migrations + +import ( + "context" + "fmt" + "slices" + + "github.com/lib/pq" + "github.com/xataio/pgroll/pkg/db" + "github.com/xataio/pgroll/pkg/schema" +) + +var _ Operation = (*OpDropMultiColumnConstraint)(nil) + +func (o *OpDropMultiColumnConstraint) Start(ctx context.Context, conn db.DB, latestSchema string, tr SQLTransformer, s *schema.Schema, cbs ...CallbackFn) (*schema.Table, error) { + table := s.GetTable(o.Table) + + // Get all columns covered by the constraint to be dropped + constraintColumns := table.GetConstraintColumns(o.Name) + columns := make([]*schema.Column, len(constraintColumns)) + for i, c := range constraintColumns { + columns[i] = table.GetColumn(c) + } + + // Duplicate each of the columns covered by the constraint to be dropped + d := NewColumnDuplicator(conn, table, columns...).WithoutConstraint(o.Name) + if err := d.Duplicate(ctx); err != nil { + return nil, fmt.Errorf("failed to duplicate column: %w", err) + } + + // Create triggers for each column covered by the constraint to be dropped + for _, columnName := range table.GetConstraintColumns(o.Name) { + // Add a trigger to copy values from the old column to the new, rewriting values using the `up` SQL. + err := createTrigger(ctx, conn, tr, triggerConfig{ + Name: TriggerName(o.Table, columnName), + Direction: TriggerDirectionUp, + Columns: table.Columns, + SchemaName: s.Name, + LatestSchema: latestSchema, + TableName: o.Table, + PhysicalColumn: TemporaryName(columnName), + SQL: o.upSQL(columnName), + }) + if err != nil { + return nil, fmt.Errorf("failed to create up trigger: %w", err) + } + + // Add the new column to the internal schema representation. This is done + // here, before creation of the down trigger, so that the trigger can declare + // a variable for the new column. + table.AddColumn(columnName, schema.Column{ + Name: TemporaryName(columnName), + }) + + // Add a trigger to copy values from the new column to the old, rewriting values using the `down` SQL. + err = createTrigger(ctx, conn, tr, triggerConfig{ + Name: TriggerName(o.Table, TemporaryName(columnName)), + Direction: TriggerDirectionDown, + Columns: table.Columns, + SchemaName: s.Name, + LatestSchema: latestSchema, + TableName: o.Table, + PhysicalColumn: columnName, + SQL: o.Down[columnName], + }) + if err != nil { + return nil, fmt.Errorf("failed to create down trigger: %w", err) + } + } + + return table, nil +} + +func (o *OpDropMultiColumnConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { + table := s.GetTable(o.Table) + + for _, columnName := range table.GetConstraintColumns(o.Name) { + // Remove the up function and trigger + _, err := conn.ExecContext(ctx, fmt.Sprintf("DROP FUNCTION IF EXISTS %s CASCADE", + pq.QuoteIdentifier(TriggerFunctionName(o.Table, columnName)))) + if err != nil { + return err + } + + // Remove the down function and trigger + _, err = conn.ExecContext(ctx, fmt.Sprintf("DROP FUNCTION IF EXISTS %s CASCADE", + pq.QuoteIdentifier(TriggerFunctionName(o.Table, TemporaryName(columnName))))) + if err != nil { + return err + } + + // Drop the old column + _, err = conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s DROP COLUMN IF EXISTS %s", + pq.QuoteIdentifier(o.Table), + pq.QuoteIdentifier(columnName))) + if err != nil { + return err + } + + // Rename the new column to the old column name + column := table.GetColumn(columnName) + if err := RenameDuplicatedColumn(ctx, conn, table, column); err != nil { + return err + } + } + + return nil +} + +func (o *OpDropMultiColumnConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { + table := s.GetTable(o.Table) + + for _, columnName := range table.GetConstraintColumns(o.Name) { + // Drop the new column + _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s DROP COLUMN IF EXISTS %s", + pq.QuoteIdentifier(o.Table), + pq.QuoteIdentifier(TemporaryName(columnName)), + )) + if err != nil { + return err + } + + // Remove the up function and trigger + _, err = conn.ExecContext(ctx, fmt.Sprintf("DROP FUNCTION IF EXISTS %s CASCADE", + pq.QuoteIdentifier(TriggerFunctionName(o.Table, columnName)), + )) + if err != nil { + return err + } + + // Remove the down function and trigger + _, err = conn.ExecContext(ctx, fmt.Sprintf("DROP FUNCTION IF EXISTS %s CASCADE", + pq.QuoteIdentifier(TriggerFunctionName(o.Table, TemporaryName(columnName))), + )) + if err != nil { + return err + } + } + + return nil +} + +func (o *OpDropMultiColumnConstraint) Validate(ctx context.Context, s *schema.Schema) error { + table := s.GetTable(o.Table) + if table == nil { + return TableDoesNotExistError{Name: o.Table} + } + + if o.Name == "" { + return FieldRequiredError{Name: "name"} + } + + if !table.ConstraintExists(o.Name) { + return ConstraintDoesNotExistError{Table: o.Table, Constraint: o.Name} + } + + if o.Down == nil { + return FieldRequiredError{Name: "down"} + } + + // Ensure that `down` migrations are present for all columns covered by the + // constraint to be dropped. + for _, columnName := range table.GetConstraintColumns(o.Name) { + if _, ok := o.Down[columnName]; !ok { + return ColumnMigrationMissingError{ + Table: o.Table, + Name: columnName, + } + } + } + + // Ensure that only columns covered by the constraint are present in the + // `up` and `down` migrations. + for _, m := range []map[string]string{o.Down, o.Up} { + for columnName := range m { + if !slices.Contains(table.GetConstraintColumns(o.Name), columnName) { + return ColumnMigrationRedundantError{ + Table: o.Table, + Name: columnName, + } + } + } + } + + return nil +} + +func (o *OpDropMultiColumnConstraint) upSQL(column string) string { + if o.Up[column] != "" { + return o.Up[column] + } + + return pq.QuoteIdentifier(column) +} diff --git a/pkg/migrations/op_drop_multicolumn_constraint_test.go b/pkg/migrations/op_drop_multicolumn_constraint_test.go new file mode 100644 index 00000000..62b9c7dc --- /dev/null +++ b/pkg/migrations/op_drop_multicolumn_constraint_test.go @@ -0,0 +1,657 @@ +// SPDX-License-Identifier: Apache-2.0 + +package migrations_test + +import ( + "database/sql" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/xataio/pgroll/internal/testutils" + "github.com/xataio/pgroll/pkg/migrations" +) + +func TestDropMultiColumnConstraint(t *testing.T) { + t.Parallel() + + ExecuteTests(t, TestCases{ + { + name: "can drop a multi-column check constraint", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "products", + Columns: []migrations.Column{ + { + Name: "id", + Type: "integer", + Pk: ptr(true), + }, + { + Name: "name", + Type: "text", + }, + { + Name: "price", + Type: "integer", + }, + { + Name: "discount", + Type: "integer", + }, + }, + }, + }, + }, + { + Name: "02_create_check_constraint", + Operations: migrations.Operations{ + &migrations.OpCreateConstraint{ + Table: "products", + Name: "products_check_price_discount", + Columns: []string{"price", "discount"}, + Type: migrations.OpCreateConstraintTypeCheck, + Check: ptr("(discount = 0) OR (price > 0)"), + Up: map[string]string{ + "price": "price", + "discount": "discount", + }, + Down: map[string]string{ + "price": "price", + "discount": "discount", + }, + }, + }, + }, + { + Name: "03_drop_check_constraint", + Operations: migrations.Operations{ + &migrations.OpDropMultiColumnConstraint{ + Table: "products", + Name: "products_check_price_discount", + Up: map[string]string{ + "price": "price", + "discount": "discount", + }, + Down: map[string]string{ + "price": "SELECT CASE price WHEN 0 THEN 100 ELSE 0 END", + "discount": "discount", + }, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row into old schema that violates the check constraint fails. + MustNotInsert(t, db, schema, "02_create_check_constraint", "products", map[string]string{ + "id": "1", + "name": "apple", + "price": "0", + "discount": "1", + }, testutils.CheckViolationErrorCode) + + // Inserting a row into old schema that meets the check constraint succeeds. + MustInsert(t, db, schema, "02_create_check_constraint", "products", map[string]string{ + "id": "1", + "name": "apple", + "price": "1", + "discount": "1", + }) + + // Inserting a row into the new schema that violates the check constraint succeeds. + MustInsert(t, db, schema, "03_drop_check_constraint", "products", map[string]string{ + "id": "2", + "name": "banana", + "price": "0", + "discount": "1", + }) + + // The `products` table in the new schema has the expected data. + rows := MustSelect(t, db, schema, "03_drop_check_constraint", "products") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "apple", "price": 1, "discount": 1}, + {"id": 2, "name": "banana", "price": 0, "discount": 1}, + }, rows) + + // The `products` table in the old schema has the expected data. + // The row that violated the check constraint was migrated using the down SQL. + rows = MustSelect(t, db, schema, "02_create_check_constraint", "products") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "apple", "price": 1, "discount": 1}, + {"id": 2, "name": "banana", "price": 100, "discount": 1}, + }, rows) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The temporary columns no longer exist. + ColumnMustNotExist(t, db, schema, "products", migrations.TemporaryName("price")) + ColumnMustNotExist(t, db, schema, "products", migrations.TemporaryName("discount")) + + // The up functions no longer exist. + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("products", "price")) + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("products", "discount")) + + // The down functions no longer exist. + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("products", migrations.TemporaryName("price"))) + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("products", migrations.TemporaryName("discount"))) + + // The up triggers no longer exist. + TriggerMustNotExist(t, db, schema, "products", migrations.TriggerName("products", "price")) + TriggerMustNotExist(t, db, schema, "products", migrations.TriggerName("products", "discount")) + + // The down triggers no longer exist. + TriggerMustNotExist(t, db, schema, "products", migrations.TriggerName("products", migrations.TemporaryName("price"))) + TriggerMustNotExist(t, db, schema, "products", migrations.TriggerName("products", migrations.TemporaryName("discount"))) + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row into the new schema that violates the check constraint succeeds. + MustInsert(t, db, schema, "03_drop_check_constraint", "products", map[string]string{ + "id": "3", + "name": "carrot", + "price": "0", + "discount": "1", + }) + + // The `products` table in the new schema has the expected data. + rows := MustSelect(t, db, schema, "03_drop_check_constraint", "products") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "apple", "price": 1, "discount": 1}, + {"id": 2, "name": "banana", "price": 100, "discount": 1}, + {"id": 3, "name": "carrot", "price": 0, "discount": 1}, + }, rows) + }, + }, + { + name: "can drop a multi-column unique constraint", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "products", + Columns: []migrations.Column{ + { + Name: "id", + Type: "integer", + Pk: ptr(true), + }, + { + Name: "name", + Type: "text", + }, + { + Name: "description", + Type: "text", + }, + }, + }, + }, + }, + { + Name: "02_create_unique_constraint", + Operations: migrations.Operations{ + &migrations.OpCreateConstraint{ + Table: "products", + Name: "products_unique_name_description", + Columns: []string{"name", "description"}, + Type: migrations.OpCreateConstraintTypeUnique, + Up: map[string]string{ + "name": "name", + "description": "description", + }, + Down: map[string]string{ + "name": "name", + "description": "description", + }, + }, + }, + }, + { + Name: "03_drop_unique_constraint", + Operations: migrations.Operations{ + &migrations.OpDropMultiColumnConstraint{ + Table: "products", + Name: "products_unique_name_description", + Up: map[string]string{ + "name": "name", + "description": "description", + }, + Down: map[string]string{ + "name": "name || '-foo'", + "description": "description", + }, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Inserting an initial row into old schema succeeds. + MustInsert(t, db, schema, "02_create_unique_constraint", "products", map[string]string{ + "id": "1", "name": "apple", "description": "red", + }) + + // Inserting a row into the old schema that violates the unique constraint fails. + MustNotInsert(t, db, schema, "02_create_unique_constraint", "products", map[string]string{ + "id": "2", "name": "apple", "description": "red", + }, testutils.UniqueViolationErrorCode) + + // Inserting a row into the new schema that violates the unique constraint succeeds. + MustInsert(t, db, schema, "03_drop_unique_constraint", "products", map[string]string{ + "id": "2", "name": "apple", "description": "red", + }) + + // The `products` table in the new schema has the expected data. + rows := MustSelect(t, db, schema, "03_drop_unique_constraint", "products") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "apple", "description": "red"}, + {"id": 2, "name": "apple", "description": "red"}, + }, rows) + + // The `products` table in the old schema has the expected data. + // The row that violated the unique constraint was migrated using the down SQL. + rows = MustSelect(t, db, schema, "02_create_unique_constraint", "products") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "apple", "description": "red"}, + {"id": 2, "name": "apple-foo", "description": "red"}, + }, rows) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The temporary columns no longer exist. + ColumnMustNotExist(t, db, schema, "products", migrations.TemporaryName("name")) + ColumnMustNotExist(t, db, schema, "products", migrations.TemporaryName("description")) + + // The up functions no longer exist. + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("products", "name")) + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("products", "description")) + + // The down functions no longer exist. + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("products", migrations.TemporaryName("name"))) + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("products", migrations.TemporaryName("description"))) + + // The up triggers no longer exist. + TriggerMustNotExist(t, db, schema, "products", migrations.TriggerName("products", "name")) + TriggerMustNotExist(t, db, schema, "products", migrations.TriggerName("products", "description")) + + // The down triggers no longer exist. + TriggerMustNotExist(t, db, schema, "products", migrations.TriggerName("products", migrations.TemporaryName("name"))) + TriggerMustNotExist(t, db, schema, "products", migrations.TriggerName("products", migrations.TemporaryName("description"))) + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row into the new schema that violates the unique constraint succeeds. + MustInsert(t, db, schema, "03_drop_unique_constraint", "products", map[string]string{ + "id": "3", "name": "apple", "description": "red", + }) + + // The `products` table in the new schema has the expected data. + rows := MustSelect(t, db, schema, "03_drop_unique_constraint", "products") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "apple", "description": "red"}, + {"id": 2, "name": "apple-foo", "description": "red"}, + {"id": 3, "name": "apple", "description": "red"}, + }, rows) + }, + }, + { + name: "can drop a multi-column foreign key constraint", + migrations: []migrations.Migration{ + { + Name: "01_create_tables", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "integer", + Pk: ptr(true), + }, + { + Name: "name", + Type: "varchar(255)", + Nullable: ptr(false), + }, + { + Name: "zip", + Type: "integer", + Pk: ptr(true), + }, + }, + }, + &migrations.OpCreateTable{ + Name: "reports", + Columns: []migrations.Column{ + { + Name: "id", + Type: "integer", + Pk: ptr(true), + }, + { + Name: "description", + Type: "varchar(255)", + Nullable: ptr(false), + }, + { + Name: "user_id", + Type: "integer", + Nullable: ptr(true), + }, + { + Name: "user_zip", + Type: "integer", + Nullable: ptr(true), + }, + }, + }, + }, + }, + { + Name: "02_create_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpCreateConstraint{ + Name: "fk_users_reports", + Table: "reports", + Type: migrations.OpCreateConstraintTypeForeignKey, + Columns: []string{"user_id", "user_zip"}, + References: &migrations.OpCreateConstraintReferences{ + Table: "users", + Columns: []string{"id", "zip"}, + }, + Up: map[string]string{ + "user_id": "user_id", + "user_zip": "user_zip", + }, + Down: map[string]string{ + "user_id": "user_id", + "user_zip": "user_zip", + }, + }, + }, + }, + { + Name: "03_drop_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpDropMultiColumnConstraint{ + Table: "reports", + Name: "fk_users_reports", + Up: map[string]string{ + "user_id": "user_id", + "user_zip": "user_zip", + }, + Down: map[string]string{ + "user_id": "1", + "user_zip": "user_zip", + }, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Insert a seed row into the `users` table. + MustInsert(t, db, schema, "03_drop_fk_constraint", "users", map[string]string{ + "id": "1", "name": "alice", "zip": "12345", + }) + + // Inserting a row that meets the FK constraint into the old schema succeeds. + MustInsert(t, db, schema, "02_create_fk_constraint", "reports", map[string]string{ + "id": "1", "description": "awesome", "user_id": "1", "user_zip": "12345", + }) + + // Inserting a row that violates the FK constraint into the old schema fails. + MustNotInsert(t, db, schema, "02_create_fk_constraint", "reports", map[string]string{ + "id": "2", "description": "awesome", "user_id": "2", "user_zip": "12345", + }, testutils.FKViolationErrorCode) + + // Inserting a row that violates the FK constraint into the new schema succeeds. + MustInsert(t, db, schema, "03_drop_fk_constraint", "reports", map[string]string{ + "id": "2", "description": "better", "user_id": "2", "user_zip": "12345", + }) + + // The `reports` table in the new schema contains the expected rows + rows := MustSelect(t, db, schema, "03_drop_fk_constraint", "reports") + assert.Equal(t, []map[string]any{ + {"id": 1, "description": "awesome", "user_id": 1, "user_zip": 12345}, + {"id": 2, "description": "better", "user_id": 2, "user_zip": 12345}, + }, rows) + + // The `reports` table in the new schema contains the expected rows + // The row that violated the FK constraint was migrated using the down SQL. + rows = MustSelect(t, db, schema, "02_create_fk_constraint", "reports") + assert.Equal(t, []map[string]any{ + {"id": 1, "description": "awesome", "user_id": 1, "user_zip": 12345}, + {"id": 2, "description": "better", "user_id": 1, "user_zip": 12345}, + }, rows) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The temporary columns no longer exist. + ColumnMustNotExist(t, db, schema, "reports", migrations.TemporaryName("user_id")) + ColumnMustNotExist(t, db, schema, "reports", migrations.TemporaryName("user_zip")) + + // The up functions no longer exist. + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("reports", "user_id")) + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("reports", "user_zip")) + + // The down functions no longer exist. + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("reports", migrations.TemporaryName("user_id"))) + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("reports", migrations.TemporaryName("user_zip"))) + + // The up triggers no longer exist. + TriggerMustNotExist(t, db, schema, "reports", migrations.TriggerName("reports", "user_id")) + TriggerMustNotExist(t, db, schema, "reports", migrations.TriggerName("reports", "user_zip")) + + // The down triggers no longer exist. + TriggerMustNotExist(t, db, schema, "reports", migrations.TriggerName("reports", migrations.TemporaryName("user_id"))) + TriggerMustNotExist(t, db, schema, "reports", migrations.TriggerName("reports", migrations.TemporaryName("user_zip"))) + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row that violates the FK constraint into the new schema succeeds. + MustInsert(t, db, schema, "03_drop_fk_constraint", "reports", map[string]string{ + "id": "3", "description": "consistent", "user_id": "2", "user_zip": "12345", + }) + + // The `reports` table in the new schema contains the expected rows + rows := MustSelect(t, db, schema, "03_drop_fk_constraint", "reports") + assert.Equal(t, []map[string]any{ + {"id": 1, "description": "awesome", "user_id": 1, "user_zip": 12345}, + {"id": 2, "description": "better", "user_id": 1, "user_zip": 12345}, + {"id": 3, "description": "consistent", "user_id": 2, "user_zip": 12345}, + }, rows) + }, + }, + }) +} + +func TestDropMultiColumnConstraintValidation(t *testing.T) { + t.Parallel() + + createTableMigration := migrations.Migration{ + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "posts", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "title", + Type: "text", + }, + }, + }, + }, + } + addCheckMigration := migrations.Migration{ + Name: "02_add_check_constraint", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "posts", + Column: "title", + Check: &migrations.CheckConstraint{ + Name: "check_title_length", + Constraint: "length(title) > 3", + }, + Up: "SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END", + Down: "title", + }, + }, + } + + ExecuteTests(t, TestCases{ + { + name: "table must exist", + migrations: []migrations.Migration{ + createTableMigration, + addCheckMigration, + { + Name: "03_drop_check_constraint", + Operations: migrations.Operations{ + &migrations.OpDropMultiColumnConstraint{ + Table: "doesntexist", + Name: "check_title_length", + Up: map[string]string{ + "title": "title", + }, + Down: map[string]string{ + "title": "SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END", + }, + }, + }, + }, + }, + wantStartErr: migrations.TableDoesNotExistError{Name: "doesntexist"}, + }, + { + name: "constraint must exist", + migrations: []migrations.Migration{ + createTableMigration, + addCheckMigration, + { + Name: "03_drop_check_constraint", + Operations: migrations.Operations{ + &migrations.OpDropMultiColumnConstraint{ + Table: "posts", + Name: "doesntexist", + Up: map[string]string{ + "title": "title", + }, + Down: map[string]string{ + "title": "SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END", + }, + }, + }, + }, + }, + wantStartErr: migrations.ConstraintDoesNotExistError{Table: "posts", Constraint: "doesntexist"}, + }, + { + name: "name is mandatory", + migrations: []migrations.Migration{ + createTableMigration, + addCheckMigration, + { + Name: "03_drop_check_constraint", + Operations: migrations.Operations{ + &migrations.OpDropMultiColumnConstraint{ + Table: "posts", + Up: map[string]string{ + "title": "title", + }, + Down: map[string]string{ + "title": "SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END", + }, + }, + }, + }, + }, + wantStartErr: migrations.FieldRequiredError{Name: "name"}, + }, + { + name: "down SQL is mandatory", + migrations: []migrations.Migration{ + createTableMigration, + addCheckMigration, + { + Name: "03_drop_check_constraint", + Operations: migrations.Operations{ + &migrations.OpDropMultiColumnConstraint{ + Table: "posts", + Name: "check_title_length", + Up: map[string]string{ + "title": "title", + }, + }, + }, + }, + }, + wantStartErr: migrations.FieldRequiredError{Name: "down"}, + }, + { + name: "down SQL must be present for all columns covered by the constraint", + migrations: []migrations.Migration{ + createTableMigration, + addCheckMigration, + { + Name: "03_drop_check_constraint", + Operations: migrations.Operations{ + &migrations.OpDropMultiColumnConstraint{ + Table: "posts", + Name: "check_title_length", + Up: map[string]string{ + "title": "title", + }, + Down: map[string]string{}, + }, + }, + }, + }, + wantStartErr: migrations.ColumnMigrationMissingError{Table: "posts", Name: "title"}, + }, + { + name: "down SQL for columns not covered by the constraint is not allowed", + migrations: []migrations.Migration{ + createTableMigration, + addCheckMigration, + { + Name: "03_drop_check_constraint", + Operations: migrations.Operations{ + &migrations.OpDropMultiColumnConstraint{ + Table: "posts", + Name: "check_title_length", + Down: map[string]string{ + "title": "SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END", + "not_covered": "not_covered", + }, + }, + }, + }, + }, + wantStartErr: migrations.ColumnMigrationRedundantError{Table: "posts", Name: "not_covered"}, + }, + { + name: "up SQL for columns not covered by the constraint is not allowed", + migrations: []migrations.Migration{ + createTableMigration, + addCheckMigration, + { + Name: "03_drop_check_constraint", + Operations: migrations.Operations{ + &migrations.OpDropMultiColumnConstraint{ + Table: "posts", + Name: "check_title_length", + Up: map[string]string{ + "not_covered": "not_covered", + }, + Down: map[string]string{ + "title": "SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END", + }, + }, + }, + }, + }, + wantStartErr: migrations.ColumnMigrationRedundantError{Table: "posts", Name: "not_covered"}, + }, + }) +} diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 59acb96c..8cc6b402 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -67,6 +67,12 @@ const ForeignKeyReferenceOnDeleteRESTRICT ForeignKeyReferenceOnDelete = "RESTRIC const ForeignKeyReferenceOnDeleteSETDEFAULT ForeignKeyReferenceOnDelete = "SET DEFAULT" const ForeignKeyReferenceOnDeleteSETNULL ForeignKeyReferenceOnDelete = "SET NULL" +// Map of column names to down SQL expressions +type MultiColumnDownSQL map[string]string + +// Map of column names to up SQL expressions +type MultiColumnUpSQL map[string]string + // Add column operation type OpAddColumn struct { // Column to add @@ -128,8 +134,8 @@ type OpCreateConstraint struct { // Columns to add constraint to Columns []string `json:"columns,omitempty"` - // SQL expression of down migration by column - Down OpCreateConstraintDown `json:"down"` + // SQL expressions for down migrations + Down MultiColumnDownSQL `json:"down"` // Name of the constraint Name string `json:"name"` @@ -143,13 +149,10 @@ type OpCreateConstraint struct { // Type of the constraint Type OpCreateConstraintType `json:"type"` - // SQL expression of up migration by column - Up OpCreateConstraintUp `json:"up"` + // SQL expressions for up migrations + Up MultiColumnUpSQL `json:"up"` } -// SQL expression of down migration by column -type OpCreateConstraintDown map[string]string - // Reference to the foreign key type OpCreateConstraintReferences struct { // Columns to reference @@ -168,9 +171,6 @@ const OpCreateConstraintTypeCheck OpCreateConstraintType = "check" const OpCreateConstraintTypeForeignKey OpCreateConstraintType = "foreign_key" const OpCreateConstraintTypeUnique OpCreateConstraintType = "unique" -// SQL expression of up migration by column -type OpCreateConstraintUp map[string]string - // Create index operation type OpCreateIndex struct { // Names of columns on which to define the index @@ -249,6 +249,21 @@ type OpDropIndex struct { Name string `json:"name"` } +// Drop multi-column constraint operation +type OpDropMultiColumnConstraint struct { + // SQL expressions for down migrations + Down MultiColumnDownSQL `json:"down"` + + // Name of the constraint + Name string `json:"name"` + + // Name of the table + Table string `json:"table"` + + // SQL expressions for up migrations + Up MultiColumnUpSQL `json:"up,omitempty"` +} + // Drop table operation type OpDropTable struct { // Name of the table diff --git a/schema.json b/schema.json index 719d80a9..bd00b98d 100644 --- a/schema.json +++ b/schema.json @@ -494,19 +494,41 @@ } }, "up": { - "type": "object", - "additionalProperties": { "type": "string" }, - "description": "SQL expression of up migration by column" + "description": "SQL expressions for up migrations", + "$ref": "#/$defs/MultiColumnUpSQL" }, "down": { - "type": "object", - "additionalProperties": { "type": "string" }, - "description": "SQL expression of down migration by column" + "description": "SQL expressions for down migrations", + "$ref": "#/$defs/MultiColumnDownSQL" } }, "required": ["name", "table", "type", "up", "down"], "type": "object" }, + "OpDropMultiColumnConstraint": { + "additionalProperties": false, + "description": "Drop multi-column constraint operation", + "properties": { + "table": { + "description": "Name of the table", + "type": "string" + }, + "name": { + "description": "Name of the constraint", + "type": "string" + }, + "up": { + "description": "SQL expressions for up migrations", + "$ref": "#/$defs/MultiColumnUpSQL" + }, + "down": { + "description": "SQL expressions for down migrations", + "$ref": "#/$defs/MultiColumnDownSQL" + } + }, + "required": ["name", "table", "down"], + "type": "object" + }, "PgRollOperation": { "anyOf": [ { @@ -575,6 +597,17 @@ }, "required": ["drop_constraint"] }, + { + "type": "object", + "description": "Drop multi-column constraint operation", + "additionalProperties": false, + "properties": { + "drop_multicolumn_constraint": { + "$ref": "#/$defs/OpDropMultiColumnConstraint" + } + }, + "required": ["drop_multicolumn_constraint"] + }, { "type": "object", "description": "Rename constraint operation", @@ -702,6 +735,16 @@ }, "required": ["name"], "type": "object" + }, + "MultiColumnUpSQL": { + "type": "object", + "additionalProperties": { "type": "string" }, + "description": "Map of column names to up SQL expressions" + }, + "MultiColumnDownSQL": { + "type": "object", + "additionalProperties": { "type": "string" }, + "description": "Map of column names to down SQL expressions" } } }