Skip to content

sql/postgres: treat serial as equivalent to integer + nextval default#1

Open
wokalski wants to merge 4 commits into
masterfrom
fix/postgres-serial-nextval-diff
Open

sql/postgres: treat serial as equivalent to integer + nextval default#1
wokalski wants to merge 4 commits into
masterfrom
fix/postgres-serial-nextval-diff

Conversation

@wokalski

Copy link
Copy Markdown

Summary

A column declared as serial in PostgreSQL is semantically identical to an integer column with a DEFAULT nextval('<table>_<column>_seq') clause. The PostgreSQL inspector already normalizes the latter to a SerialType (see columnDefault in inspect.go), but the HCL parser does not — so a state-based diff between the two declarative forms reports a spurious ChangeType + ChangeDefault change.

Example that previously produced a diff but no longer does:

// BEFORE
table "test" {
  schema = schema.test
  column "id" {
    null = false
    type = serial
  }
}

// AFTER
table "test" {
  schema = schema.test
  column "id" {
    null = false
    type = integer
    default = sql("nextval('test.test_id_seq'::regclass)")
  }
}

Approach

Two commits:

  1. d9d7853d — reproduction: adds three subtest cases to TestDiff_TableDiff covering serial ↔ integer + nextval, bigserial ↔ bigint + nextval, and the reversed direction. Without the fix these tests fail with ChangeType | ChangeDefault.

  2. 6404929a — fix: implements sqlx.Normalizer on the postgres diff driver. The Normalize method rewrites any IntegerType column whose Default is a RawExpr matching nextval('<table>_<column>_seq'[::regclass]) into the equivalent SerialType with the default cleared, mirroring the existing inspection-time normalization. Both from and to tables are normalized before diffing, so both declarative forms collapse to the same representation. CockroachDB's crdbDiff still overrides Normalize with its own logic, so it is unaffected.

Also adds an HCL-level end-to-end test TestDiff_SerialVsIntegerNextval exercising the user's exact scenario through the HCL parser.

Test plan

  • go test ./sql/postgres/... — passes including the new tests
  • go test ./... — passes everywhere except atlasexec (pre-existing failure: tests require an atlas CLI binary in $PATH, unrelated)
  • Manual smoke against a real PostgreSQL DB (sequence must exist; this PR only touches the diff path, not validation)

🤖 Generated with Claude Code

wokalski and others added 4 commits May 31, 2026 13:29
A column declared as `serial` is semantically equivalent to an integer
column with a `nextval('<table>_<column>_seq')` default. Switching
between the two declarative forms currently produces a (type + default)
change even though no migration is required.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A column declared as `serial` is semantically identical to an integer
column with a `DEFAULT nextval('<table>_<column>_seq')` clause. The
PostgreSQL inspector already normalizes the latter to a SerialType
(see columnDefault in inspect.go) but the HCL parser does not, so a
state-based diff between the two declarative forms reported a spurious
ChangeType + ChangeDefault change.

Mirror the inspect-time normalization at the diff step by implementing
sqlx.Normalizer on the postgres diff driver so that both forms collapse
to the same representation before comparison.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Drop the misleading HCL-level test that compared `type = serial`
  against `type = integer + nextval default` without a sequence block;
  the AFTER state was incomplete and looked like a sequence drop.
- Add a positive guard for the user-reported case: `serial -> integer`
  with no default must still report `ChangeType` so the migration plan
  can emit `DROP SEQUENCE`.

Verified end-to-end against a live PostgreSQL: inspected serial column
diffed against a programmatic schema with `integer + nextval default +
explicit Sequence object` produces 0 changes, while diffing against
plain `integer` produces `DROP DEFAULT` + `DROP SEQUENCE`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Collapse Normalize doc comment to match diff.go's one-line style.
- Skip *schema.NamedDefault wrappers so a named DEFAULT constraint is
  not silently dropped while normalizing.
- Guard against unrecognized integer T values where SerialType.SetType
  leaves T empty (would otherwise hard-error in FormatType).
- Chain crdbDiff.Normalize into the base differ so the equivalence
  also applies on CockroachDB.
- Drop the verbose test introduction comment; rewrite the misleading
  "drops the sequence" comment to point at the existing plan-level
  assertion in TestPlanChanges ("Drop serial sequence").

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant