diff --git a/internal/diff/column.go b/internal/diff/column.go index 4ebd2174..53d5934e 100644 --- a/internal/diff/column.go +++ b/internal/diff/column.go @@ -112,36 +112,34 @@ func (cd *ColumnDiff) generateColumnSQL(tableSchema, tableName string, targetSch // needsUsingClause determines if a type conversion requires a USING clause. // -// This is especially important when converting to or from custom types (like ENUMs), -// because PostgreSQL often cannot implicitly cast these types. To avoid generating -// invalid migrations, this function takes a conservative approach: -// -// - Any conversion involving at least one non–built-in (custom) type will require -// a USING clause. -// - For built-in → built-in conversions we still assume PostgreSQL provides an -// implicit cast in most cases; callers should be aware that some edge cases -// (e.g. certain text → json conversions) may still need manual adjustment. +// PostgreSQL rejects bare ALTER COLUMN TYPE when no implicit cast exists between +// the old and new types (SQLSTATE 42804). When the base type names differ, we +// always emit USING col::newtype — a redundant USING is harmless when an implicit +// cast exists, but a missing one breaks the migration. When only modifiers change +// (e.g. numeric(18,6) → numeric(20,6)), no USING is needed. func needsUsingClause(oldType, newType string) bool { - // Check if old type is text-like - oldIsTextLike := ir.IsTextLikeType(oldType) - - // Determine whether the old/new types are PostgreSQL built-ins - oldIsBuiltIn := ir.IsBuiltInType(oldType) - newIsBuiltIn := ir.IsBuiltInType(newType) - - // Preserve existing behavior: text-like → non–built-in likely needs USING - if oldIsTextLike && !newIsBuiltIn { - return true + oldNorm := normalizeBaseTypeName(oldType) + newNorm := normalizeBaseTypeName(newType) + if oldNorm == newNorm { + return false } + return true +} - // Be conservative for any conversion involving custom (non–built-in) types: - // this covers custom → custom and built-in ↔ custom conversions. - if !oldIsBuiltIn || !newIsBuiltIn { - return true +func normalizeBaseTypeName(typeName string) string { + t := typeName + if open := strings.Index(t, "("); open != -1 { + if close := strings.Index(t[open:], ")"); close != -1 { + t = t[:open] + t[open+close+1:] + } else { + t = t[:open] + } } - - // For built-in → built-in types we assume an implicit cast is available. - return false + t = strings.TrimPrefix(t, "pg_catalog.") + if !strings.Contains(t, "\"") { + t = strings.ToLower(t) + } + return t } // comparableColumnType returns the column's data type including any diff --git a/internal/diff/column_test.go b/internal/diff/column_test.go new file mode 100644 index 00000000..3c6198bb --- /dev/null +++ b/internal/diff/column_test.go @@ -0,0 +1,59 @@ +package diff + +import "testing" + +func TestNormalizeBaseTypeName(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"integer", "integer"}, + {"INTEGER", "integer"}, + {"bigint", "bigint"}, + {"numeric(18,6)", "numeric"}, + {"numeric(20,6)", "numeric"}, + {"varchar(128)", "varchar"}, + {"character varying(255)", "character varying"}, + {"integer[]", "integer[]"}, + {"character varying(128)[]", "character varying[]"}, + {"timestamp(6) with time zone", "timestamp with time zone"}, + {"pg_catalog.int4", "int4"}, + {`"MyStatus"`, `"MyStatus"`}, + {`"mystatus"`, `"mystatus"`}, + {`"MyStatus"[]`, `"MyStatus"[]`}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got := normalizeBaseTypeName(tt.input) + if got != tt.want { + t.Errorf("normalizeBaseTypeName(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + +func TestNeedsUsingClause(t *testing.T) { + tests := []struct { + old string + new string + want bool + }{ + {"text", "integer", true}, + {"integer", "bigint", true}, + {"numeric(18,6)", "numeric(20,6)", false}, + {"integer", "integer[]", true}, + {"text", "action_type", true}, + {"varchar(128)", "varchar(255)", false}, + {"timestamp(3) with time zone", "timestamp(6) with time zone", false}, + {"timestamp without time zone", "timestamp with time zone", true}, + {`"MyStatus"`, `"mystatus"`, true}, + } + for _, tt := range tests { + t.Run(tt.old+"→"+tt.new, func(t *testing.T) { + got := needsUsingClause(tt.old, tt.new) + if got != tt.want { + t.Errorf("needsUsingClause(%q, %q) = %v, want %v", tt.old, tt.new, got, tt.want) + } + }) + } +} diff --git a/testdata/diff/create_table/alter_column_quoted_identifier/diff.sql b/testdata/diff/create_table/alter_column_quoted_identifier/diff.sql index 3f8698d4..bd4c40cc 100644 --- a/testdata/diff/create_table/alter_column_quoted_identifier/diff.sql +++ b/testdata/diff/create_table/alter_column_quoted_identifier/diff.sql @@ -1 +1 @@ -ALTER TABLE ex ALTER COLUMN "ID" TYPE bigint; +ALTER TABLE ex ALTER COLUMN "ID" TYPE bigint USING "ID"::bigint; diff --git a/testdata/diff/create_table/alter_column_quoted_identifier/plan.json b/testdata/diff/create_table/alter_column_quoted_identifier/plan.json index 7835855c..df0bbd27 100644 --- a/testdata/diff/create_table/alter_column_quoted_identifier/plan.json +++ b/testdata/diff/create_table/alter_column_quoted_identifier/plan.json @@ -1,6 +1,6 @@ { "version": "1.0.0", - "pgschema_version": "1.12.1", + "pgschema_version": "1.12.2", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { "hash": "9d443bc536153eed8fce077bfacc3d7f42b1a94f02d33868bb78be3b9de05088" @@ -9,7 +9,7 @@ { "steps": [ { - "sql": "ALTER TABLE ex ALTER COLUMN \"ID\" TYPE bigint;", + "sql": "ALTER TABLE ex ALTER COLUMN \"ID\" TYPE bigint USING \"ID\"::bigint;", "type": "table.column", "operation": "alter", "path": "public.ex.ID" diff --git a/testdata/diff/create_table/alter_column_quoted_identifier/plan.sql b/testdata/diff/create_table/alter_column_quoted_identifier/plan.sql index 3f8698d4..bd4c40cc 100644 --- a/testdata/diff/create_table/alter_column_quoted_identifier/plan.sql +++ b/testdata/diff/create_table/alter_column_quoted_identifier/plan.sql @@ -1 +1 @@ -ALTER TABLE ex ALTER COLUMN "ID" TYPE bigint; +ALTER TABLE ex ALTER COLUMN "ID" TYPE bigint USING "ID"::bigint; diff --git a/testdata/diff/create_table/alter_column_quoted_identifier/plan.txt b/testdata/diff/create_table/alter_column_quoted_identifier/plan.txt index 555ca8a6..df980f56 100644 --- a/testdata/diff/create_table/alter_column_quoted_identifier/plan.txt +++ b/testdata/diff/create_table/alter_column_quoted_identifier/plan.txt @@ -10,4 +10,4 @@ Tables: DDL to be executed: -------------------------------------------------- -ALTER TABLE ex ALTER COLUMN "ID" TYPE bigint; +ALTER TABLE ex ALTER COLUMN "ID" TYPE bigint USING "ID"::bigint; diff --git a/testdata/diff/create_table/alter_column_types/diff.sql b/testdata/diff/create_table/alter_column_types/diff.sql index b36b7731..9332566e 100644 --- a/testdata/diff/create_table/alter_column_types/diff.sql +++ b/testdata/diff/create_table/alter_column_types/diff.sql @@ -4,11 +4,11 @@ CREATE TYPE action_type AS ENUM ( 'rejected' ); -ALTER TABLE user_pending_permissions ALTER COLUMN id TYPE bigint; +ALTER TABLE user_pending_permissions ALTER COLUMN id TYPE bigint USING id::bigint; -ALTER TABLE user_pending_permissions ALTER COLUMN user_id TYPE bigint; +ALTER TABLE user_pending_permissions ALTER COLUMN user_id TYPE bigint USING user_id::bigint; -ALTER TABLE user_pending_permissions ALTER COLUMN object_ids_ints TYPE bigint[]; +ALTER TABLE user_pending_permissions ALTER COLUMN object_ids_ints TYPE bigint[] USING object_ids_ints::bigint[]; ALTER TABLE user_pending_permissions ALTER COLUMN action TYPE action_type USING action::action_type; @@ -21,3 +21,11 @@ ALTER TABLE user_pending_permissions ALTER COLUMN status SET DEFAULT 'pending':: ALTER TABLE user_pending_permissions ALTER COLUMN tags TYPE action_type[] USING tags::action_type[]; ALTER TABLE user_pending_permissions ALTER COLUMN amount TYPE numeric(20,6); + +ALTER TABLE user_pending_permissions ALTER COLUMN arfcn_dl DROP DEFAULT; + +ALTER TABLE user_pending_permissions ALTER COLUMN arfcn_dl TYPE integer USING arfcn_dl::integer; + +ALTER TABLE user_pending_permissions ALTER COLUMN arfcn_dl SET DEFAULT 0; + +ALTER TABLE user_pending_permissions ALTER COLUMN priority TYPE integer USING priority::integer; diff --git a/testdata/diff/create_table/alter_column_types/new.sql b/testdata/diff/create_table/alter_column_types/new.sql index 7dbfb787..333d3f4f 100644 --- a/testdata/diff/create_table/alter_column_types/new.sql +++ b/testdata/diff/create_table/alter_column_types/new.sql @@ -8,5 +8,7 @@ CREATE TABLE public.user_pending_permissions ( action public.action_type, status public.action_type DEFAULT 'pending', tags public.action_type[], - amount numeric(20,6) NOT NULL DEFAULT 0 + amount numeric(20,6) NOT NULL DEFAULT 0, + arfcn_dl integer DEFAULT 0, + priority integer ); \ No newline at end of file diff --git a/testdata/diff/create_table/alter_column_types/old.sql b/testdata/diff/create_table/alter_column_types/old.sql index bb6d2ae6..e162cdc0 100644 --- a/testdata/diff/create_table/alter_column_types/old.sql +++ b/testdata/diff/create_table/alter_column_types/old.sql @@ -6,5 +6,7 @@ CREATE TABLE public.user_pending_permissions ( action text, status text DEFAULT 'pending', tags text[], - amount numeric(18,6) NOT NULL DEFAULT 0 + amount numeric(18,6) NOT NULL DEFAULT 0, + arfcn_dl text DEFAULT 'unknown', + priority text ); \ No newline at end of file diff --git a/testdata/diff/create_table/alter_column_types/plan.json b/testdata/diff/create_table/alter_column_types/plan.json index 3b7a6677..c95db1db 100644 --- a/testdata/diff/create_table/alter_column_types/plan.json +++ b/testdata/diff/create_table/alter_column_types/plan.json @@ -1,9 +1,9 @@ { "version": "1.0.0", - "pgschema_version": "1.12.1", + "pgschema_version": "1.12.2", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "614655f95d349ec321da570d4e232db71f8bfb57404998153f9fd53720cd2acb" + "hash": "31ef5d21f4bfa9713df469db3eb54585bf60ea348f09608b21f7d05eae9f3f02" }, "groups": [ { @@ -15,19 +15,19 @@ "path": "public.action_type" }, { - "sql": "ALTER TABLE user_pending_permissions ALTER COLUMN id TYPE bigint;", + "sql": "ALTER TABLE user_pending_permissions ALTER COLUMN id TYPE bigint USING id::bigint;", "type": "table.column", "operation": "alter", "path": "public.user_pending_permissions.id" }, { - "sql": "ALTER TABLE user_pending_permissions ALTER COLUMN user_id TYPE bigint;", + "sql": "ALTER TABLE user_pending_permissions ALTER COLUMN user_id TYPE bigint USING user_id::bigint;", "type": "table.column", "operation": "alter", "path": "public.user_pending_permissions.user_id" }, { - "sql": "ALTER TABLE user_pending_permissions ALTER COLUMN object_ids_ints TYPE bigint[];", + "sql": "ALTER TABLE user_pending_permissions ALTER COLUMN object_ids_ints TYPE bigint[] USING object_ids_ints::bigint[];", "type": "table.column", "operation": "alter", "path": "public.user_pending_permissions.object_ids_ints" @@ -67,6 +67,30 @@ "type": "table.column", "operation": "alter", "path": "public.user_pending_permissions.amount" + }, + { + "sql": "ALTER TABLE user_pending_permissions ALTER COLUMN arfcn_dl DROP DEFAULT;", + "type": "table.column", + "operation": "alter", + "path": "public.user_pending_permissions.arfcn_dl" + }, + { + "sql": "ALTER TABLE user_pending_permissions ALTER COLUMN arfcn_dl TYPE integer USING arfcn_dl::integer;", + "type": "table.column", + "operation": "alter", + "path": "public.user_pending_permissions.arfcn_dl" + }, + { + "sql": "ALTER TABLE user_pending_permissions ALTER COLUMN arfcn_dl SET DEFAULT 0;", + "type": "table.column", + "operation": "alter", + "path": "public.user_pending_permissions.arfcn_dl" + }, + { + "sql": "ALTER TABLE user_pending_permissions ALTER COLUMN priority TYPE integer USING priority::integer;", + "type": "table.column", + "operation": "alter", + "path": "public.user_pending_permissions.priority" } ] } diff --git a/testdata/diff/create_table/alter_column_types/plan.sql b/testdata/diff/create_table/alter_column_types/plan.sql index b36b7731..9332566e 100644 --- a/testdata/diff/create_table/alter_column_types/plan.sql +++ b/testdata/diff/create_table/alter_column_types/plan.sql @@ -4,11 +4,11 @@ CREATE TYPE action_type AS ENUM ( 'rejected' ); -ALTER TABLE user_pending_permissions ALTER COLUMN id TYPE bigint; +ALTER TABLE user_pending_permissions ALTER COLUMN id TYPE bigint USING id::bigint; -ALTER TABLE user_pending_permissions ALTER COLUMN user_id TYPE bigint; +ALTER TABLE user_pending_permissions ALTER COLUMN user_id TYPE bigint USING user_id::bigint; -ALTER TABLE user_pending_permissions ALTER COLUMN object_ids_ints TYPE bigint[]; +ALTER TABLE user_pending_permissions ALTER COLUMN object_ids_ints TYPE bigint[] USING object_ids_ints::bigint[]; ALTER TABLE user_pending_permissions ALTER COLUMN action TYPE action_type USING action::action_type; @@ -21,3 +21,11 @@ ALTER TABLE user_pending_permissions ALTER COLUMN status SET DEFAULT 'pending':: ALTER TABLE user_pending_permissions ALTER COLUMN tags TYPE action_type[] USING tags::action_type[]; ALTER TABLE user_pending_permissions ALTER COLUMN amount TYPE numeric(20,6); + +ALTER TABLE user_pending_permissions ALTER COLUMN arfcn_dl DROP DEFAULT; + +ALTER TABLE user_pending_permissions ALTER COLUMN arfcn_dl TYPE integer USING arfcn_dl::integer; + +ALTER TABLE user_pending_permissions ALTER COLUMN arfcn_dl SET DEFAULT 0; + +ALTER TABLE user_pending_permissions ALTER COLUMN priority TYPE integer USING priority::integer; diff --git a/testdata/diff/create_table/alter_column_types/plan.txt b/testdata/diff/create_table/alter_column_types/plan.txt index f7294263..0daab2b4 100644 --- a/testdata/diff/create_table/alter_column_types/plan.txt +++ b/testdata/diff/create_table/alter_column_types/plan.txt @@ -11,8 +11,10 @@ Tables: ~ user_pending_permissions ~ action (column) ~ amount (column) + ~ arfcn_dl (column) ~ id (column) ~ object_ids_ints (column) + ~ priority (column) ~ status (column) ~ tags (column) ~ user_id (column) @@ -26,11 +28,11 @@ CREATE TYPE action_type AS ENUM ( 'rejected' ); -ALTER TABLE user_pending_permissions ALTER COLUMN id TYPE bigint; +ALTER TABLE user_pending_permissions ALTER COLUMN id TYPE bigint USING id::bigint; -ALTER TABLE user_pending_permissions ALTER COLUMN user_id TYPE bigint; +ALTER TABLE user_pending_permissions ALTER COLUMN user_id TYPE bigint USING user_id::bigint; -ALTER TABLE user_pending_permissions ALTER COLUMN object_ids_ints TYPE bigint[]; +ALTER TABLE user_pending_permissions ALTER COLUMN object_ids_ints TYPE bigint[] USING object_ids_ints::bigint[]; ALTER TABLE user_pending_permissions ALTER COLUMN action TYPE action_type USING action::action_type; @@ -43,3 +45,11 @@ ALTER TABLE user_pending_permissions ALTER COLUMN status SET DEFAULT 'pending':: ALTER TABLE user_pending_permissions ALTER COLUMN tags TYPE action_type[] USING tags::action_type[]; ALTER TABLE user_pending_permissions ALTER COLUMN amount TYPE numeric(20,6); + +ALTER TABLE user_pending_permissions ALTER COLUMN arfcn_dl DROP DEFAULT; + +ALTER TABLE user_pending_permissions ALTER COLUMN arfcn_dl TYPE integer USING arfcn_dl::integer; + +ALTER TABLE user_pending_permissions ALTER COLUMN arfcn_dl SET DEFAULT 0; + +ALTER TABLE user_pending_permissions ALTER COLUMN priority TYPE integer USING priority::integer;