Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions internal/diff/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,72 @@ func needsUsingClause(oldType, newType string) bool {
return false
}

// columnsMatchForRename checks if two columns are the same column with a different name.
// Both position AND all properties (except name and comment) must match for a rename to be detected.
func columnsMatchForRename(old, new *ir.Column, targetSchema string) bool {
// Names must differ (otherwise it's not a rename)
if old.Name == new.Name {
return false
}

// Position must match
if old.Position != new.Position {
return false
}

// Data type must match (normalize schema prefix)
oldType := stripSchemaPrefix(old.DataType, targetSchema)
newType := stripSchemaPrefix(new.DataType, targetSchema)
if oldType != newType {
return false
}

// Nullability must match
if old.IsNullable != new.IsNullable {
return false
}

// Default values must match
if (old.DefaultValue == nil) != (new.DefaultValue == nil) {
return false
}
if old.DefaultValue != nil && new.DefaultValue != nil && *old.DefaultValue != *new.DefaultValue {
return false
}

// 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
}
Comment on lines +174 to +180
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Precision changes skipped

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.


// 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
}
}
Comment on lines +182 to +190
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Identity options skipped

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.


// Generated column must match
if old.IsGenerated != new.IsGenerated {
return false
}
if (old.GeneratedExpr == nil) != (new.GeneratedExpr == nil) {
return false
}
if old.GeneratedExpr != nil && new.GeneratedExpr != nil && *old.GeneratedExpr != *new.GeneratedExpr {
return false
}

// Comment changes are OK — rename still applies, comment change emitted separately
return true
}

// columnsEqual compares two columns for equality
// targetSchema is used to normalize type names before comparison
func columnsEqual(old, new *ir.Column, targetSchema string) bool {
Expand Down
84 changes: 84 additions & 0 deletions internal/diff/column_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package diff

import (
"testing"

"github.com/pgplex/pgschema/ir"
)

func TestColumnsMatchForRename(t *testing.T) {
boolDefault := "false"

tests := []struct {
name string
old *ir.Column
new *ir.Column
schema string
expected bool
}{
{
name: "same type and position, different name = match",
old: &ir.Column{Name: "active", Position: 3, DataType: "boolean", IsNullable: false},
new: &ir.Column{Name: "active_n", Position: 3, DataType: "boolean", IsNullable: false},
expected: true,
},
{
name: "different type = no match",
old: &ir.Column{Name: "active", Position: 3, DataType: "boolean", IsNullable: false},
new: &ir.Column{Name: "active_n", Position: 3, DataType: "integer", IsNullable: false},
expected: false,
},
{
name: "different position = no match",
old: &ir.Column{Name: "active", Position: 3, DataType: "boolean", IsNullable: false},
new: &ir.Column{Name: "active_n", Position: 4, DataType: "boolean", IsNullable: false},
expected: false,
},
{
name: "different nullability = no match",
old: &ir.Column{Name: "active", Position: 3, DataType: "boolean", IsNullable: false},
new: &ir.Column{Name: "active_n", Position: 3, DataType: "boolean", IsNullable: true},
expected: false,
},
{
name: "same name = no match (not a rename)",
old: &ir.Column{Name: "active", Position: 3, DataType: "boolean", IsNullable: false},
new: &ir.Column{Name: "active", Position: 3, DataType: "boolean", IsNullable: false},
expected: false,
},
{
name: "with matching defaults = match",
old: &ir.Column{Name: "old_col", Position: 2, DataType: "boolean", DefaultValue: &boolDefault},
new: &ir.Column{Name: "new_col", Position: 2, DataType: "boolean", DefaultValue: &boolDefault},
expected: true,
},
{
name: "with matching identity = match",
old: &ir.Column{Name: "old_id", Position: 1, DataType: "integer", Identity: &ir.Identity{Generation: "ALWAYS"}},
new: &ir.Column{Name: "new_id", Position: 1, DataType: "integer", Identity: &ir.Identity{Generation: "ALWAYS"}},
expected: true,
},
{
name: "different identity generation = no match",
old: &ir.Column{Name: "old_id", Position: 1, DataType: "integer", Identity: &ir.Identity{Generation: "ALWAYS"}},
new: &ir.Column{Name: "new_id", Position: 1, DataType: "integer", Identity: &ir.Identity{Generation: "BY DEFAULT"}},
expected: false,
},
{
name: "schema-qualified type normalized = match",
old: &ir.Column{Name: "old_col", Position: 1, DataType: "public.my_type"},
new: &ir.Column{Name: "new_col", Position: 1, DataType: "my_type"},
schema: "public",
expected: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := columnsMatchForRename(tt.old, tt.new, tt.schema)
if result != tt.expected {
t.Errorf("columnsMatchForRename() = %v, want %v", result, tt.expected)
}
})
}
}
31 changes: 31 additions & 0 deletions internal/diff/constraint.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,3 +223,34 @@ func constraintsEqual(old, new *ir.Constraint) bool {

return true
}

// applyRenameMapToConstraint returns a shallow copy of the constraint with
// column names updated according to the rename map. This is used to compare
// constraints after column renames — PostgreSQL automatically updates constraint
// column references when a column is renamed.
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 &copy
Comment on lines +231 to +255
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Referenced columns untouched

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.

}
17 changes: 17 additions & 0 deletions internal/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const (
DiffTypeTableColumnComment
DiffTypeTableIndexComment
DiffTypeTablePersistence
DiffTypeTableColumnRename
DiffTypeView
DiffTypeViewTrigger
DiffTypeViewComment
Expand Down Expand Up @@ -69,6 +70,8 @@ func (d DiffType) String() string {
return "table.index.comment"
case DiffTypeTablePersistence:
return "table.persistence"
case DiffTypeTableColumnRename:
return "table.column_rename"
case DiffTypeView:
return "view"
case DiffTypeViewTrigger:
Expand Down Expand Up @@ -143,6 +146,8 @@ func (d *DiffType) UnmarshalJSON(data []byte) error {
*d = DiffTypeTableIndexComment
case "table.persistence":
*d = DiffTypeTablePersistence
case "table.column_rename":
*d = DiffTypeTableColumnRename
case "view":
*d = DiffTypeView
case "view.trigger":
Expand Down Expand Up @@ -375,6 +380,7 @@ type tableDiff struct {
AddedColumns []*ir.Column
DroppedColumns []*ir.Column
ModifiedColumns []*ColumnDiff
RenamedColumns []*ColumnRename
AddedConstraints []*ir.Constraint
DroppedConstraints []*ir.Constraint
ModifiedConstraints []*ConstraintDiff
Expand All @@ -400,6 +406,12 @@ type ColumnDiff struct {
New *ir.Column
}

// ColumnRename represents a column that was renamed
type ColumnRename struct {
Old *ir.Column
New *ir.Column
}

// ConstraintDiff represents changes to a constraint
type ConstraintDiff struct {
Old *ir.Constraint
Expand Down Expand Up @@ -1857,6 +1869,10 @@ func sortTableObjects(tables []*tableDiff) {
sort.Slice(tableDiff.ModifiedColumns, func(i, j int) bool {
return tableDiff.ModifiedColumns[i].New.Position < tableDiff.ModifiedColumns[j].New.Position
})

sort.Slice(tableDiff.RenamedColumns, func(i, j int) bool {
return tableDiff.RenamedColumns[i].Old.Position < tableDiff.RenamedColumns[j].Old.Position
})
}
}

Expand Down Expand Up @@ -2166,6 +2182,7 @@ func (d *triggerDiff) GetObjectName() string { return d.New.Name }
func (d *viewDiff) GetObjectName() string { return d.New.Name }
func (d *tableDiff) GetObjectName() string { return d.Table.Name }
func (d *ColumnDiff) GetObjectName() string { return d.New.Name }
func (d *ColumnRename) GetObjectName() string { return d.New.Name }
func (d *ConstraintDiff) GetObjectName() string { return d.New.Name }
func (d *IndexDiff) GetObjectName() string { return d.New.Name }
func (d *policyDiff) GetObjectName() string { return d.New.Name }
Expand Down
Loading