Skip to content

Commit

Permalink
Up not required when adding serial columns (#432)
Browse files Browse the repository at this point in the history
`serial` columns have an implicit default value so the `up` field is not
required.

Closes #193
  • Loading branch information
ryanslade authored Oct 25, 2024
1 parent 3c7b74b commit 2d2b4a0
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 1 deletion.
2 changes: 2 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,8 @@ An add column operation creates a new column on an existing table.

Default values are subject to the usual rules for quoting SQL expressions. In particular, string literals should be surrounded with single quotes.

**NOTE:** As a special case, the `up` field can be omitted when adding `smallserial`, `serial` and `bigserial` columns.

Example **add column** migrations:

* [03_add_column.json](../examples/03_add_column.json)
Expand Down
10 changes: 10 additions & 0 deletions pkg/migrations/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,13 @@ func (c *Column) IsPrimaryKey() bool {
}
return false
}

// HasImplicitDefault returns true if the column has an implicit default value
func (c *Column) HasImplicitDefault() bool {
switch c.Type {
case "smallserial", "serial", "bigserial":
return true
default:
return false
}
}
3 changes: 2 additions & 1 deletion pkg/migrations/op_add_column.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"

"github.com/lib/pq"

"github.com/xataio/pgroll/pkg/db"
"github.com/xataio/pgroll/pkg/schema"
)
Expand Down Expand Up @@ -171,7 +172,7 @@ func (o *OpAddColumn) Validate(ctx context.Context, s *schema.Schema) error {
}
}

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

Expand Down
103 changes: 103 additions & 0 deletions pkg/migrations/op_add_column_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,109 @@ func TestAddColumn(t *testing.T) {
},
wantStartErr: migrations.BackfillNotPossibleError{Table: "users"},
},
{
name: "add serial columns",
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: "varchar(255)",
Unique: ptr(true),
},
},
},
},
},
{
Name: "02_add_column",
Operations: migrations.Operations{
&migrations.OpAddColumn{
Table: "users",
Column: migrations.Column{
Name: "counter_smallserial",
Type: "smallserial",
Nullable: ptr(false),
},
},
&migrations.OpAddColumn{
Table: "users",
Column: migrations.Column{
Name: "counter_serial",
Type: "serial",
Nullable: ptr(false),
},
}, &migrations.OpAddColumn{
Table: "users",
Column: migrations.Column{
Name: "counter_bigserial",
Type: "bigserial",
Nullable: ptr(false),
},
},
},
},
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// old and new views of the table should exist
ViewMustExist(t, db, schema, "01_add_table", "users")
ViewMustExist(t, db, schema, "02_add_column", "users")

// inserting via both the old and the new views works
MustInsert(t, db, schema, "01_add_table", "users", map[string]string{
"name": "Alice",
})
MustInsert(t, db, schema, "02_add_column", "users", map[string]string{
"name": "Bob",
})

// selecting from both the old and the new views works
resOld := MustSelect(t, db, schema, "01_add_table", "users")
assert.Equal(t, []map[string]any{
{"id": 1, "name": "Alice"},
{"id": 2, "name": "Bob"},
}, resOld)
resNew := MustSelect(t, db, schema, "02_add_column", "users")
assert.Equal(t, []map[string]any{
{"id": 1, "name": "Alice", "counter_smallserial": 1, "counter_serial": 1, "counter_bigserial": 1},
{"id": 2, "name": "Bob", "counter_smallserial": 2, "counter_serial": 2, "counter_bigserial": 2},
}, resNew)
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
// The new column has been dropped from the underlying table
columnName := migrations.TemporaryName("age")
ColumnMustNotExist(t, db, schema, "users", columnName)

// The table's column count reflects the drop of the new column
TableMustHaveColumnCount(t, db, schema, "users", 2)
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// The new view still exists
ViewMustExist(t, db, schema, "02_add_column", "users")

// Inserting into the new view still works
MustInsert(t, db, schema, "02_add_column", "users", map[string]string{
"name": "Carl",
})

// Selecting from the new view still works
res := MustSelect(t, db, schema, "02_add_column", "users")
assert.Equal(t, []map[string]any{
{"id": 1, "name": "Alice", "counter_smallserial": 1, "counter_serial": 1, "counter_bigserial": 1},
{"id": 2, "name": "Bob", "counter_smallserial": 2, "counter_serial": 2, "counter_bigserial": 2},
{"id": 3, "name": "Carl", "counter_smallserial": 3, "counter_serial": 3, "counter_bigserial": 3},
}, res)
},
},
})
}

Expand Down

0 comments on commit 2d2b4a0

Please sign in to comment.