Skip to content

Commit

Permalink
Update schema.json to correctly describe the 'alter column' operati…
Browse files Browse the repository at this point in the history 
…on (#261)

Update `schema.json` to correctly describe the 'alter column' operation.

The [alter column
](https://github.com/xataio/pgroll/tree/main/docs#alter-column)
operation is used to perform different alter column operations according
to which fields are present.

The rules are:
* `table` and `column` are always required.
* Exactly one of `name`, `type`, `check`, `unique`, `references`,
`nullable` must be present.
* If `type`, `check`, `unique`, `references` or `nullable` is present,
then `up` and `down` must also be present.
* If `name` is present then neither `up` or `down` is allowed.

This PR updates `schema.json` to capture these dependencies.

This change causes churn in the test code as some fields that previously
`string` have become `*string`.

Fixes #222
  • Loading branch information
@andrew-farries
andrew-farries committed Feb 2, 2024
1 parent bb79eec commit 5a6a83a
Show file tree
Hide file tree
Showing 12 changed files with 177 additions and 141 deletions.
47 changes: 27 additions & 20 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 != "" {
if o.Up != nil {
return NoUpSQLAllowedError{}
}
if o.Down != "" {
if o.Down != nil {
return NoDownSQLAllowedError{}
}
}
Expand All @@ -67,63 +67,63 @@ func (o *OpAlterColumn) Validate(ctx context.Context, s *schema.Schema) error {

func (o *OpAlterColumn) innerOperation() Operation {
switch {
case o.Name != "":
case o.Name != nil:
return &OpRenameColumn{
Table: o.Table,
From: o.Column,
To: o.Name,
To: *o.Name,
}

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

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

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

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

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

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

if o.Name != "" {
if o.Name != nil {
fieldsSet++
}
if o.Type != "" {
if o.Type != nil {
fieldsSet++
}
if o.Check != nil {
Expand All @@ -155,3 +155,10 @@ func (o *OpAlterColumn) numChanges() int {

return fieldsSet
}

func ptrToStr(s *string) string {
if s == nil {
return ""
}
return *s
}
18 changes: 9 additions & 9 deletions pkg/migrations/op_alter_column_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestAlterColumnValidation(t *testing.T) {
&migrations.OpAlterColumn{
Table: "doesntexist",
Column: "title",
Name: "renamed_title",
Name: ptr("renamed_title"),
},
},
},
Expand All @@ -77,7 +77,7 @@ func TestAlterColumnValidation(t *testing.T) {
&migrations.OpAlterColumn{
Table: "posts",
Column: "doesntexist",
Name: "renamed_title",
Name: ptr("renamed_title"),
},
},
},
Expand All @@ -94,8 +94,8 @@ func TestAlterColumnValidation(t *testing.T) {
&migrations.OpAlterColumn{
Table: "posts",
Column: "title",
Name: "renamed_title",
Up: "some up sql",
Name: ptr("renamed_title"),
Up: ptr("some up sql"),
},
},
},
Expand All @@ -112,8 +112,8 @@ func TestAlterColumnValidation(t *testing.T) {
&migrations.OpAlterColumn{
Table: "posts",
Column: "title",
Name: "renamed_title",
Down: "some down sql",
Name: ptr("renamed_title"),
Down: ptr("some down sql"),
},
},
},
Expand Down Expand Up @@ -146,8 +146,8 @@ func TestAlterColumnValidation(t *testing.T) {
&migrations.OpAlterColumn{
Table: "posts",
Column: "title",
Name: "renamed_title",
Type: "varchar(255)",
Name: ptr("renamed_title"),
Type: ptr("varchar(255)"),
},
},
},
Expand All @@ -172,7 +172,7 @@ func TestAlterColumnValidation(t *testing.T) {
&migrations.OpAlterColumn{
Table: "orders",
Column: "quantity",
Name: "renamed_quantity",
Name: ptr("renamed_quantity"),
},
},
},
Expand Down
44 changes: 22 additions & 22 deletions pkg/migrations/op_change_type_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ func TestChangeColumnType(t *testing.T) {
&migrations.OpAlterColumn{
Table: "reviews",
Column: "rating",
Type: "integer",
Up: "CAST (rating AS integer)",
Down: "CAST (rating AS text)",
Type: ptr("integer"),
Up: ptr("CAST (rating AS integer)"),
Down: ptr("CAST (rating AS text)"),
},
},
},
Expand Down Expand Up @@ -205,9 +205,9 @@ func TestChangeColumnType(t *testing.T) {
&migrations.OpAlterColumn{
Table: "employees",
Column: "department_id",
Type: "bigint",
Up: "department_id",
Down: "department_id",
Type: ptr("bigint"),
Up: ptr("department_id"),
Down: ptr("department_id"),
},
},
},
Expand Down Expand Up @@ -253,9 +253,9 @@ func TestChangeColumnType(t *testing.T) {
&migrations.OpAlterColumn{
Table: "users",
Column: "username",
Type: "varchar(255)",
Up: "username",
Down: "username",
Type: ptr("varchar(255)"),
Up: ptr("username"),
Down: ptr("username"),
},
},
},
Expand Down Expand Up @@ -321,9 +321,9 @@ func TestChangeColumnType(t *testing.T) {
&migrations.OpAlterColumn{
Table: "users",
Column: "username",
Type: "varchar(255)",
Up: "username",
Down: "username",
Type: ptr("varchar(255)"),
Up: ptr("username"),
Down: ptr("username"),
},
},
},
Expand Down Expand Up @@ -374,9 +374,9 @@ func TestChangeColumnType(t *testing.T) {
&migrations.OpAlterColumn{
Table: "users",
Column: "username",
Type: "varchar(255)",
Up: "username",
Down: "username",
Type: ptr("varchar(255)"),
Up: ptr("username"),
Down: ptr("username"),
},
},
},
Expand Down Expand Up @@ -425,9 +425,9 @@ func TestChangeColumnType(t *testing.T) {
&migrations.OpAlterColumn{
Table: "users",
Column: "username",
Type: "varchar(255)",
Up: "username",
Down: "username",
Type: ptr("varchar(255)"),
Up: ptr("username"),
Down: ptr("username"),
},
},
},
Expand Down Expand Up @@ -502,8 +502,8 @@ func TestChangeColumnTypeValidation(t *testing.T) {
&migrations.OpAlterColumn{
Table: "reviews",
Column: "rating",
Type: "integer",
Down: "CAST (rating AS text)",
Type: ptr("integer"),
Down: ptr("CAST (rating AS text)"),
},
},
},
Expand All @@ -520,8 +520,8 @@ func TestChangeColumnTypeValidation(t *testing.T) {
&migrations.OpAlterColumn{
Table: "reviews",
Column: "rating",
Type: "integer",
Up: "CAST (rating AS integer)",
Type: ptr("integer"),
Up: ptr("CAST (rating AS integer)"),
},
},
},
Expand Down
24 changes: 12 additions & 12 deletions pkg/migrations/op_drop_constraint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ func TestDropConstraint(t *testing.T) {
Name: "check_title_length",
Constraint: "length(title) > 3",
},
Up: "(SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END)",
Down: "title",
Up: ptr("(SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END)"),
Down: ptr("title"),
},
},
},
Expand Down Expand Up @@ -176,8 +176,8 @@ func TestDropConstraint(t *testing.T) {
Name: "check_title_length",
Constraint: "length(title) > 3",
},
Up: "(SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END)",
Down: "title",
Up: ptr("(SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END)"),
Down: ptr("title"),
},
},
},
Expand Down Expand Up @@ -262,8 +262,8 @@ func TestDropConstraint(t *testing.T) {
Table: "users",
Column: "id",
},
Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)",
Down: "user_id",
Up: ptr("(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)"),
Down: ptr("user_id"),
},
},
},
Expand Down Expand Up @@ -637,8 +637,8 @@ func TestDropConstraint(t *testing.T) {
Name: "check_title_length",
Constraint: "length(title) > 3",
},
Up: "(SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END)",
Down: "title",
Up: ptr("(SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END)"),
Down: ptr("title"),
},
},
},
Expand Down Expand Up @@ -706,8 +706,8 @@ func TestDropConstraint(t *testing.T) {
Name: "check_title_length",
Constraint: "length(title) > 3",
},
Up: "(SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END)",
Down: "title",
Up: ptr("(SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END)"),
Down: ptr("title"),
},
},
},
Expand Down Expand Up @@ -836,8 +836,8 @@ func TestDropConstraintValidation(t *testing.T) {
Name: "check_title_length",
Constraint: "length(title) > 3",
},
Up: "(SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END)",
Down: "title",
Up: ptr("(SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END)"),
Down: ptr("title"),
},
},
}
Expand Down
Loading

0 comments on commit 5a6a83a

Please sign in to comment.