Skip to content

Added support to change numeric precision #363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marioloko wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added support to change numeric precision #363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
38 changes: 35 additions & 3 deletions internal/diff/column.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ func (cd *ColumnDiff) generateColumnSQL(tableSchema, tableName string, targetSch
var statements []string
qualifiedTableName := getTableNameWithSchema(tableSchema, tableName, targetSchema)

// Handle data type changes - normalize types by stripping target schema prefix
oldType := stripSchemaPrefix(cd.Old.DataType, targetSchema)
newType := stripSchemaPrefix(cd.New.DataType, targetSchema)
// Handle data type changes - use full type with precision/scale/length modifiers
oldType := stripSchemaPrefix(columnTypeWithModifiers(cd.Old), targetSchema)
newType := stripSchemaPrefix(columnTypeWithModifiers(cd.New), targetSchema)

// Check if there's a type change AND the column has a default value
// When a USING clause is needed, we must: DROP DEFAULT -> ALTER TYPE -> SET DEFAULT
Expand Down Expand Up @@ -170,6 +170,22 @@ func columnsEqual(old, new *ir.Column, targetSchema string) bool {
return false
}

// Compare precision
if (old.Precision == nil) != (new.Precision == nil) {
return false
}
if old.Precision != nil && new.Precision != nil && *old.Precision != *new.Precision {
return false
}

// Compare scale
if (old.Scale == nil) != (new.Scale == nil) {
return false
}
if old.Scale != nil && new.Scale != nil && *old.Scale != *new.Scale {
return false
}

// Compare identity columns
if (old.Identity == nil) != (new.Identity == nil) {
return false
Expand All @@ -187,3 +203,19 @@ func columnsEqual(old, new *ir.Column, targetSchema string) bool {

return true
}

// columnTypeWithModifiers returns the column's DataType with precision/scale/length
// modifiers appended, without serial type transformation.
func columnTypeWithModifiers(col *ir.Column) string {
dt := col.DataType
if col.MaxLength != nil && (dt == "varchar" || dt == "character varying") {
return fmt.Sprintf("varchar(%d)", *col.MaxLength)
} else if col.MaxLength != nil && dt == "character" {
return fmt.Sprintf("character(%d)", *col.MaxLength)
} else if col.Precision != nil && col.Scale != nil && (dt == "numeric" || dt == "decimal") {
return fmt.Sprintf("%s(%d,%d)", dt, *col.Precision, *col.Scale)
} else if col.Precision != nil && (dt == "numeric" || dt == "decimal") {
return fmt.Sprintf("%s(%d)", dt, *col.Precision)
}
Comment on lines +211 to +219
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

columnTypeWithModifiers only applies MaxLength modifiers for varchar/character types. But columnsEqual compares MaxLength for all columns, and the inspector populates MaxLength from character_maximum_length, which also applies to types like bit / bit varying (varbit). If a bit-string length changes, it will be detected as a column change but generateColumnSQL won’t emit an ALTER ... TYPE bit(n) because oldType/newType will both remain the bare DataType. Consider extending columnTypeWithModifiers (or using a shared formatter) to include length modifiers for other length-typed columns where MaxLength is set.

Suggested change
if col.MaxLength != nil && (dt == "varchar" || dt == "character varying") {
return fmt.Sprintf("varchar(%d)", *col.MaxLength)
} else if col.MaxLength != nil && dt == "character" {
return fmt.Sprintf("character(%d)", *col.MaxLength)
} else if col.Precision != nil && col.Scale != nil && (dt == "numeric" || dt == "decimal") {
return fmt.Sprintf("%s(%d,%d)", dt, *col.Precision, *col.Scale)
} else if col.Precision != nil && (dt == "numeric" || dt == "decimal") {
return fmt.Sprintf("%s(%d)", dt, *col.Precision)
}
// Length-based modifiers (character and bit string types)
if col.MaxLength != nil {
switch dt {
case "varchar", "character varying":
// Normalize to varchar(n) for varying-length character types
return fmt.Sprintf("varchar(%d)", *col.MaxLength)
case "character":
return fmt.Sprintf("character(%d)", *col.MaxLength)
case "bit":
return fmt.Sprintf("bit(%d)", *col.MaxLength)
case "bit varying", "varbit":
// Use canonical "bit varying(n)" representation
return fmt.Sprintf("bit varying(%d)", *col.MaxLength)
}
}
// Numeric precision/scale modifiers
if col.Precision != nil && col.Scale != nil && (dt == "numeric" || dt == "decimal") {
return fmt.Sprintf("%s(%d,%d)", dt, *col.Precision, *col.Scale)
} else if col.Precision != nil && (dt == "numeric" || dt == "decimal") {
return fmt.Sprintf("%s(%d)", dt, *col.Precision)
}

Copilot uses AI. Check for mistakes.
return dt
Comment on lines +210 to +220
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

columnTypeWithModifiers duplicates the type/modifier formatting logic already implemented in internal/diff/table.go (e.g., formatColumnDataType / formatColumnDataTypeForCreate). To avoid these drifting out of sync (especially as more modifier-carrying types are added), consider refactoring to reuse a single formatter (possibly with an option to skip SERIAL transformation, since this helper mentions that requirement).

Suggested change
dt := col.DataType
if col.MaxLength != nil && (dt == "varchar" || dt == "character varying") {
return fmt.Sprintf("varchar(%d)", *col.MaxLength)
} else if col.MaxLength != nil && dt == "character" {
return fmt.Sprintf("character(%d)", *col.MaxLength)
} else if col.Precision != nil && col.Scale != nil && (dt == "numeric" || dt == "decimal") {
return fmt.Sprintf("%s(%d,%d)", dt, *col.Precision, *col.Scale)
} else if col.Precision != nil && (dt == "numeric" || dt == "decimal") {
return fmt.Sprintf("%s(%d)", dt, *col.Precision)
}
return dt
return formatColumnDataType(col)

Copilot uses AI. Check for mistakes.
}
Comment on lines +209 to +221
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Mar 22, 2026

Choose a reason for hiding this comment

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

P2 Duplicated modifier-formatting logic

columnTypeWithModifiers is essentially the same as formatColumnDataType (in table.go), which already handles the varchar/character/numeric/decimal cases. The only intentional difference is that columnTypeWithModifiers skips the serial → serial/bigserial rewrite, but formatColumnDataType can also return the raw integer type when the column is not a serial.

Reusing formatColumnDataType (or extracting the shared modifier-building block into a small unexported helper) would keep the logic in one place and reduce the risk of future drift — e.g., if bit(n) or timestamp(p) precision support is added later, it would need to be patched in both files.

1 change: 1 addition & 0 deletions testdata/diff/create_table/alter_numeric_precision/diff.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE transactions ALTER COLUMN amount TYPE numeric(16,6);
Copy link
Copy Markdown
Contributor

@tianzhou tianzhou Mar 23, 2026

Choose a reason for hiding this comment

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

for file name, pls prefix with issue_xxx

#352

5 changes: 5 additions & 0 deletions testdata/diff/create_table/alter_numeric_precision/new.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
CREATE TABLE public.transactions (
id integer NOT NULL,
description text,
amount numeric(16,6)
);
5 changes: 5 additions & 0 deletions testdata/diff/create_table/alter_numeric_precision/old.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
CREATE TABLE public.transactions (
id integer NOT NULL,
description text,
amount numeric(15,4)
);
20 changes: 20 additions & 0 deletions testdata/diff/create_table/alter_numeric_precision/plan.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"version": "1.0.0",
"pgschema_version": "1.7.4",
"created_at": "1970-01-01T00:00:00Z",
"source_fingerprint": {
"hash": "4dae5400719835d4bc41b032c23f8510b2050a8cad16ca8461f488c927c68d20"
},
"groups": [
{
"steps": [
{
"sql": "ALTER TABLE transactions ALTER COLUMN amount TYPE numeric(16,6);",
"type": "table.column",
"operation": "alter",
"path": "public.transactions.amount"
}
]
}
]
}
1 change: 1 addition & 0 deletions testdata/diff/create_table/alter_numeric_precision/plan.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE transactions ALTER COLUMN amount TYPE numeric(16,6);
13 changes: 13 additions & 0 deletions testdata/diff/create_table/alter_numeric_precision/plan.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
Plan: 1 to modify.

Summary by type:
tables: 1 to modify

Tables:
~ transactions
~ amount (column)

DDL to be executed:
--------------------------------------------------

ALTER TABLE transactions ALTER COLUMN amount TYPE numeric(16,6);
Loading