Skip to content

Commit

Permalink
Add a 'set comment' sub-operation to 'alter column' (#344)
Browse files Browse the repository at this point in the history
Allow 'alter column' operations to set the comment on a column:

```json
{
  "name": "02_change_comment",
  "operations": [
    {
      "alter_column": {
        "table": "events",
        "column": "name",
        "comment": "The full name of the event"
      }
    }
  ]
}
```

This is a versioned migration so the column to which the comment is
applied is duplicated and backfilled according to the `up` and `down`
SQL supplied with the 'alter column' operation. `up` and `down` default
to a simple copy of the field between new and old columns.

The intention is that this can be combined with a 'change type'
sub-operation if there is some column metadata that should be updated as
part of the type change:

```json
{
  "name": "35_alter_column_multiple",
  "operations": [
    {
      "alter_column": {
        "table": "events",
        "column": "name",
        "name": "event_name",
        "type": "text",
        "comment": "{type: some-metadata}",
        "up": "name",
        "down": "name"
      }
    }
  ]
}
```

Fixes #328
  • Loading branch information
andrew-farries authored Apr 29, 2024
1 parent afad3d8 commit 4fbfdf7
Show file tree
Hide file tree
Showing 9 changed files with 271 additions and 4 deletions.
22 changes: 22 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* [Rename column](#rename-column)
* [Change type](#change-type)
* [Change default](#change-default)
* [Change comment](#change-comment)
* [Add check constraint](#add-check-constraint)
* [Add foreign key](#add-foreign-key)
* [Add not null constraint](#add-not-null-constraint)
Expand Down Expand Up @@ -676,6 +677,7 @@ See the [examples](../examples) directory for examples of each kind of operation
* [Rename column](#rename-column)
* [Change type](#change-type)
* [Change default](#change-default)
* [Change comment](#change-comment)
* [Add check constraint](#add-check-constraint)
* [Add foreign key](#add-foreign-key)
* [Add not null constraint](#add-not-null-constraint)
Expand Down Expand Up @@ -806,6 +808,26 @@ Example **change default** migrations:

* [35_alter_column_multiple.json](../examples/35_alter_column_multiple.json)

#### Change comment

A change comment operation changes the comment on a column.

**change comment** operations have this structure:

```json
{
"alter_column": {
"table": "table name",
"column": "column name",
"comment": "new comment for column",
"up": "SQL expression",
"down": "SQL expression"
}
}
```

* [35_alter_column_multiple.json](../examples/35_alter_column_multiple.json)

#### Add check constraint

An add check constraint operation adds a `CHECK` constraint to a column.
Expand Down
3 changes: 2 additions & 1 deletion examples/34_create_events_table.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
"name": "name",
"type": "varchar(255)",
"nullable": true,
"default": "'<placeholder>'"
"default": "'<placeholder>'",
"comment": "the name of the event"
}
]
}
Expand Down
1 change: 1 addition & 0 deletions examples/35_alter_column_multiple.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"type": "text",
"default": "'unknown event'",
"nullable": false,
"comment": "the full name of the event",
"unique": {
"name": "events_event_name_unique"
},
Expand Down
13 changes: 11 additions & 2 deletions pkg/migrations/op_alter_column.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,15 @@ func (o *OpAlterColumn) subOperations() []Operation {
Down: o.Down,
})
}
if o.Comment != nil {
ops = append(ops, &OpSetComment{
Table: o.Table,
Column: o.Column,
Comment: *o.Comment,
Up: o.Up,
Down: o.Down,
})
}

return ops
}
Expand Down Expand Up @@ -324,7 +333,7 @@ func (o *OpAlterColumn) downSQLForOperations(ops []Operation) string {

for _, op := range ops {
switch (op).(type) {
case *OpSetUnique, *OpSetNotNull, *OpSetDefault:
case *OpSetUnique, *OpSetNotNull, *OpSetDefault, *OpSetComment:
return pq.QuoteIdentifier(o.Column)
}
}
Expand All @@ -341,7 +350,7 @@ func (o *OpAlterColumn) upSQLForOperations(ops []Operation) string {

for _, op := range ops {
switch (op).(type) {
case *OpDropNotNull, *OpSetDefault:
case *OpDropNotNull, *OpSetDefault, *OpSetComment:
return pq.QuoteIdentifier(o.Column)
}
}
Expand Down
9 changes: 8 additions & 1 deletion pkg/migrations/op_alter_column_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func TestAlterColumnMultipleSubOperations(t *testing.T) {

ExecuteTests(t, TestCases{
{
name: "can alter a column: set not null, change type, rename and add check constraint",
name: "can alter a column: set not null, change type, change comment, rename and add check constraint",
migrations: []migrations.Migration{
{
Name: "01_create_table",
Expand Down Expand Up @@ -48,6 +48,7 @@ func TestAlterColumnMultipleSubOperations(t *testing.T) {
Down: "name",
Name: ptr("event_name"),
Type: ptr("text"),
Comment: ptr("the name of the event"),
Nullable: ptr(false),
Check: &migrations.CheckConstraint{
Name: "event_name_length",
Expand Down Expand Up @@ -130,6 +131,9 @@ func TestAlterColumnMultipleSubOperations(t *testing.T) {

// The type of the new column in the underlying table should be `text`
ColumnMustHaveType(t, db, schema, "events", migrations.TemporaryName("name"), "text")

// The new column should have the new comment.
ColumnMustHaveComment(t, db, schema, "events", migrations.TemporaryName("name"), "the name of the event")
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
// The new (temporary) `name` column should not exist on the underlying table.
Expand All @@ -151,6 +155,9 @@ func TestAlterColumnMultipleSubOperations(t *testing.T) {
// The type of the new column in the underlying table should be `text`
ColumnMustHaveType(t, db, schema, "events", "event_name", "text")

// The new column should have the new comment.
ColumnMustHaveComment(t, db, schema, "events", "event_name", "the name of the event")

// The column in the underlying table should be `event_name`
ColumnMustExist(t, db, schema, "events", "event_name")

Expand Down
38 changes: 38 additions & 0 deletions pkg/migrations/op_set_comment.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// SPDX-License-Identifier: Apache-2.0

package migrations

import (
"context"
"database/sql"

"github.com/xataio/pgroll/pkg/schema"
)

type OpSetComment struct {
Table string `json:"table"`
Column string `json:"column"`
Comment string `json:"comment"`
Up string `json:"up"`
Down string `json:"down"`
}

var _ Operation = (*OpSetComment)(nil)

func (o *OpSetComment) Start(ctx context.Context, conn *sql.DB, stateSchema string, tr SQLTransformer, s *schema.Schema, cbs ...CallbackFn) (*schema.Table, error) {
tbl := s.GetTable(o.Table)

return tbl, addCommentToColumn(ctx, conn, o.Table, TemporaryName(o.Column), o.Comment)
}

func (o *OpSetComment) Complete(ctx context.Context, conn *sql.DB, tr SQLTransformer, s *schema.Schema) error {
return nil
}

func (o *OpSetComment) Rollback(ctx context.Context, conn *sql.DB, tr SQLTransformer) error {
return nil
}

func (o *OpSetComment) Validate(ctx context.Context, s *schema.Schema) error {
return nil
}
182 changes: 182 additions & 0 deletions pkg/migrations/op_set_comment_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
// SPDX-License-Identifier: Apache-2.0

package migrations_test

import (
"database/sql"
"testing"

"github.com/stretchr/testify/assert"
"github.com/xataio/pgroll/pkg/migrations"
)

func TestSetComment(t *testing.T) {
t.Parallel()

ExecuteTests(t, TestCases{
{
name: "set column comment with default up and down SQL",
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: "text",
Comment: ptr("apples"),
},
},
},
},
},
{
Name: "02_set_comment",
Operations: migrations.Operations{
&migrations.OpAlterColumn{
Table: "users",
Column: "name",
Comment: ptr("name of the user"),
},
},
},
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// Inserting a row into the new schema succeeds
MustInsert(t, db, schema, "02_set_comment", "users", map[string]string{
"name": "alice",
})

// Inserting a row into the old schema succeeds
MustInsert(t, db, schema, "01_add_table", "users", map[string]string{
"name": "bob",
})

// The old column should have the old comment.
ColumnMustHaveComment(t, db, schema, "users", "name", "apples")

// The new column should have the new comment.
ColumnMustHaveComment(t, db, schema, "users", migrations.TemporaryName("name"), "name of the user")

// The old schema view has the expected rows
rows := MustSelect(t, db, schema, "01_add_table", "users")
assert.Equal(t, []map[string]any{
{"id": 1, "name": "alice"},
{"id": 2, "name": "bob"},
}, rows)

// The new schema view has the expected rows
rows = MustSelect(t, db, schema, "02_set_comment", "users")
assert.Equal(t, []map[string]any{
{"id": 1, "name": "alice"},
{"id": 2, "name": "bob"},
}, rows)
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
// The column should have the old comment.
ColumnMustHaveComment(t, db, schema, "users", "name", "apples")
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// The new column should have the new comment.
ColumnMustHaveComment(t, db, schema, "users", "name", "name of the user")

// The new schema view has the expected rows
rows := MustSelect(t, db, schema, "02_set_comment", "users")
assert.Equal(t, []map[string]any{
{"id": 1, "name": "alice"},
{"id": 2, "name": "bob"},
}, rows)
},
},
{
name: "set column comment with user-supplied up and down SQL",
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: "text",
Comment: ptr("apples"),
},
},
},
},
},
{
Name: "02_set_comment",
Operations: migrations.Operations{
&migrations.OpAlterColumn{
Table: "users",
Column: "name",
Comment: ptr("name of the user"),
Up: "'rewritten by up SQL'",
Down: "'rewritten by down SQL'",
},
},
},
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// Inserting a row into the new schema succeeds
MustInsert(t, db, schema, "02_set_comment", "users", map[string]string{
"name": "alice",
})

// Inserting a row into the old schema succeeds
MustInsert(t, db, schema, "01_add_table", "users", map[string]string{
"name": "bob",
})

// The old column should have the old comment.
ColumnMustHaveComment(t, db, schema, "users", "name", "apples")

// The new column should have the new comment.
ColumnMustHaveComment(t, db, schema, "users", migrations.TemporaryName("name"), "name of the user")

// The old schema view has the expected rows
rows := MustSelect(t, db, schema, "01_add_table", "users")
assert.Equal(t, []map[string]any{
{"id": 1, "name": "rewritten by down SQL"},
{"id": 2, "name": "bob"},
}, rows)

// The new schema view has the expected rows
rows = MustSelect(t, db, schema, "02_set_comment", "users")
assert.Equal(t, []map[string]any{
{"id": 1, "name": "alice"},
{"id": 2, "name": "rewritten by up SQL"},
}, rows)
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
// The column should have the old comment.
ColumnMustHaveComment(t, db, schema, "users", "name", "apples")
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// The new column should have the new comment.
ColumnMustHaveComment(t, db, schema, "users", "name", "name of the user")

// The new schema view has the expected rows
rows := MustSelect(t, db, schema, "02_set_comment", "users")
assert.Equal(t, []map[string]any{
{"id": 1, "name": "rewritten by up SQL"},
{"id": 2, "name": "rewritten by up SQL"},
}, rows)
},
},
})
}
3 changes: 3 additions & 0 deletions pkg/migrations/types.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@
"$ref": "#/$defs/UniqueConstraint",
"description": "Add unique constraint to the column"
},
"comment": {
"description": "New comment on the column",
"type": "string"
},
"up": {
"default": "",
"description": "SQL expression for up migration",
Expand Down

0 comments on commit 4fbfdf7

Please sign in to comment.