Skip to content

A declared CHECK never round-trips: migrate proposes dropping and re-adding it every run #24

Description

@jryannel

A hand-written schema.Check never round-trips, so sqlb migrate proposes dropping and re-adding it on every run.

Found by running sqlb migrate -check ./taskschema (#22) against example/tasks, which is the first thing to compare a declared schema against an introspected one end to end.

What happens

example/tasks/taskschema/schema.go:226 declares:

Check("done_tasks_have_a_completion_time",
    "status <> 'done' OR completed_at IS NOT NULL")

Replaying the committed history into a shadow database and diffing against the declaration gives a migration that is a no-op with two statements in it:

-- +goose Up
ALTER TABLE "tasks" DROP CONSTRAINT "done_tasks_have_a_completion_time";
ALTER TABLE "tasks" ADD CONSTRAINT "done_tasks_have_a_completion_time"
    CHECK (status <> 'done' OR completed_at IS NOT NULL);

-- +goose Down
ALTER TABLE "tasks" DROP CONSTRAINT "done_tasks_have_a_completion_time";
ALTER TABLE "tasks" ADD CONSTRAINT "done_tasks_have_a_completion_time"
    CHECK (((status <> 'done'::text) OR (completed_at IS NOT NULL)));

The Down section is the tell: that is what pg_get_constraintdef hands back. Postgres normalises the expression — full parenthesisation, and an explicit ::text cast on the literal — and stores the normalised form. It never returns the string that was written.

Why

migrate/diff.go:672 compares constraint definitions as raw strings:

if t.def == curCons[name].def {
    continue
}

introspect reads def from pg_get_constraintdef (introspect/introspect.go:170), so current always holds Postgres's spelling and target always holds the author's. For a CHECK they are never equal, so the constraint is dropped and re-added — forever, on every run, and the ADD is flagged LOCK ACCESS EXCLUSIVE because revalidating it scans the table.

Scope

Enums are unaffected. example/tasks declares three (role, status, priority) and none of them appears in the diff, so whatever ADR-0017 emits for an enum already matches what comes back. This is specific to schema.Check with a hand-written expression — which is the one case where the text is the author's rather than sqlb's.

Foreign keys, unique constraints and primary keys are also unaffected in this example.

Why it matters

It is not cosmetic once sqlb migrate exists. sqlb migrate -check is meant to be the answer to "has the schema moved ahead of the history", and on any schema with a hand-written CHECK the answer is permanently yes. That makes it unusable as a gate for exactly the projects most likely to want one, and worse, it trains people to ignore what it reports.

It also means migrate.Diff against an introspected registry — the ADR-0014 adoption path, sqlb import — has been producing this churn all along. Nothing surfaced it because nothing else compares a declared CHECK with an introspected one.

Possible directions

  • Normalise before comparing. Cheap version: strip redundant parentheses, drop ::type casts on literals, collapse whitespace. Fragile, and wrong in the interesting cases — but it covers the common ones and is honest if it is documented as a heuristic.
  • Ask Postgres. Round-trip the declared expression through the shadow database and compare pg_get_constraintdef with pg_get_constraintdef, so both sides are in Postgres's spelling. Correct by construction, and it costs a database — which sqlb migrate already has open at that moment, though migrate.Diff as a pure function does not.
  • Compare structurally. Parse both expressions. The most correct and by far the most work, and ADR-0005 has already declined to own a SQL parser.
  • Store the normalised form at declaration time, by having schema.Check record what Postgres would return. Only possible with a database in hand, so it collapses into the second option.

Worth being explicit that the second option is the one that fits what exists: the shadow database is already connected when this comparison happens.

Reproducing

docker run --rm -d -p 55433:5432 -e POSTGRES_PASSWORD=x postgres:18-alpine
cd example/tasks
SQLB_SHADOW_DSN='postgres://postgres:x@localhost:55433/postgres?sslmode=disable' \
  go run github.com/jryannel/sqlb/cmd/sqlb migrate -dry-run ./taskschema

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions