From afad3d88cdcbb1b32b730e5d4aff9b629e74b1a8 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 25 Apr 2024 11:47:44 +0100 Subject: [PATCH] Add a 'set default' sub-operation to 'alter column' (#346) Allow 'alter column' operations to set the default value on a column: ```json { "name": "02_change_default", "operations": [ { "alter_column": { "table": "events", "column": "name", "default": "'new default value'" } } ] } ``` This is a versioned migration so the column to which the default is applied is duplicated and backfilled according to the `up` and `down` SQL supplied with the 'alter column' operation. `up` and `down` default to a simple copy of the field between new and old columns. Fixes #327 --- docs/README.md | 24 ++ examples/34_create_events_table.json | 3 +- examples/35_alter_column_multiple.json | 1 + pkg/migrations/op_alter_column.go | 14 +- pkg/migrations/op_alter_column_test.go | 117 +++++++++ pkg/migrations/op_common_test.go | 6 +- pkg/migrations/op_set_default.go | 48 ++++ pkg/migrations/op_set_default_test.go | 324 +++++++++++++++++++++++++ pkg/migrations/types.go | 3 + schema.json | 4 + 10 files changed, 540 insertions(+), 4 deletions(-) create mode 100644 pkg/migrations/op_set_default.go create mode 100644 pkg/migrations/op_set_default_test.go diff --git a/docs/README.md b/docs/README.md index 8658604..87455bf 100644 --- a/docs/README.md +++ b/docs/README.md @@ -18,6 +18,7 @@ * [Alter column](#alter-column) * [Rename column](#rename-column) * [Change type](#change-type) + * [Change default](#change-default) * [Add check constraint](#add-check-constraint) * [Add foreign key](#add-foreign-key) * [Add not null constraint](#add-not-null-constraint) @@ -674,6 +675,7 @@ See the [examples](../examples) directory for examples of each kind of operation * [Alter column](#alter-column) * [Rename column](#rename-column) * [Change type](#change-type) + * [Change default](#change-default) * [Add check constraint](#add-check-constraint) * [Add foreign key](#add-foreign-key) * [Add not null constraint](#add-not-null-constraint) @@ -782,6 +784,28 @@ Example **change type** migrations: * [18_change_column_type.json](../examples/18_change_column_type.json) +#### Change default + +A change default operation changes the default value of a column. + +**change default** operations have this structure: + +```json +{ + "alter_column": { + "table": "table name", + "column": "column name", + "default": "new default value", + "up": "SQL expression", + "down": "SQL expression" + } +} +``` + +Example **change default** migrations: + +* [35_alter_column_multiple.json](../examples/35_alter_column_multiple.json) + #### Add check constraint An add check constraint operation adds a `CHECK` constraint to a column. diff --git a/examples/34_create_events_table.json b/examples/34_create_events_table.json index 232995e..5cc9dd5 100644 --- a/examples/34_create_events_table.json +++ b/examples/34_create_events_table.json @@ -13,7 +13,8 @@ { "name": "name", "type": "varchar(255)", - "nullable": true + "nullable": true, + "default": "''" } ] } diff --git a/examples/35_alter_column_multiple.json b/examples/35_alter_column_multiple.json index 89c59eb..f8cb3c4 100644 --- a/examples/35_alter_column_multiple.json +++ b/examples/35_alter_column_multiple.json @@ -7,6 +7,7 @@ "column": "name", "name": "event_name", "type": "text", + "default": "'unknown event'", "nullable": false, "unique": { "name": "events_event_name_unique" diff --git a/pkg/migrations/op_alter_column.go b/pkg/migrations/op_alter_column.go index eb3a137..30e022b 100644 --- a/pkg/migrations/op_alter_column.go +++ b/pkg/migrations/op_alter_column.go @@ -287,6 +287,15 @@ func (o *OpAlterColumn) subOperations() []Operation { Down: o.Down, }) } + if o.Default != nil { + ops = append(ops, &OpSetDefault{ + Table: o.Table, + Column: o.Column, + Default: *o.Default, + Up: o.Up, + Down: o.Down, + }) + } return ops } @@ -315,7 +324,7 @@ func (o *OpAlterColumn) downSQLForOperations(ops []Operation) string { for _, op := range ops { switch (op).(type) { - case *OpSetUnique, *OpSetNotNull: + case *OpSetUnique, *OpSetNotNull, *OpSetDefault: return pq.QuoteIdentifier(o.Column) } } @@ -331,7 +340,8 @@ func (o *OpAlterColumn) upSQLForOperations(ops []Operation) string { } for _, op := range ops { - if _, ok := op.(*OpDropNotNull); ok { + switch (op).(type) { + case *OpDropNotNull, *OpSetDefault: return pq.QuoteIdentifier(o.Column) } } diff --git a/pkg/migrations/op_alter_column_test.go b/pkg/migrations/op_alter_column_test.go index 330fdd2..1b4492f 100644 --- a/pkg/migrations/op_alter_column_test.go +++ b/pkg/migrations/op_alter_column_test.go @@ -164,6 +164,123 @@ func TestAlterColumnMultipleSubOperations(t *testing.T) { }, rows) }, }, + { + name: "can alter a column: set not null, and add a default", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "events", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "varchar(255)", + Nullable: ptr(true), + }, + }, + }, + }, + }, + { + Name: "02_alter_column", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "events", + Column: "name", + Nullable: ptr(false), + Default: ptr("'default'"), + Up: "(SELECT CASE WHEN name IS NULL THEN 'rewritten by up SQL' ELSE name END)", + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Inserting with no `name` into the new schema should succeed + MustInsert(t, db, schema, "02_alter_column", "events", map[string]string{ + "id": "1", + }) + + // Inserting a NULL explicitly into the new schema should fail + MustNotInsert(t, db, schema, "02_alter_column", "events", map[string]string{ + "id": "100", + "name": "NULL", + }, testutils.CheckViolationErrorCode) + + // Inserting with no `name` into the old schema should succeed + MustInsert(t, db, schema, "01_create_table", "events", map[string]string{ + "id": "2", + }) + + // The new schema has the expected rows: + // * The first row has a default value because it was inserted with no + // value into the new schema which has a default + // * The second row was rewritten by the `up` SQL because it was + // inserted with no value into the old schema which has no default + rows := MustSelect(t, db, schema, "02_alter_column", "events") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "default"}, + {"id": 2, "name": "rewritten by up SQL"}, + }, rows) + + // The old schema has the expected rows: + // * The first row has a default value because it was inserted with no + // value into the new schema which has a default and then backfilled + // to the old schema + // * The second row is NULL because it was inserted with no value into + // the old schema which has no default + rows = MustSelect(t, db, schema, "01_create_table", "events") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "default"}, + {"id": 2, "name": nil}, + }, rows) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // Inserting with no `name` into the old schema should succeed + MustInsert(t, db, schema, "01_create_table", "events", map[string]string{ + "id": "3", + }) + + // The old schema has the expected rows + rows := MustSelect(t, db, schema, "01_create_table", "events") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "default"}, + {"id": 2, "name": nil}, + {"id": 3, "name": nil}, + }, rows) + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Inserting with no `name` into the new schema should succeed + MustInsert(t, db, schema, "02_alter_column", "events", map[string]string{ + "id": "4", + }) + + // Inserting a NULL explicitly into the new schema should fail + MustNotInsert(t, db, schema, "02_alter_column", "events", map[string]string{ + "id": "100", + "name": "NULL", + }, testutils.NotNullViolationErrorCode) + + // The new schema has the expected rows: + // * The first and fourth rows have default values because they were + // inserted with no value into the new schema which has a default + // * The second and third rows were rewritten by the `up` SQL because + // they were inserted with no value into the old schema which has no + // default + rows := MustSelect(t, db, schema, "02_alter_column", "events") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "default"}, + {"id": 2, "name": "rewritten by up SQL"}, + {"id": 3, "name": "rewritten by up SQL"}, + {"id": 4, "name": "default"}, + }, rows) + }, + }, { name: "can alter a column: rename and add a unique constraint", migrations: []migrations.Migration{ diff --git a/pkg/migrations/op_common_test.go b/pkg/migrations/op_common_test.go index 6e46f5b..76fe0c5 100644 --- a/pkg/migrations/op_common_test.go +++ b/pkg/migrations/op_common_test.go @@ -578,7 +578,11 @@ func insert(t *testing.T, db *sql.DB, schema, version, table string, record map[ if i > 0 { recordStr += ", " } - recordStr += fmt.Sprintf("'%s'", record[c]) + if record[c] == "NULL" { + recordStr += record[c] + } else { + recordStr += fmt.Sprintf("'%s'", record[c]) + } } recordStr += ")" diff --git a/pkg/migrations/op_set_default.go b/pkg/migrations/op_set_default.go new file mode 100644 index 0000000..6da2a04 --- /dev/null +++ b/pkg/migrations/op_set_default.go @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: Apache-2.0 + +package migrations + +import ( + "context" + "database/sql" + "fmt" + + "github.com/lib/pq" + "github.com/xataio/pgroll/pkg/schema" +) + +type OpSetDefault struct { + Table string `json:"table"` + Column string `json:"column"` + Default string `json:"default"` + Up string `json:"up"` + Down string `json:"down"` +} + +var _ Operation = (*OpSetDefault)(nil) + +func (o *OpSetDefault) Start(ctx context.Context, conn *sql.DB, stateSchema string, tr SQLTransformer, s *schema.Schema, cbs ...CallbackFn) (*schema.Table, error) { + tbl := s.GetTable(o.Table) + + _, err := conn.ExecContext(ctx, fmt.Sprintf(`ALTER TABLE %s ALTER COLUMN %s SET DEFAULT %s`, + pq.QuoteIdentifier(o.Table), + pq.QuoteIdentifier(TemporaryName(o.Column)), + o.Default)) + if err != nil { + return nil, err + } + + return tbl, nil +} + +func (o *OpSetDefault) Complete(ctx context.Context, conn *sql.DB, tr SQLTransformer, s *schema.Schema) error { + return nil +} + +func (o *OpSetDefault) Rollback(ctx context.Context, conn *sql.DB, tr SQLTransformer) error { + return nil +} + +func (o *OpSetDefault) Validate(ctx context.Context, s *schema.Schema) error { + return nil +} diff --git a/pkg/migrations/op_set_default_test.go b/pkg/migrations/op_set_default_test.go new file mode 100644 index 0000000..45479de --- /dev/null +++ b/pkg/migrations/op_set_default_test.go @@ -0,0 +1,324 @@ +// SPDX-License-Identifier: Apache-2.0 + +package migrations_test + +import ( + "database/sql" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/xataio/pgroll/pkg/migrations" +) + +func TestSetDefault(t *testing.T) { + t.Parallel() + + ExecuteTests(t, TestCases{ + { + name: "set column default with default up and down SQL", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "text", + Nullable: ptr(true), + }, + }, + }, + }, + }, + { + Name: "02_set_default", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "users", + Column: "name", + Default: ptr("'unknown user'"), + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row into the new schema succeeds + MustInsert(t, db, schema, "02_set_default", "users", map[string]string{ + "id": "1", + }) + + // Inserting a row into the old schema succeeds + MustInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "id": "2", + }) + + // The new schema has the expected rows: + // * The first row has a default value because it was inserted without + // a value into the new schema which has a default + // * The second is NULL because it was backfilled from the old schema + // which does not have a default + rows := MustSelect(t, db, schema, "02_set_default", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "unknown user"}, + {"id": 2, "name": nil}, + }, rows) + + // The old schema has the expected rows: + // * The first row has a default value because it was backfilled from the + // new schema which has a default + // * The second row is NULL because it was inserted without a value + // into the old schema which does not have a default + rows = MustSelect(t, db, schema, "01_add_table", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "unknown user"}, + {"id": 2, "name": nil}, + }, rows) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row into the old schema succeeds + MustInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "id": "3", + }) + + // The old schema has the expected rows + rows := MustSelect(t, db, schema, "01_add_table", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "unknown user"}, + {"id": 2, "name": nil}, + {"id": 3, "name": nil}, + }, rows) + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row into the new schema succeeds + MustInsert(t, db, schema, "02_set_default", "users", map[string]string{ + "id": "4", + }) + + // The new schema has the expected rows: + // The new schema has the expected rows: + // * The first and fourth rows have default values because they were + // inserted without values into the new schema which has a default + // * The second and third are NULL because they were backfilled from + // the old schema which does not have a default + rows := MustSelect(t, db, schema, "02_set_default", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "unknown user"}, + {"id": 2, "name": nil}, + {"id": 3, "name": nil}, + {"id": 4, "name": "unknown user"}, + }, rows) + }, + }, + { + name: "set column default with user-supplied up and down SQL", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "text", + Nullable: ptr(true), + }, + }, + }, + }, + }, + { + Name: "02_set_default", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "users", + Column: "name", + Default: ptr("'unknown user'"), + Up: "'rewritten by up SQL'", + Down: "'rewritten by down SQL'", + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row into the new schema succeeds + MustInsert(t, db, schema, "02_set_default", "users", map[string]string{ + "id": "1", + }) + + // Inserting a row into the old schema succeeds + MustInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "id": "2", + }) + + // The new schema has the expected rows: + // * The first row has a default value because it was inserted without + // a value into the new schema which has a default + // * The second was rewritten because it was backfilled from the old schema + rows := MustSelect(t, db, schema, "02_set_default", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "unknown user"}, + {"id": 2, "name": "rewritten by up SQL"}, + }, rows) + + // The old schema has the expected rows: + // * The first row has a rewritten value because it was backfilled from the + // new schema. + // * The second row is NULL because it was inserted without a value + // into the old schema which does not have a default + rows = MustSelect(t, db, schema, "01_add_table", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "rewritten by down SQL"}, + {"id": 2, "name": nil}, + }, rows) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row into the old schema succeeds + MustInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "id": "3", + }) + + // The old schema has the expected rows + rows := MustSelect(t, db, schema, "01_add_table", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "rewritten by down SQL"}, + {"id": 2, "name": nil}, + {"id": 3, "name": nil}, + }, rows) + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row into the new schema succeeds + MustInsert(t, db, schema, "02_set_default", "users", map[string]string{ + "id": "4", + }) + + // The new schema has the expected rows: + // * The first three rows have rewritten values because they were + // backfilled from the old schema + // * The fourth row has a default value because it was inserted into + // the new schema + rows := MustSelect(t, db, schema, "02_set_default", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "rewritten by up SQL"}, + {"id": 2, "name": "rewritten by up SQL"}, + {"id": 3, "name": "rewritten by up SQL"}, + {"id": 4, "name": "unknown user"}, + }, rows) + }, + }, + { + name: "set column default: remove the default", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "text", + Nullable: ptr(true), + Default: ptr("'unknown user'"), + }, + }, + }, + }, + }, + { + Name: "02_set_default", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "users", + Column: "name", + Default: ptr("NULL"), + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row into the new schema succeeds + MustInsert(t, db, schema, "02_set_default", "users", map[string]string{ + "id": "1", + }) + + // Inserting a row into the old schema succeeds + MustInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "id": "2", + }) + + // The new schema has the expected rows: + // * The first row is NULL because it was inserted into the new schema which + // does not have a default + // * The second has a default value because it was backfilled from the old + // schema which has a default + rows := MustSelect(t, db, schema, "02_set_default", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": nil}, + {"id": 2, "name": "unknown user"}, + }, rows) + + // The old schema has the expected rows: + // * The first row is NULL because it was backfilled from the new schema + // which does not have a default + // * The second row has a default value because it was inserted without + // a value into the old schema which has a default + rows = MustSelect(t, db, schema, "01_add_table", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": nil}, + {"id": 2, "name": "unknown user"}, + }, rows) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row into the old schema succeeds + MustInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "id": "3", + }) + + // The old schema has the expected rows + rows := MustSelect(t, db, schema, "01_add_table", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": nil}, + {"id": 2, "name": "unknown user"}, + {"id": 3, "name": "unknown user"}, + }, rows) + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Inserting a row into the new schema succeeds + MustInsert(t, db, schema, "02_set_default", "users", map[string]string{ + "id": "4", + }) + + // The new schema has the expected rows: + // * The first row is NULL because it was inserted into the new schema which + // does not have a default + // * The second has a default value because it was backfilled from the old + // schema which has a default + rows := MustSelect(t, db, schema, "02_set_default", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": nil}, + {"id": 2, "name": "unknown user"}, + {"id": 3, "name": "unknown user"}, + {"id": 4, "name": nil}, + }, rows) + }, + }, + }) +} diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index e6df431..f100ebc 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -85,6 +85,9 @@ type OpAlterColumn struct { // Name of the column Column string `json:"column"` + // Default value of the column + Default *string `json:"default,omitempty"` + // SQL expression for down migration Down string `json:"down,omitempty"` diff --git a/schema.json b/schema.json index f462372..2ec5c70 100644 --- a/schema.json +++ b/schema.json @@ -133,6 +133,10 @@ "description": "New name of the column (for rename column operation)", "type": "string" }, + "default": { + "description": "Default value of the column", + "type": "string" + }, "nullable": { "description": "Indicates if the column is nullable (for add/remove not null constraint operation)", "type": "boolean"