Skip to content

Commit 81dd0a7

Browse files
ryansladeryanslade
authored
Support dropping default for a column (#478)
We no allow setting the value of `default` to `null` when altering a column. In practice this will cause us to drop the default. It is also possible to set `default` the *value* `NULL` which has a similar effect but is recorded slightly differently in the PG catalogue. (Details [here](#450 (comment))) Closes #450
1 parent 0a282a2 commit 81dd0a7

11 files changed

+167
-21
lines changed

‎docs/README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,7 @@ A change default operation changes the default value of a column.
862862
"alter_column": {
863863
"table": "table name",
864864
"column": "column name",
865-
"default": "new default value",
865+
"default": "new default value" | null,
866866
"up": "SQL expression",
867867
"down": "SQL expression"
868868
}
@@ -872,6 +872,7 @@ A change default operation changes the default value of a column.
872872
Example **change default** migrations:
873873

874874
* [35_alter_column_multiple.json](../examples/35_alter_column_multiple.json)
875+
* [46_alter_column_drop_default](../examples/46_alter_column_drop_default.json)
875876

876877
#### Change comment
877878

@@ -884,7 +885,7 @@ A change comment operation changes the comment on a column.
884885
"alter_column": {
885886
"table": "table name",
886887
"column": "column name",
887-
"comment": "new comment for column" or null,
888+
"comment": "new comment for column" | null,
888889
"up": "SQL expression",
889890
"down": "SQL expression"
890891
}

‎examples/.ledger

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,4 @@
4343
43_create_tickets_table.json
4444
44_add_table_unique_constraint.json
4545
45_add_table_check_constraint.json
46+
46_alter_column_drop_default.json

‎examples/43_create_tickets_table.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@
1717
{
1818
"name": "sellers_zip",
1919
"type": "integer"
20+
},
21+
{
22+
"name": "ticket_type",
23+
"type": "varchar(255)",
24+
"default": "'paper'"
2025
}
2126
]
2227
}

‎examples/46_alter_column_drop_default.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"name": "46_alter_column_drop_default",
3+
"operations": [
4+
{
5+
"alter_column": {
6+
"table": "tickets",
7+
"column": "ticket_type",
8+
"default": null,
9+
"up": "ticket_type",
10+
"down": "ticket_type"
11+
}
12+
}
13+
]
14+
}

‎pkg/migrations/comment.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"fmt"
88

99
"github.com/lib/pq"
10+
1011
"github.com/xataio/pgroll/pkg/db"
1112
)
1213

