Allow the set NOT NULL operation to take down SQL (#85)

For consistency with other operations, flexibility and the ability to
more easily combine operations into one 'alter column' operation, allow
the set `NOT NULL` operation to take user-supplied`down` SQL to be run
when moving values from the new to the old schema.

Previously, the `down` SQL was assumed to be just a straight copy of the
value from the new to the old column.
This commit is contained in:
Andrew Farries 2023-09-11 10:57:51 +01:00 committed by GitHub
parent ac84a4d859
commit fbb555b753
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 225 additions and 140 deletions

View File

@ -5,7 +5,8 @@
"set_not_null": {
"table": "reviews",
"column": "review",
"up": "(SELECT CASE WHEN review IS NULL THEN product || ' is good' ELSE review END)"
"up": "(SELECT CASE WHEN review IS NULL THEN product || ' is good' ELSE review END)",
"down": "review"
}
}
]

View File

@ -13,6 +13,7 @@ type OpSetNotNull struct {
Table string `json:"table"`
Column string `json:"column"`
Up *string `json:"up"`
Down *string `json:"down"`
}
var _ Operation = (*OpSetNotNull)(nil)
@ -51,6 +52,13 @@ func (o *OpSetNotNull) Start(ctx context.Context, conn *sql.DB, stateSchema stri
return fmt.Errorf("failed to backfill column: %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(o.Column, schema.Column{
Name: TemporaryName(o.Column),
})
// Add a trigger to copy values from the new column to the old.
err = createTrigger(ctx, conn, triggerConfig{
Name: TriggerName(o.Table, TemporaryName(o.Column)),
@ -60,16 +68,12 @@ func (o *OpSetNotNull) Start(ctx context.Context, conn *sql.DB, stateSchema stri
TableName: o.Table,
PhysicalColumn: o.Column,
StateSchema: stateSchema,
SQL: fmt.Sprintf("NEW.%s", pq.QuoteIdentifier(TemporaryName(o.Column))),
SQL: o.downSQL(),
})
if err != nil {
return fmt.Errorf("failed to create down trigger: %w", err)
}
table.AddColumn(o.Column, schema.Column{
Name: TemporaryName(o.Column),
})
return nil
}
@ -179,9 +183,18 @@ func (o *OpSetNotNull) Validate(ctx context.Context, s *schema.Schema) error {
if o.Up == nil {
return FieldRequiredError{Name: "up"}
}
return nil
}
// Down SQL is either user-specified or defaults to copying the value from the new column to the old.
func (o *OpSetNotNull) downSQL() string {
if o.Down == nil {
return fmt.Sprintf("NEW.%s", pq.QuoteIdentifier(TemporaryName(o.Column)))
}
return *o.Down
}
func duplicateColumn(ctx context.Context, conn *sql.DB, table *schema.Table, column schema.Column) error {
column.Name = TemporaryName(column.Name)

View File

@ -11,147 +11,214 @@ import (
func TestSetNotNull(t *testing.T) {
t.Parallel()
ExecuteTests(t, TestCases{{
name: "set not null",
migrations: []migrations.Migration{
{
Name: "01_add_table",
Operations: migrations.Operations{
&migrations.OpCreateTable{
Name: "reviews",
Columns: []migrations.Column{
{
Name: "id",
Type: "serial",
PrimaryKey: true,
},
{
Name: "username",
Type: "text",
Nullable: false,
},
{
Name: "product",
Type: "text",
Nullable: false,
},
{
Name: "review",
Type: "text",
Nullable: true,
ExecuteTests(t, TestCases{
{
name: "set not null with default down sql",
migrations: []migrations.Migration{
{
Name: "01_add_table",
Operations: migrations.Operations{
&migrations.OpCreateTable{
Name: "reviews",
Columns: []migrations.Column{
{
Name: "id",
Type: "serial",
PrimaryKey: true,
},
{
Name: "username",
Type: "text",
Nullable: false,
},
{
Name: "product",
Type: "text",
Nullable: false,
},
{
Name: "review",
Type: "text",
Nullable: true,
},
},
},
},
},
},
{
Name: "02_set_not_null",
Operations: migrations.Operations{
&migrations.OpSetNotNull{
Table: "reviews",
Column: "review",
Up: ptr("(SELECT CASE WHEN review IS NULL THEN product || ' is good' ELSE review END)"),
{
Name: "02_set_not_null",
Operations: migrations.Operations{
&migrations.OpSetNotNull{
Table: "reviews",
Column: "review",
Up: ptr("(SELECT CASE WHEN review IS NULL THEN product || ' is good' ELSE review END)"),
},
},
},
},
afterStart: func(t *testing.T, db *sql.DB) {
// The new (temporary) `review` column should exist on the underlying table.
ColumnMustExist(t, db, "public", "reviews", migrations.TemporaryName("review"))
// Inserting a NULL into the new `review` column should fail
MustNotInsert(t, db, "public", "02_set_not_null", "reviews", map[string]string{
"username": "alice",
"product": "apple",
})
// Inserting a non-NULL value into the new `review` column should succeed
MustInsert(t, db, "public", "02_set_not_null", "reviews", map[string]string{
"username": "alice",
"product": "apple",
"review": "amazing",
})
// The value inserted into the new `review` column has been backfilled into the
// old `review` column.
rows := MustSelect(t, db, "public", "01_add_table", "reviews")
assert.Equal(t, []map[string]any{
{"id": 2, "username": "alice", "product": "apple", "review": "amazing"},
}, rows)
// Inserting a NULL value into the old `review` column should succeed
MustInsert(t, db, "public", "01_add_table", "reviews", map[string]string{
"username": "bob",
"product": "banana",
})
// The NULL value inserted into the old `review` column has been written into
// the new `review` column using the `up` SQL.
rows = MustSelect(t, db, "public", "02_set_not_null", "reviews")
assert.Equal(t, []map[string]any{
{"id": 2, "username": "alice", "product": "apple", "review": "amazing"},
{"id": 3, "username": "bob", "product": "banana", "review": "banana is good"},
}, rows)
// Inserting a non-NULL value into the old `review` column should succeed
MustInsert(t, db, "public", "01_add_table", "reviews", map[string]string{
"username": "carl",
"product": "carrot",
"review": "crunchy",
})
// The non-NULL value inserted into the old `review` column has been copied
// unchanged into the new `review` column.
rows = MustSelect(t, db, "public", "02_set_not_null", "reviews")
assert.Equal(t, []map[string]any{
{"id": 2, "username": "alice", "product": "apple", "review": "amazing"},
{"id": 3, "username": "bob", "product": "banana", "review": "banana is good"},
{"id": 4, "username": "carl", "product": "carrot", "review": "crunchy"},
}, rows)
},
afterRollback: func(t *testing.T, db *sql.DB) {
// The new (temporary) `review` column should not exist on the underlying table.
ColumnMustNotExist(t, db, "public", "reviews", migrations.TemporaryName("review"))
// The up function no longer exists.
FunctionMustNotExist(t, db, "public", migrations.TriggerFunctionName("reviews", "review"))
// The down function no longer exists.
FunctionMustNotExist(t, db, "public", migrations.TriggerFunctionName("reviews", migrations.TemporaryName("review")))
// The up trigger no longer exists.
TriggerMustNotExist(t, db, "public", "reviews", migrations.TriggerName("reviews", "review"))
// The down trigger no longer exists.
TriggerMustNotExist(t, db, "public", "reviews", migrations.TriggerName("reviews", migrations.TemporaryName("review")))
},
afterComplete: func(t *testing.T, db *sql.DB) {
// The new (temporary) `review` column should not exist on the underlying table.
ColumnMustNotExist(t, db, "public", "reviews", migrations.TemporaryName("review"))
// Selecting from the `reviews` view should succeed.
rows := MustSelect(t, db, "public", "02_set_not_null", "reviews")
assert.Equal(t, []map[string]any{
{"id": 2, "username": "alice", "product": "apple", "review": "amazing"},
{"id": 3, "username": "bob", "product": "banana", "review": "banana is good"},
{"id": 4, "username": "carl", "product": "carrot", "review": "crunchy"},
}, rows)
// Writing NULL reviews into the `review` column should fail.
MustNotInsert(t, db, "public", "02_set_not_null", "reviews", map[string]string{
"username": "daisy",
"product": "durian",
})
// The up function no longer exists.
FunctionMustNotExist(t, db, "public", migrations.TriggerFunctionName("reviews", "review"))
// The down function no longer exists.
FunctionMustNotExist(t, db, "public", migrations.TriggerFunctionName("reviews", migrations.TemporaryName("review")))
// The up trigger no longer exists.
TriggerMustNotExist(t, db, "public", "reviews", migrations.TriggerName("reviews", "review"))
// The down trigger no longer exists.
TriggerMustNotExist(t, db, "public", "reviews", migrations.TriggerName("reviews", migrations.TemporaryName("review")))
},
},
afterStart: func(t *testing.T, db *sql.DB) {
// The new (temporary) `review` column should exist on the underlying table.
ColumnMustExist(t, db, "public", "reviews", migrations.TemporaryName("review"))
{
name: "set not null with user-supplied down sql",
migrations: []migrations.Migration{
{
Name: "01_add_table",
Operations: migrations.Operations{
&migrations.OpCreateTable{
Name: "reviews",
Columns: []migrations.Column{
{
Name: "id",
Type: "serial",
PrimaryKey: true,
},
{
Name: "username",
Type: "text",
Nullable: false,
},
{
Name: "product",
Type: "text",
Nullable: false,
},
{
Name: "review",
Type: "text",
Nullable: true,
},
},
},
},
},
{
Name: "02_set_not_null",
Operations: migrations.Operations{
&migrations.OpSetNotNull{
Table: "reviews",
Column: "review",
Up: ptr("(SELECT CASE WHEN review IS NULL THEN product || ' is good' ELSE review END)"),
Down: ptr("review || ' (from new column)'"),
},
},
},
},
afterStart: func(t *testing.T, db *sql.DB) {
// Inserting a non-NULL value into the new `review` column should succeed
MustInsert(t, db, "public", "02_set_not_null", "reviews", map[string]string{
"username": "alice",
"product": "apple",
"review": "amazing",
})
// Inserting a NULL into the new `review` column should fail
MustNotInsert(t, db, "public", "02_set_not_null", "reviews", map[string]string{
"username": "alice",
"product": "apple",
})
// Inserting a non-NULL value into the new `review` column should succeed
MustInsert(t, db, "public", "02_set_not_null", "reviews", map[string]string{
"username": "alice",
"product": "apple",
"review": "amazing",
})
// The value inserted into the new `review` column has been backfilled into the
// old `review` column.
rows := MustSelect(t, db, "public", "01_add_table", "reviews")
assert.Equal(t, []map[string]any{
{"id": 2, "username": "alice", "product": "apple", "review": "amazing"},
}, rows)
// Inserting a NULL value into the old `review` column should succeed
MustInsert(t, db, "public", "01_add_table", "reviews", map[string]string{
"username": "bob",
"product": "banana",
})
// The NULL value inserted into the old `review` column has been written into
// the new `review` column using the `up` SQL.
rows = MustSelect(t, db, "public", "02_set_not_null", "reviews")
assert.Equal(t, []map[string]any{
{"id": 2, "username": "alice", "product": "apple", "review": "amazing"},
{"id": 3, "username": "bob", "product": "banana", "review": "banana is good"},
}, rows)
// Inserting a non-NULL value into the old `review` column should succeed
MustInsert(t, db, "public", "01_add_table", "reviews", map[string]string{
"username": "carl",
"product": "carrot",
"review": "crunchy",
})
// The non-NULL value inserted into the old `review` column has been copied
// unchanged into the new `review` column.
rows = MustSelect(t, db, "public", "02_set_not_null", "reviews")
assert.Equal(t, []map[string]any{
{"id": 2, "username": "alice", "product": "apple", "review": "amazing"},
{"id": 3, "username": "bob", "product": "banana", "review": "banana is good"},
{"id": 4, "username": "carl", "product": "carrot", "review": "crunchy"},
}, rows)
// The value inserted into the new `review` column has been backfilled into the
// old `review` column using the user-supplied `down` SQL.
rows := MustSelect(t, db, "public", "01_add_table", "reviews")
assert.Equal(t, []map[string]any{
{"id": 1, "username": "alice", "product": "apple", "review": "amazing (from new column)"},
}, rows)
},
afterRollback: func(t *testing.T, db *sql.DB) {
},
afterComplete: func(t *testing.T, db *sql.DB) {
},
},
afterRollback: func(t *testing.T, db *sql.DB) {
// The new (temporary) `review` column should not exist on the underlying table.
ColumnMustNotExist(t, db, "public", "reviews", migrations.TemporaryName("review"))
// The up function no longer exists.
FunctionMustNotExist(t, db, "public", migrations.TriggerFunctionName("reviews", "review"))
// The down function no longer exists.
FunctionMustNotExist(t, db, "public", migrations.TriggerFunctionName("reviews", migrations.TemporaryName("review")))
// The up trigger no longer exists.
TriggerMustNotExist(t, db, "public", "reviews", migrations.TriggerName("reviews", "review"))
// The down trigger no longer exists.
TriggerMustNotExist(t, db, "public", "reviews", migrations.TriggerName("reviews", migrations.TemporaryName("review")))
},
afterComplete: func(t *testing.T, db *sql.DB) {
// The new (temporary) `review` column should not exist on the underlying table.
ColumnMustNotExist(t, db, "public", "reviews", migrations.TemporaryName("review"))
// Selecting from the `reviews` view should succeed.
rows := MustSelect(t, db, "public", "02_set_not_null", "reviews")
assert.Equal(t, []map[string]any{
{"id": 2, "username": "alice", "product": "apple", "review": "amazing"},
{"id": 3, "username": "bob", "product": "banana", "review": "banana is good"},
{"id": 4, "username": "carl", "product": "carrot", "review": "crunchy"},
}, rows)
// Writing NULL reviews into the `review` column should fail.
MustNotInsert(t, db, "public", "02_set_not_null", "reviews", map[string]string{
"username": "daisy",
"product": "durian",
})
// The up function no longer exists.
FunctionMustNotExist(t, db, "public", migrations.TriggerFunctionName("reviews", "review"))
// The down function no longer exists.
FunctionMustNotExist(t, db, "public", migrations.TriggerFunctionName("reviews", migrations.TemporaryName("review")))
// The up trigger no longer exists.
TriggerMustNotExist(t, db, "public", "reviews", migrations.TriggerName("reviews", "review"))
// The down trigger no longer exists.
TriggerMustNotExist(t, db, "public", "reviews", migrations.TriggerName("reviews", migrations.TemporaryName("review")))
},
}})
})
}
func TestSetNotNullValidation(t *testing.T) {
@ -199,6 +266,7 @@ func TestSetNotNullValidation(t *testing.T) {
&migrations.OpSetNotNull{
Table: "reviews",
Column: "review",
Down: ptr("review"),
},
},
},
@ -215,7 +283,8 @@ func TestSetNotNullValidation(t *testing.T) {
&migrations.OpSetNotNull{
Table: "doesntexist",
Column: "review",
Up: ptr("product || ' is good'"),
Up: ptr("(SELECT CASE WHEN review IS NULL THEN product || ' is good' ELSE review END)"),
Down: ptr("review"),
},
},
},
@ -232,7 +301,8 @@ func TestSetNotNullValidation(t *testing.T) {
&migrations.OpSetNotNull{
Table: "reviews",
Column: "doesntexist",
Up: ptr("product || ' is good'"),
Up: ptr("(SELECT CASE WHEN review IS NULL THEN product || ' is good' ELSE review END)"),
Down: ptr("review"),
},
},
},
@ -278,7 +348,8 @@ func TestSetNotNullValidation(t *testing.T) {
&migrations.OpSetNotNull{
Table: "reviews",
Column: "review",
Up: ptr("product || ' is good'"),
Up: ptr("(SELECT CASE WHEN review IS NULL THEN product || ' is good' ELSE review END)"),
Down: ptr("review"),
},
},
},