feat: Column Rename Detection#435
Conversation
Add infrastructure for column rename detection: - DiffTypeTableColumnRename enum value with String/UnmarshalJSON - ColumnRename struct with Old/New column pointers - RenamedColumns field on tableDiff - GetObjectName implementation for ColumnRename - Sorting and initialization for RenamedColumns Co-Authored-By: Claude (claude-opus-4-6) <noreply@anthropic.com>
Implement columnsMatchForRename to check if two columns at the same position with identical properties represent a rename. Add detectColumnRenames to extract rename pairs from added/dropped columns, and wire it into diffTables. Co-Authored-By: Claude (claude-opus-4-6) <noreply@anthropic.com>
Add RENAME COLUMN SQL generation in generateAlterTableStatements, emitted before constraint drops and column drops to preserve data. Also handle constraint comparison with renamed columns by applying the rename map to old constraint column names before comparing. Co-Authored-By: Claude (claude-opus-4-6) <noreply@anthropic.com>
Greptile SummaryThis PR adds automatic column rename detection to table diffs. It changes:
Confidence Score: 3/5This should be fixed before merging.
Focus on rename-map handling in Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Identity must match | ||
| if (old.Identity == nil) != (new.Identity == nil) { | ||
| return false | ||
| } | ||
| if old.Identity != nil && new.Identity != nil { | ||
| if old.Identity.Generation != new.Identity.Generation { | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
This only compares the identity generation mode, so a renamed identity column with changed sequence options is treated as a pure rename. For example, changing GENERATED ALWAYS AS IDENTITY START 1 to START 100 while renaming the column emits only ALTER TABLE ... RENAME COLUMN, leaving the identity sequence options unchanged in the database.
| // Max length must match | ||
| if (old.MaxLength == nil) != (new.MaxLength == nil) { | ||
| return false | ||
| } | ||
| if old.MaxLength != nil && new.MaxLength != nil && *old.MaxLength != *new.MaxLength { | ||
| return false | ||
| } |
There was a problem hiding this comment.
The rename matcher checks MaxLength but not Precision or Scale. A column renamed from amount numeric(10,2) to total numeric(10,4) has the same DataType and position, so this path classifies it as a rename and emits no type alteration, leaving the old precision/scale in place.
| func applyRenameMapToConstraint(c *ir.Constraint, renameMap map[string]string) *ir.Constraint { | ||
| needsUpdate := false | ||
| for _, col := range c.Columns { | ||
| if _, ok := renameMap[col.Name]; ok { | ||
| needsUpdate = true | ||
| break | ||
| } | ||
| } | ||
| if !needsUpdate { | ||
| return c | ||
| } | ||
|
|
||
| // Shallow copy the constraint and update column names | ||
| copy := *c | ||
| copy.Columns = make([]*ir.ConstraintColumn, len(c.Columns)) | ||
| for i, col := range c.Columns { | ||
| if newName, ok := renameMap[col.Name]; ok { | ||
| colCopy := *col | ||
| colCopy.Name = newName | ||
| copy.Columns[i] = &colCopy | ||
| } else { | ||
| copy.Columns[i] = col | ||
| } | ||
| } | ||
| return © |
There was a problem hiding this comment.
This only rewrites the constraint's local Columns, not ReferencedColumns. When table a.id is renamed to a.account_id, a foreign key on another table that references a(id) is automatically updated by PostgreSQL, but the diff for that other table still compares old referenced column id against new referenced column account_id and emits a spurious FK drop/recreate.
| // Rename columns before any drops/adds to preserve data | ||
| for _, rename := range td.RenamedColumns { | ||
| tableName := getTableNameWithSchema(td.Table.Schema, td.Table.Name, targetSchema) | ||
| sql := fmt.Sprintf("ALTER TABLE %s RENAME COLUMN %s TO %s;", | ||
| tableName, ir.QuoteIdentifier(rename.Old.Name), ir.QuoteIdentifier(rename.New.Name)) | ||
|
|
||
| context := &diffContext{ | ||
| Type: DiffTypeTableColumnRename, | ||
| Operation: DiffOperationAlter, | ||
| Path: fmt.Sprintf("%s.%s.%s", td.Table.Schema, td.Table.Name, rename.New.Name), | ||
| Source: rename, | ||
| CanRunInTransaction: true, | ||
| } | ||
| collector.collect(context, sql) | ||
| } |
There was a problem hiding this comment.
Renames are emitted before remaining column drops, which fails when the rename target is the name of a column that is also being dropped. With old columns a, b and new column a representing b -> a plus dropping the old a, this emits RENAME COLUMN b TO a while a still exists, so PostgreSQL rejects the migration with a duplicate column name.
2 participants
Summary
Adds automatic detection of column renames, generating
ALTER TABLE ... RENAME COLUMNinstead ofDROP COLUMN+ADD COLUMN. This preserves data during migrations when a column is simply renamed.Problem
Previously, renaming a column in a schema file (e.g.
active->is_active) produced a destructive plan:This loses all data in the column. The correct migration is:
Additionally, when a renamed column participates in constraints (foreign keys, primary keys, unique), PostgreSQL automatically updates constraint definitions to reference the new column name. The old behavior would emit spurious constraint drop/recreate statements.
Approach
Rename detection uses a position + properties heuristic: if a dropped column and an added column share the same ordinal position and all properties match (type, nullability, default, identity, generated expression) but differ only in name, it is treated as a rename.
Key design decisions
rename_column_type_changetest case.Changes
Core logic (
internal/diff/)column.go-columnsMatchForRename(): compares two columns for rename eligibility (same position, type, nullability, default, identity, generated expr; different name)column_test.go- Unit tests forcolumnsMatchForRenamecovering type mismatch, position mismatch, nullability mismatch, identity, schema-qualified types, etc.table.go-detectColumnRenames(): pairs added/dropped columns by position, returns renames and remaining unpaired columns. Integrates intodiffTables()andgenerateAlterTableStatements()to emitRENAME COLUMNSQL before any drops/adds. Handles comment changes on renamed columns.constraint.go-applyRenameMapToConstraint(): shallow-copies a constraint with column names updated per the rename map, so constraint comparison ignores already-handled renames.diff.go- AddsDiffTypeTableColumnRenamediff type,ColumnRenamestruct, and sorting/serialization support.Test cases (
testdata/diff/create_table/)rename_column/active->is_active(same type, same position)rename_column_type_change/issue_384_rename_column_constraint/RENAME COLUMNinstead of drop/recreate constraintTest plan
PGSCHEMA_TEST_FILTER="rename_column/" go test -v ./internal/diff -run TestDiffFromFiles- rename detection diff testsPGSCHEMA_TEST_FILTER="rename_column_type_change/" go test -v ./internal/diff -run TestDiffFromFiles- type change falls back to drop/addPGSCHEMA_TEST_FILTER="issue_384" go test -v ./internal/diff -run TestDiffFromFiles- constraint handling with renamesgo test -v ./internal/diff -run TestColumnsMatchForRename- unit tests for match logicgo test -v ./internal/diff -run TestDiffFromFiles- all diff tests passPGSCHEMA_TEST_FILTER="rename_column/" go test -v ./cmd -run TestPlanAndApply- integration testgo test ./...- full test suite