‎pkg/migrations/op_alter_column.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,11 +302,17 @@ func (o *OpAlterColumn) subOperations() []Operation {
302302
Down: o.Down,
303303
})
304304
}
305-
if o.Default != nil {
305+
if o.Default.IsSpecified() {
306+
// o.Default is either a valid value or `null`.
307+
var defaultPtr *string
308+
if d, err := o.Default.Get(); err == nil {
309+
defaultPtr = &d
310+
}
311+
306312
ops = append(ops, &OpSetDefault{
307313
Table: o.Table,
308314
Column: o.Column,
309-
Default: *o.Default,
315+
Default: defaultPtr,
310316
Up: o.Up,
311317
Down: o.Down,
312318
})

‎pkg/migrations/op_alter_column_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func TestAlterColumnMultipleSubOperations(t *testing.T) {
204204
Table: "events",
205205
Column: "name",
206206
Nullable: ptr(false),
207-
Default: ptr("'default'"),
207+
Default: nullable.NewNullableWithValue("'default'"),
208208
Up: "(SELECT CASE WHEN name IS NULL THEN 'rewritten by up SQL' ELSE name END)",
209209
},
210210
},

‎pkg/migrations/op_set_default.go

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,27 +7,35 @@ import (
77
"fmt"
88

99
"github.com/lib/pq"
10+
1011
"github.com/xataio/pgroll/pkg/db"
1112
"github.com/xataio/pgroll/pkg/schema"
1213
)
1314

1415
type OpSetDefault struct {
15-
Table string `json:"table"`
16-
Column string `json:"column"`
17-
Default string `json:"default"`
18-
Up string `json:"up"`
19-
Down string `json:"down"`
16+
Table string `json:"table"`
17+
Column string `json:"column"`
18+
Default *string `json:"default"`
19+
Up string `json:"up"`
20+
Down string `json:"down"`
2021
}
2122

2223
var _ Operation = (*OpSetDefault)(nil)
2324

2425
func (o *OpSetDefault) Start(ctx context.Context, conn db.DB, latestSchema string, tr SQLTransformer, s *schema.Schema, cbs ...CallbackFn) (*schema.Table, error) {
2526
tbl := s.GetTable(o.Table)
2627

27-
_, err := conn.ExecContext(ctx, fmt.Sprintf(`ALTER TABLE %s ALTER COLUMN %s SET DEFAULT %s`,
28-
pq.QuoteIdentifier(o.Table),
29-
pq.QuoteIdentifier(TemporaryName(o.Column)),
30-
o.Default))
28+
var err error
29+
if o.Default == nil {
30+
_, err = conn.ExecContext(ctx, fmt.Sprintf(`ALTER TABLE %s ALTER COLUMN %s DROP DEFAULT`,
31+
pq.QuoteIdentifier(o.Table),
32+
pq.QuoteIdentifier(TemporaryName(o.Column))))
33+
} else {
34+
_, err = conn.ExecContext(ctx, fmt.Sprintf(`ALTER TABLE %s ALTER COLUMN %s SET DEFAULT %s`,
35+
pq.QuoteIdentifier(o.Table),
36+
pq.QuoteIdentifier(TemporaryName(o.Column)),
37+
*o.Default))
38+
}
3139
if err != nil {
3240
return nil, err
3341
}

‎pkg/migrations/op_set_default_test.go

Lines changed: 107 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import (
66
"database/sql"
77
"testing"
88

9+
"github.com/oapi-codegen/nullable"
910
"github.com/stretchr/testify/assert"
11+
1012
"github.com/xataio/pgroll/pkg/migrations"
1113
)
1214

@@ -43,7 +45,7 @@ func TestSetDefault(t *testing.T) {
4345
&migrations.OpAlterColumn{
4446
Table: "users",
4547
Column: "name",
46-
Default: ptr("'unknown user'"),
48+
Default: nullable.NewNullableWithValue("'unknown user'"),
4749
},
4850
},
4951
},
@@ -145,7 +147,7 @@ func TestSetDefault(t *testing.T) {
145147
&migrations.OpAlterColumn{
146148
Table: "users",
147149
Column: "name",
148-
Default: ptr("'unknown user'"),
150+
Default: nullable.NewNullableWithValue("'unknown user'"),
149151
Up: "'rewritten by up SQL'",
150152
Down: "'rewritten by down SQL'",
151153
},
@@ -248,7 +250,109 @@ func TestSetDefault(t *testing.T) {
248250
&migrations.OpAlterColumn{
249251
Table: "users",
250252
Column: "name",
251-
Default: ptr("NULL"),
253+
Default: nullable.NewNullableWithValue("NULL"),
254+
},
255+
},
256+
},
257+
},
258+
afterStart: func(t *testing.T, db *sql.DB, schema string) {
259+
// Inserting a row into the new schema succeeds
260+
MustInsert(t, db, schema, "02_set_default", "users", map[string]string{
261+
"id": "1",
262+
})
263+
264+
// Inserting a row into the old schema succeeds
265+
MustInsert(t, db, schema, "01_add_table", "users", map[string]string{
266+
"id": "2",
267+
})
268+
269+
// The new schema has the expected rows:
270+
// * The first row is NULL because it was inserted into the new schema which
271+
// does not have a default
272+
// * The second has a default value because it was backfilled from the old
273+
// schema which has a default
274+
rows := MustSelect(t, db, schema, "02_set_default", "users")
275+
assert.Equal(t, []map[string]any{
276+
{"id": 1, "name": nil},
277+
{"id": 2, "name": "unknown user"},
278+
}, rows)
279+
280+
// The old schema has the expected rows:
281+
// * The first row is NULL because it was backfilled from the new schema
282+
// which does not have a default
283+
// * The second row has a default value because it was inserted without
284+
// a value into the old schema which has a default
285+
rows = MustSelect(t, db, schema, "01_add_table", "users")
286+
assert.Equal(t, []map[string]any{
287+
{"id": 1, "name": nil},
288+
{"id": 2, "name": "unknown user"},
289+
}, rows)
290+
},
291+
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
292+
// Inserting a row into the old schema succeeds
293+
MustInsert(t, db, schema, "01_add_table", "users", map[string]string{
294+
"id": "3",
295+
})
296+
297+
// The old schema has the expected rows
298+
rows := MustSelect(t, db, schema, "01_add_table", "users")
299+
assert.Equal(t, []map[string]any{
300+
{"id": 1, "name": nil},
301+
{"id": 2, "name": "unknown user"},
302+
{"id": 3, "name": "unknown user"},
303+
}, rows)
304+
},
305+
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
306+
// Inserting a row into the new schema succeeds
307+
MustInsert(t, db, schema, "02_set_default", "users", map[string]string{
308+
"id": "4",
309+
})
310+
311+
// The new schema has the expected rows:
312+
// * The first row is NULL because it was inserted into the new schema which
313+
// does not have a default
314+
// * The second has a default value because it was backfilled from the old
315+
// schema which has a default
316+
rows := MustSelect(t, db, schema, "02_set_default", "users")
317+
assert.Equal(t, []map[string]any{
318+
{"id": 1, "name": nil},
319+
{"id": 2, "name": "unknown user"},
320+
{"id": 3, "name": "unknown user"},
321+
{"id": 4, "name": nil},
322+
}, rows)
323+
},
324+
},
325+
{
326+
name: "set column default: remove the default by setting it null",
327+
migrations: []migrations.Migration{
328+
{
329+
Name: "01_add_table",
330+
Operations: migrations.Operations{
331+
&migrations.OpCreateTable{
332+
Name: "users",
333+
Columns: []migrations.Column{
334+
{
335+
Name: "id",
336+
Type: "serial",
337+
Pk: ptr(true),
338+
},
339+
{
340+
Name: "name",
341+
Type: "text",
342+
Nullable: ptr(true),
343+
Default: ptr("'unknown user'"),
344+
},
345+
},
346+
},
347+
},
348+
},
349+
{
350+
Name: "02_set_default",
351+
Operations: migrations.Operations{
352+
&migrations.OpAlterColumn{
353+
Table: "users",
354+
Column: "name",
355+
Default: nullable.NewNullNullable[string](),
252356
},
253357
},
254358
},

‎pkg/migrations/types.go

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)