Skip to content

Commit

Permalink
Update operation types to set a default of "" for up and down S…
Browse files Browse the repository at this point in the history 
…QL (#325)

Improve consistency between operation types by updating operation types
to ensure that all operations that use `up` or `down` SQL default these
fields to `""`.

There is no distinction between an empty string and `nil` for these
fields.
  • Loading branch information
@andrew-farries
andrew-farries committed Mar 26, 2024
1 parent 3e49151 commit cf66d1f
Show file tree
Hide file tree
Showing 16 changed files with 180 additions and 182 deletions.
8 changes: 4 additions & 4 deletions pkg/migrations/op_add_column.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (o *OpAddColumn) Start(ctx context.Context, conn *sql.DB, stateSchema strin
}

var tableToBackfill *schema.Table
if o.Up != nil {
if o.Up != "" {
err := createTrigger(ctx, conn, triggerConfig{
Name: TriggerName(o.Table, o.Column.Name),
Direction: TriggerDirectionUp,
Expand All @@ -50,7 +50,7 @@ func (o *OpAddColumn) Start(ctx context.Context, conn *sql.DB, stateSchema strin
TableName: o.Table,
PhysicalColumn: TemporaryName(o.Column.Name),
StateSchema: stateSchema,
SQL: *o.Up,
SQL: o.Up,
})
if err != nil {
return nil, fmt.Errorf("failed to create trigger: %w", err)
Expand Down Expand Up @@ -164,14 +164,14 @@ func (o *OpAddColumn) Validate(ctx context.Context, s *schema.Schema) error {
}

// Ensure backfill is possible
if o.Up != nil {
if o.Up != "" {
err := checkBackfill(table)
if err != nil {
return err
}
}

if !o.Column.IsNullable() && o.Column.Default == nil && o.Up == nil {
if !o.Column.IsNullable() && o.Column.Default == nil && o.Up == "" {
return FieldRequiredError{Name: "up"}
}

Expand Down
18 changes: 9 additions & 9 deletions pkg/migrations/op_add_column_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func TestAddColumn(t *testing.T) {
Nullable: ptr(false),
Unique: ptr(true),
},
Up: ptr("'this is a description'"),
Up: "'this is a description'",
},
},
},
Expand Down Expand Up @@ -310,7 +310,7 @@ func TestAddForeignKeyColumn(t *testing.T) {
},
Nullable: ptr(false),
},
Up: ptr("1"),
Up: "1",
},
},
},
Expand Down Expand Up @@ -627,7 +627,7 @@ func TestAddColumnWithUpSql(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpAddColumn{
Table: "products",
Up: ptr("UPPER(name)"),
Up: "UPPER(name)",
Column: migrations.Column{
Name: "description",
Type: "varchar(255)",
Expand Down Expand Up @@ -709,7 +709,7 @@ func TestAddColumnWithUpSql(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpAddColumn{
Table: "products",
Up: ptr("UPPER(name)"),
Up: "UPPER(name)",
Column: migrations.Column{
Name: "description",
Type: "varchar(255)",
Expand Down Expand Up @@ -798,7 +798,7 @@ func TestAddColumnWithUpSql(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpAddColumn{
Table: "products",
Up: ptr("UPPER(name)"),
Up: "UPPER(name)",
Column: migrations.Column{
Name: "description",
Type: "varchar(255)",
Expand Down Expand Up @@ -892,7 +892,7 @@ func TestAddNotNullColumnWithNoDefault(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpAddColumn{
Table: "products",
Up: ptr("UPPER(name)"),
Up: "UPPER(name)",
Column: migrations.Column{
Name: "description",
Type: "varchar(255)",
Expand Down Expand Up @@ -1075,7 +1075,7 @@ func TestAddColumnValidation(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpAddColumn{
Table: "orders",
Up: ptr("UPPER(name)"),
Up: "UPPER(name)",
Column: migrations.Column{
Name: "description",
Type: "text",
Expand Down Expand Up @@ -1123,7 +1123,7 @@ func TestAddColumnValidation(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpAddColumn{
Table: "users",
Up: ptr("UPPER(name)"),
Up: "UPPER(name)",
Column: migrations.Column{
Default: ptr("'foo'"),
Name: "description",
Expand All @@ -1144,7 +1144,7 @@ func TestAddColumnValidation(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpAddColumn{
Table: "users",
Up: ptr("UPPER(name)"),
Up: "UPPER(name)",
Column: migrations.Column{
Default: ptr("'foo'"),
Name: "description",
Expand Down
35 changes: 14 additions & 21 deletions pkg/migrations/op_alter_column.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ func (o *OpAlterColumn) Validate(ctx context.Context, s *schema.Schema) error {
// Apply any special validation rules for the inner operation
op := o.innerOperation()
if _, ok := op.(*OpRenameColumn); ok {
if o.Up != nil {
if o.Up != "" {
return NoUpSQLAllowedError{}
}
if o.Down != nil {
if o.Down != "" {
return NoDownSQLAllowedError{}
}
}
Expand All @@ -79,51 +79,51 @@ func (o *OpAlterColumn) innerOperation() Operation {
Table: o.Table,
Column: o.Column,
Type: *o.Type,
Up: ptrToStr(o.Up),
Down: ptrToStr(o.Down),
Up: o.Up,
Down: o.Down,
}

case o.Check != nil:
return &OpSetCheckConstraint{
Table: o.Table,
Column: o.Column,
Check: *o.Check,
Up: ptrToStr(o.Up),
Down: ptrToStr(o.Down),
Up: o.Up,
Down: o.Down,
}

case o.References != nil:
return &OpSetForeignKey{
Table: o.Table,
Column: o.Column,
References: *o.References,
Up: ptrToStr(o.Up),
Down: ptrToStr(o.Down),
Up: o.Up,
Down: o.Down,
}

case o.Nullable != nil && !*o.Nullable:
return &OpSetNotNull{
Table: o.Table,
Column: o.Column,
Up: ptrToStr(o.Up),
Down: ptrToStr(o.Down),
Up: o.Up,
Down: o.Down,
}

case o.Nullable != nil && *o.Nullable:
return &OpDropNotNull{
Table: o.Table,
Column: o.Column,
Up: ptrToStr(o.Up),
Down: ptrToStr(o.Down),
Up: o.Up,
Down: o.Down,
}

case o.Unique != nil:
return &OpSetUnique{
Table: o.Table,
Column: o.Column,
Name: o.Unique.Name,
Up: ptrToStr(o.Up),
Down: ptrToStr(o.Down),
Up: o.Up,
Down: o.Down,
}
}
return nil
Expand Down Expand Up @@ -155,10 +155,3 @@ func (o *OpAlterColumn) numChanges() int {

return fieldsSet
}

func ptrToStr(s *string) string {
if s == nil {
return ""
}
return *s
}
4 changes: 2 additions & 2 deletions pkg/migrations/op_alter_column_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func TestAlterColumnValidation(t *testing.T) {
Table: "posts",
Column: "title",
Name: ptr("renamed_title"),
Up: ptr("some up sql"),
Up: "some up sql",
},
},
},
Expand All @@ -113,7 +113,7 @@ func TestAlterColumnValidation(t *testing.T) {
Table: "posts",
Column: "title",
Name: ptr("renamed_title"),
Down: ptr("some down sql"),
Down: "some down sql",
},
},
},
Expand Down
36 changes: 18 additions & 18 deletions pkg/migrations/op_change_type_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ func TestChangeColumnType(t *testing.T) {
Table: "reviews",
Column: "rating",
Type: ptr("integer"),
Up: ptr("CAST (rating AS integer)"),
Down: ptr("CAST (rating AS text)"),
Up: "CAST (rating AS integer)",
Down: "CAST (rating AS text)",
},
},
},
Expand Down Expand Up @@ -207,8 +207,8 @@ func TestChangeColumnType(t *testing.T) {
Table: "employees",
Column: "department_id",
Type: ptr("bigint"),
Up: ptr("department_id"),
Down: ptr("department_id"),
Up: "department_id",
Down: "department_id",
},
},
},
Expand Down Expand Up @@ -255,8 +255,8 @@ func TestChangeColumnType(t *testing.T) {
Table: "users",
Column: "username",
Type: ptr("varchar(255)"),
Up: ptr("username"),
Down: ptr("username"),
Up: "username",
Down: "username",
},
},
},
Expand Down Expand Up @@ -323,8 +323,8 @@ func TestChangeColumnType(t *testing.T) {
Table: "users",
Column: "username",
Type: ptr("varchar(255)"),
Up: ptr("username"),
Down: ptr("username"),
Up: "username",
Down: "username",
},
},
},
Expand Down Expand Up @@ -376,8 +376,8 @@ func TestChangeColumnType(t *testing.T) {
Table: "users",
Column: "username",
Type: ptr("varchar(255)"),
Up: ptr("username"),
Down: ptr("username"),
Up: "username",
Down: "username",
},
},
},
Expand Down Expand Up @@ -426,8 +426,8 @@ func TestChangeColumnType(t *testing.T) {
Table: "users",
Column: "username",
Unique: &migrations.UniqueConstraint{Name: "unique_username"},
Up: ptr("username"),
Down: ptr("username"),
Up: "username",
Down: "username",
},
},
},
Expand All @@ -438,8 +438,8 @@ func TestChangeColumnType(t *testing.T) {
Table: "users",
Column: "username",
Type: ptr("varchar(255)"),
Up: ptr("username"),
Down: ptr("username"),
Up: "username",
Down: "username",
},
},
},
Expand Down Expand Up @@ -502,8 +502,8 @@ func TestChangeColumnType(t *testing.T) {
Table: "users",
Column: "username",
Type: ptr("varchar(255)"),
Up: ptr("username"),
Down: ptr("username"),
Up: "username",
Down: "username",
},
},
},
Expand Down Expand Up @@ -565,7 +565,7 @@ func TestChangeColumnTypeValidation(t *testing.T) {
Table: "reviews",
Column: "rating",
Type: ptr("integer"),
Down: ptr("CAST (rating AS text)"),
Down: "CAST (rating AS text)",
},
},
},
Expand All @@ -583,7 +583,7 @@ func TestChangeColumnTypeValidation(t *testing.T) {
Table: "reviews",
Column: "rating",
Type: ptr("integer"),
Up: ptr("CAST (rating AS integer)"),
Up: "CAST (rating AS integer)",
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/migrations/op_drop_column.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
var _ Operation = (*OpDropColumn)(nil)

func (o *OpDropColumn) Start(ctx context.Context, conn *sql.DB, stateSchema string, s *schema.Schema, cbs ...CallbackFn) (*schema.Table, error) {
if o.Down != nil {
if o.Down != "" {
err := createTrigger(ctx, conn, triggerConfig{
Name: TriggerName(o.Table, o.Column),
Direction: TriggerDirectionDown,
Expand All @@ -23,7 +23,7 @@ func (o *OpDropColumn) Start(ctx context.Context, conn *sql.DB, stateSchema stri
TableName: o.Table,
PhysicalColumn: o.Column,
StateSchema: stateSchema,
SQL: *o.Down,
SQL: o.Down,
})
if err != nil {
return nil, err
Expand Down
6 changes: 3 additions & 3 deletions pkg/migrations/op_drop_column_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestDropColumnWithDownSQL(t *testing.T) {
&migrations.OpDropColumn{
Table: "users",
Column: "name",
Down: ptr("UPPER(email)"),
Down: "UPPER(email)",
},
},
},
Expand Down Expand Up @@ -133,7 +133,7 @@ func TestDropColumnWithDownSQL(t *testing.T) {
&migrations.OpDropColumn{
Table: "users",
Column: "array",
Down: ptr("UPPER(email)"),
Down: "UPPER(email)",
},
},
},
Expand Down Expand Up @@ -175,7 +175,7 @@ func TestDropColumnWithDownSQL(t *testing.T) {
&migrations.OpDropColumn{
Table: "array",
Column: "name",
Down: ptr("UPPER(email)"),
Down: "UPPER(email)",
},
},
},
Expand Down
Loading

0 comments on commit cf66d1f

Please sign in to comment.