From 895afe45717f706efcd9cc2a4fda9b02a941c656 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Fri, 17 Jan 2025 15:08:09 +0000 Subject: [PATCH] Add `OpRenameColumn` for renaming columns (#602) Add a new `rename_column` operation for performing column renames: ```json { "name": "13_rename_column", "operations": [ { "rename_column": { "table": "employees", "from": "role", "to": "job_title" } } ] } ``` Renaming columns is currently done using the `alter_column` operation: ```json { "name": "13_rename_column", "operations": [ { "alter_column": { "table": "employees", "column": "role", "name": "job_title" } } ] } ``` As of this PR both approaches are valid, but the second approach (using `alter_column` to perform column renames) will be removed as part of #601 Part of #601 --- docs/operations/alter_column/README.md | 2 +- .../{alter_column => }/rename_column.mdx | 2 +- examples/13_rename_column.json | 6 +- examples/35_alter_column_multiple.json | 1 - examples/36_set_comment_to_null.json | 6 +- pkg/migrations/op_common.go | 7 + pkg/migrations/op_rename_column.go | 64 +++++++ pkg/migrations/op_rename_column_test.go | 156 ++++++++++++++++++ pkg/migrations/types.go | 12 ++ schema.json | 31 ++++ 10 files changed, 278 insertions(+), 9 deletions(-) rename docs/operations/{alter_column => }/rename_column.mdx (95%) create mode 100644 pkg/migrations/op_rename_column.go diff --git a/docs/operations/alter_column/README.md b/docs/operations/alter_column/README.md index d3ca5af6..298b916d 100644 --- a/docs/operations/alter_column/README.md +++ b/docs/operations/alter_column/README.md @@ -2,4 +2,4 @@ An alter column operation alters the properties of a column. The operation supports several sub-operations, described below. -An alter column operation may contain multiple sub-operations. For example, a single alter column operation may rename a column, change its type, and add a check constraint. +An alter column operation may contain multiple sub-operations. For example, a single alter column operation may change its type, and add a check constraint. diff --git a/docs/operations/alter_column/rename_column.mdx b/docs/operations/rename_column.mdx similarity index 95% rename from docs/operations/alter_column/rename_column.mdx rename to docs/operations/rename_column.mdx index 1774cdb6..11da34ba 100644 --- a/docs/operations/alter_column/rename_column.mdx +++ b/docs/operations/rename_column.mdx @@ -7,7 +7,7 @@ description: A rename column operation renames a column. ```json { - "alter_column": { + "rename_column": { "table": "table name", "column": "old column name", "name": "new column name" diff --git a/examples/13_rename_column.json b/examples/13_rename_column.json index e6500827..f73f0bea 100644 --- a/examples/13_rename_column.json +++ b/examples/13_rename_column.json @@ -2,10 +2,10 @@ "name": "13_rename_column", "operations": [ { - "alter_column": { + "rename_column": { "table": "employees", - "column": "role", - "name": "job_title" + "from": "role", + "to": "job_title" } } ] diff --git a/examples/35_alter_column_multiple.json b/examples/35_alter_column_multiple.json index a1247744..4ef46b6d 100644 --- a/examples/35_alter_column_multiple.json +++ b/examples/35_alter_column_multiple.json @@ -5,7 +5,6 @@ "alter_column": { "table": "events", "column": "name", - "name": "event_name", "type": "text", "default": "'unknown event'", "nullable": false, diff --git a/examples/36_set_comment_to_null.json b/examples/36_set_comment_to_null.json index 551a0102..a6793b55 100644 --- a/examples/36_set_comment_to_null.json +++ b/examples/36_set_comment_to_null.json @@ -4,10 +4,10 @@ { "alter_column": { "table": "events", - "column": "event_name", + "column": "name", "comment": null, - "up": "event_name", - "down": "event_name" + "up": "name", + "down": "name" } } ] diff --git a/pkg/migrations/op_common.go b/pkg/migrations/op_common.go index 01c77abe..2b75c473 100644 --- a/pkg/migrations/op_common.go +++ b/pkg/migrations/op_common.go @@ -14,6 +14,7 @@ type OpName string const ( OpNameCreateTable OpName = "create_table" OpNameRenameTable OpName = "rename_table" + OpNameRenameColumn OpName = "rename_column" OpNameDropTable OpName = "drop_table" OpNameAddColumn OpName = "add_column" OpNameDropColumn OpName = "drop_column" @@ -99,6 +100,9 @@ func (v *Operations) UnmarshalJSON(data []byte) error { case OpNameAddColumn: item = &OpAddColumn{} + case OpNameRenameColumn: + item = &OpRenameColumn{} + case OpNameDropColumn: item = &OpDropColumn{} @@ -191,6 +195,9 @@ func OperationName(op Operation) OpName { case *OpDropColumn: return OpNameDropColumn + case *OpRenameColumn: + return OpNameRenameColumn + case *OpRenameConstraint: return OpNameRenameConstraint diff --git a/pkg/migrations/op_rename_column.go b/pkg/migrations/op_rename_column.go new file mode 100644 index 00000000..5a3dbb74 --- /dev/null +++ b/pkg/migrations/op_rename_column.go @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: Apache-2.0 +package migrations + +import ( + "context" + "fmt" + + "github.com/lib/pq" + "github.com/xataio/pgroll/pkg/db" + "github.com/xataio/pgroll/pkg/schema" +) + +var _ Operation = (*OpRenameColumn)(nil) + +func (o *OpRenameColumn) Start(ctx context.Context, conn db.DB, latestSchema string, tr SQLTransformer, s *schema.Schema, cbs ...CallbackFn) (*schema.Table, error) { + // Rename the table in the in-memory schema. + table := s.GetTable(o.Table) + table.RenameColumn(o.From, o.To) + + return nil, nil +} + +func (o *OpRenameColumn) Complete(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { + // Rename the column + _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s RENAME COLUMN %s TO %s", + pq.QuoteIdentifier(o.Table), + pq.QuoteIdentifier(o.From), + pq.QuoteIdentifier(o.To))) + + return err +} + +func (o *OpRenameColumn) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { + // Rename the column back to the original name in the in-memory schema. + table := s.GetTable(o.Table) + table.RenameColumn(o.To, o.From) + + return nil +} + +func (o *OpRenameColumn) Validate(ctx context.Context, s *schema.Schema) error { + table := s.GetTable(o.Table) + + // Ensure that the table exists. + if table == nil { + return TableDoesNotExistError{Name: o.Table} + } + + // Ensure that the column exists. + if table.GetColumn(o.From) == nil { + return ColumnDoesNotExistError{Table: o.Table, Name: o.From} + } + + // Ensure that the new column name does not already exist + if table.GetColumn(o.To) != nil { + return ColumnAlreadyExistsError{Table: o.Table, Name: o.To} + } + + // Update the in-memory schema to reflect the column rename so that it is + // visible to subsequent operations' validation steps. + table.RenameColumn(o.From, o.To) + + return nil +} diff --git a/pkg/migrations/op_rename_column_test.go b/pkg/migrations/op_rename_column_test.go index ff74878f..4c9f3208 100644 --- a/pkg/migrations/op_rename_column_test.go +++ b/pkg/migrations/op_rename_column_test.go @@ -98,3 +98,159 @@ func TestRenameColumn(t *testing.T) { wantStartErr: migrations.ColumnAlreadyExistsError{Table: "users", Name: "id"}, }}) } + +func TestOpRenameColumn(t *testing.T) { + t.Parallel() + + ExecuteTests(t, TestCases{ + { + name: "rename column", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: true, + }, + { + Name: "username", + Type: "varchar(255)", + Nullable: false, + }, + }, + }, + }, + }, + { + Name: "02_rename_column", + Operations: migrations.Operations{ + &migrations.OpRenameColumn{ + Table: "users", + From: "username", + To: "name", + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The column in the underlying table has not been renamed. + ColumnMustExist(t, db, schema, "users", "username") + + // Insertions to the new column name in the new version schema should work. + MustInsert(t, db, schema, "02_rename_column", "users", map[string]string{ + "name": "alice", + }) + + // Insertions to the old column name in the old version schema should work. + MustInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "username": "bob", + }) + + // Data can be read from the view in the new version schema. + rows := MustSelect(t, db, schema, "02_rename_column", "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) { + // no-op + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // The column in the underlying table has been renamed. + ColumnMustExist(t, db, schema, "users", "name") + + // Data can be read from the view in the new version schema. + rows := MustSelect(t, db, schema, "02_rename_column", "users") + assert.Equal(t, []map[string]any{ + {"id": 1, "name": "alice"}, + {"id": 2, "name": "bob"}, + }, rows) + }, + }, + }) +} + +func TestOpRenameColumnValidation(t *testing.T) { + t.Parallel() + + createTableMigration := migrations.Migration{ + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: true, + }, + { + Name: "username", + Type: "varchar(255)", + Nullable: false, + }, + }, + }, + }, + } + + ExecuteTests(t, TestCases{ + { + name: "table must exist", + migrations: []migrations.Migration{ + createTableMigration, + { + Name: "02_rename_column", + Operations: migrations.Operations{ + &migrations.OpRenameColumn{ + Table: "doesntexist", + From: "username", + To: "id", + }, + }, + }, + }, + wantStartErr: migrations.TableDoesNotExistError{Name: "doesntexist"}, + }, + { + name: "source column must exist", + migrations: []migrations.Migration{ + createTableMigration, + { + Name: "02_rename_column", + Operations: migrations.Operations{ + &migrations.OpRenameColumn{ + Table: "users", + From: "doesntexist", + To: "id", + }, + }, + }, + }, + wantStartErr: migrations.ColumnDoesNotExistError{Table: "users", Name: "doesntexist"}, + }, + { + name: "target column must not exist", + migrations: []migrations.Migration{ + createTableMigration, + { + Name: "02_rename_column", + Operations: migrations.Operations{ + &migrations.OpRenameColumn{ + Table: "users", + From: "username", + To: "id", + }, + }, + }, + }, + wantStartErr: migrations.ColumnAlreadyExistsError{Table: "users", Name: "id"}, + }, + }) +} diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index e0b295a4..65ae8ecb 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -332,6 +332,18 @@ type OpRawSQL struct { Up string `json:"up"` } +// Rename column operation +type OpRenameColumn struct { + // Old name of the column + From string `json:"from"` + + // Name of the table + Table string `json:"table"` + + // New name of the column + To string `json:"to"` +} + // Rename constraint operation type OpRenameConstraint struct { // Name of the constraint diff --git a/schema.json b/schema.json index 4e5c43d8..19b16850 100644 --- a/schema.json +++ b/schema.json @@ -262,6 +262,26 @@ "required": ["column", "table"], "type": "object" }, + "OpRenameColumn": { + "additionalProperties": false, + "description": "Rename column operation", + "properties": { + "table": { + "description": "Name of the table", + "type": "string" + }, + "from": { + "description": "Old name of the column", + "type": "string" + }, + "to": { + "description": "New name of the column", + "type": "string" + } + }, + "required": ["table", "from", "to"], + "type": "object" + }, "OpAlterColumn": { "additionalProperties": false, "description": "Alter column operation", @@ -710,6 +730,17 @@ }, "required": ["alter_column"] }, + { + "type": "object", + "description": "Rename column operation", + "additionalProperties": false, + "properties": { + "rename_column": { + "$ref": "#/$defs/OpRenameColumn" + } + }, + "required": ["rename_column"] + }, { "type": "object", "description": "Create index operation",