Add 'alter column' operation to combine some existing operations (#91)

Introduce a new `alter_column` operation to combine existing operations
that work on columns.

The following operation types should be combined into one:

- [x] Change column type
- [x] Rename column
- [ ] Add `CHECK` constraint
- [ ] Add foreign key constraint
- [ ] Make column `NOT NULL`
- [ ] Add unique constraint

This PR implements the first two operations in the list, leaving the
rest for a later PR.

The new `alter_column` migrations look like:

```json
{
  "name": "18_change_column_type",
  "operations": [
    {
      "alter_column": {
        "table": "reviews",
        "column": "rating",
        "type": "integer",
        "up": "CAST(rating AS integer)",
        "down": "CAST(rating AS text)"
      }
    }
  ]
}

```
and

```json
{
  "name": "13_rename_column",
  "operations": [
    {
      "alter_column": {
        "table": "employees",
        "column": "role",
        "name": "job_title"
      }
    }
  ]
}
```
This commit is contained in:
Andrew Farries 2023-09-12 12:10:13 +01:00 committed by GitHub
parent e0f301917f
commit c8218e9de5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 261 additions and 31 deletions

View File

@ -2,10 +2,10 @@
"name": "13_rename_column",
"operations": [
{
"rename_column": {
"alter_column": {
"table": "employees",
"from": "role",
"to": "jobTitle"
"column": "role",
"name": "job_title"
}
}
]

View File

@ -2,7 +2,7 @@
"name": "18_change_column_type",
"operations": [
{
"change_type": {
"alter_column": {
"table": "reviews",
"column": "rating",
"type": "integer",

View File

@ -99,3 +99,21 @@ func (e ColumnReferenceError) Error() string {
e.Table,
e.Err.Error())
}
type NoUpSQLAllowedError struct{}
func (e NoUpSQLAllowedError) Error() string {
return "up SQL is not allowed for this operation"
}
type NoDownSQLAllowedError struct{}
func (e NoDownSQLAllowedError) Error() string {
return "down SQL is not allowed for this operation"
}
type MultipleAlterColumnChangesError struct{}
func (e MultipleAlterColumnChangesError) Error() string {
return "alter column operations require exactly one change"
}

View File

@ -0,0 +1,93 @@
package migrations
import (
"context"
"database/sql"
"github.com/xataio/pg-roll/pkg/schema"
)
type OpAlterColumn struct {
Table string `json:"table"`
Column string `json:"column"`
Name string `json:"name"`
Type string `json:"type"`
Up string `json:"up"`
Down string `json:"down"`
}
var _ Operation = (*OpAlterColumn)(nil)
func (o *OpAlterColumn) Start(ctx context.Context, conn *sql.DB, stateSchema string, s *schema.Schema) error {
op := o.innerOperation()
return op.Start(ctx, conn, stateSchema, s)
}
func (o *OpAlterColumn) Complete(ctx context.Context, conn *sql.DB) error {
op := o.innerOperation()
return op.Complete(ctx, conn)
}
func (o *OpAlterColumn) Rollback(ctx context.Context, conn *sql.DB) error {
op := o.innerOperation()
return op.Rollback(ctx, conn)
}
func (o *OpAlterColumn) Validate(ctx context.Context, s *schema.Schema) error {
op := o.innerOperation()
if !o.oneChange() {
return MultipleAlterColumnChangesError{}
}
if _, ok := op.(*OpRenameColumn); ok {
if o.Up != "" {
return NoUpSQLAllowedError{}
}
if o.Down != "" {
return NoDownSQLAllowedError{}
}
}
return op.Validate(ctx, s)
}
func (o *OpAlterColumn) innerOperation() Operation {
switch {
case o.Name != "":
return &OpRenameColumn{
Table: o.Table,
From: o.Column,
To: o.Name,
}
case o.Type != "":
return &OpChangeType{
Table: o.Table,
Column: o.Column,
Type: o.Type,
Up: o.Up,
Down: o.Down,
}
}
return nil
}
// oneChange ensures that the 'alter column' operation attempts to make
// only one kind of change. For example, it should not attempt to rename a
// column and change its type at the same time.
func (o *OpAlterColumn) oneChange() bool {
fieldsSet := 0
if o.Name != "" {
fieldsSet++
}
if o.Type != "" {
fieldsSet++
}
return fieldsSet == 1
}

View File

@ -0,0 +1,122 @@
package migrations_test
import (
"testing"
"github.com/xataio/pg-roll/pkg/migrations"
)
func TestAlterColumnValidation(t *testing.T) {
t.Parallel()
createTablesMigration := migrations.Migration{
Name: "01_add_tables",
Operations: migrations.Operations{
&migrations.OpCreateTable{
Name: "users",
Columns: []migrations.Column{
{
Name: "id",
Type: "serial",
PrimaryKey: true,
},
{
Name: "name",
Type: "text",
},
},
},
&migrations.OpCreateTable{
Name: "posts",
Columns: []migrations.Column{
{
Name: "id",
Type: "serial",
PrimaryKey: true,
},
{
Name: "title",
Type: "text",
},
{
Name: "user_id",
Type: "integer",
},
},
},
},
}
ExecuteTests(t, TestCases{
{
name: "column rename: no up SQL allowed",
migrations: []migrations.Migration{
createTablesMigration,
{
Name: "01_alter_column",
Operations: migrations.Operations{
&migrations.OpAlterColumn{
Table: "posts",
Column: "title",
Name: "renamed_title",
Up: "some up sql",
},
},
},
},
wantStartErr: migrations.NoUpSQLAllowedError{},
},
{
name: "column rename: no down SQL allowed",
migrations: []migrations.Migration{
createTablesMigration,
{
Name: "01_alter_column",
Operations: migrations.Operations{
&migrations.OpAlterColumn{
Table: "posts",
Column: "title",
Name: "renamed_title",
Down: "some down sql",
},
},
},
},
wantStartErr: migrations.NoDownSQLAllowedError{},
},
{
name: "cant make no changes",
migrations: []migrations.Migration{
createTablesMigration,
{
Name: "01_alter_column",
Operations: migrations.Operations{
&migrations.OpAlterColumn{
Table: "posts",
Column: "title",
},
},
},
},
wantStartErr: migrations.MultipleAlterColumnChangesError{},
},
{
name: "only one change at at time",
migrations: []migrations.Migration{
createTablesMigration,
{
Name: "01_alter_column",
Operations: migrations.Operations{
&migrations.OpAlterColumn{
Table: "posts",
Column: "title",
Name: "renamed_title",
Type: "varchar(255)",
},
},
},
},
wantStartErr: migrations.MultipleAlterColumnChangesError{},
},
})
}

View File

@ -46,7 +46,7 @@ func TestChangeColumnType(t *testing.T) {
{
Name: "02_change_type",
Operations: migrations.Operations{
&migrations.OpChangeType{
&migrations.OpAlterColumn{
Table: "reviews",
Column: "rating",
Type: "integer",
@ -184,7 +184,7 @@ func TestChangeColumnTypeValidation(t *testing.T) {
{
Name: "02_change_type",
Operations: migrations.Operations{
&migrations.OpChangeType{
&migrations.OpAlterColumn{
Table: "reviews",
Column: "rating",
Type: "integer",
@ -202,7 +202,7 @@ func TestChangeColumnTypeValidation(t *testing.T) {
{
Name: "02_change_type",
Operations: migrations.Operations{
&migrations.OpChangeType{
&migrations.OpAlterColumn{
Table: "reviews",
Column: "rating",
Type: "integer",
@ -220,7 +220,7 @@ func TestChangeColumnTypeValidation(t *testing.T) {
{
Name: "02_change_type",
Operations: migrations.Operations{
&migrations.OpChangeType{
&migrations.OpAlterColumn{
Table: "doesntexist",
Column: "rating",
Type: "integer",
@ -239,7 +239,7 @@ func TestChangeColumnTypeValidation(t *testing.T) {
{
Name: "02_change_type",
Operations: migrations.Operations{
&migrations.OpChangeType{
&migrations.OpAlterColumn{
Table: "reviews",
Column: "doesntexist",
Type: "integer",

View File

@ -11,20 +11,23 @@ import (
type OpName string
const (
OpNameCreateTable OpName = "create_table"
OpNameRenameTable OpName = "rename_table"
OpNameDropTable OpName = "drop_table"
OpNameAddColumn OpName = "add_column"
OpNameDropColumn OpName = "drop_column"
OpNameCreateIndex OpName = "create_index"
OpNameDropIndex OpName = "drop_index"
OpNameCreateTable OpName = "create_table"
OpNameRenameTable OpName = "rename_table"
OpNameDropTable OpName = "drop_table"
OpNameAddColumn OpName = "add_column"
OpNameDropColumn OpName = "drop_column"
OpNameAlterColumn OpName = "alter_column"
OpNameCreateIndex OpName = "create_index"
OpNameDropIndex OpName = "drop_index"
OpRawSQLName OpName = "sql"
// Internal operation types used by `alter_column`
OpNameRenameColumn OpName = "rename_column"
OpNameSetUnique OpName = "set_unique"
OpNameSetNotNull OpName = "set_not_null"
OpNameSetForeignKey OpName = "set_foreign_key"
OpNameSetCheckConstraint OpName = "set_check_constraint"
OpNameChangeType OpName = "change_type"
OpRawSQLName OpName = "sql"
)
func TemporaryName(name string) string {
@ -95,8 +98,8 @@ func (v *Operations) UnmarshalJSON(data []byte) error {
case OpNameDropColumn:
item = &OpDropColumn{}
case OpNameRenameColumn:
item = &OpRenameColumn{}
case OpNameAlterColumn:
item = &OpAlterColumn{}
case OpNameCreateIndex:
item = &OpCreateIndex{}
@ -116,9 +119,6 @@ func (v *Operations) UnmarshalJSON(data []byte) error {
case OpNameSetCheckConstraint:
item = &OpSetCheckConstraint{}
case OpNameChangeType:
item = &OpChangeType{}
case OpRawSQLName:
item = &OpRawSQL{}
@ -181,8 +181,8 @@ func OperationName(op Operation) OpName {
case *OpDropColumn:
return OpNameDropColumn
case *OpRenameColumn:
return OpNameRenameColumn
case *OpAlterColumn:
return OpNameAlterColumn
case *OpCreateIndex:
return OpNameCreateIndex
@ -202,9 +202,6 @@ func OperationName(op Operation) OpName {
case *OpSetCheckConstraint:
return OpNameSetCheckConstraint
case *OpChangeType:
return OpNameChangeType
case *OpRawSQL:
return OpRawSQLName

View File

@ -37,10 +37,10 @@ func TestRenameColumn(t *testing.T) {
{
Name: "02_rename_column",
Operations: migrations.Operations{
&migrations.OpRenameColumn{
Table: "users",
From: "username",
To: "name",
&migrations.OpAlterColumn{
Table: "users",
Column: "username",
Name: "name",
},
},
},