Skip to content

feat: adopt sqlx::migrate for PostgreSQL catalog and data migrations (ADR-0003) - #221

Open
yesyayen wants to merge 8 commits into
ExtendDB:mainfrom
yesyayen:feat/sqlx-migrate
Open

feat: adopt sqlx::migrate for PostgreSQL catalog and data migrations (ADR-0003)#221
yesyayen wants to merge 8 commits into
ExtendDB:mainfrom
yesyayen:feat/sqlx-migrate

Conversation

@yesyayen

@yesyayen yesyayen commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

What

Replace the homegrown PostgreSQL migration runner with sqlx's built-in migrator
(sqlx::migrate!) for both the catalog and data databases. sqlx checksums each
migration file and refuses to run when an applied file changed. extenddb migrate
now runs both migrators; the catalog_version write moves to a separate step
after the catalog migrator runs. Deletes the schema_history table and the
homegrown runner. Bumps CATALOG_VERSION 0.0.2 -> 0.1.0.

Also adds a stateless unit test that pins each shipped migration's checksum, so
editing an already-shipped (immutable) migration .sql fails the CI test job,
catching the edit at PR time, not just at runtime.

Why

The old runner tracked migrations by filename with no checksum, so editing an
already-shipped migration was a silent no-op that caused schema drift (the
incident behind ADR-0003). sqlx's _sqlx_migrations table closes that gap.

Closes # n/a (tracked by ADR-0003)

Testing done

  • cargo build, cargo fmt --all -- --check, cargo clippy --all-targets -- -D warnings,
    cargo test --workspace all green. Tripwire pins migration counts + CATALOG_VERSION.
  • CI net: migration_checksums_are_pinned pins each migration's SHA-384 checksum.
    Editing an already-shipped migration file changes its checksum and fails
    cargo test --workspace (the GitHub PR test runner, no database required), so an
    edit to an immutable migration is caught in CI, not only at runtime. Verified by
    editing a migration and observing the test flip to FAILED, then reverting.
  • Full lifecycle against local PostgreSQL: init, serve (version gate passes),
    idempotent migrate, version-gate rejection + migrate repair, editing an applied
    migration -> loud checksum failure ("migration 2 was previously applied but has
    been modified"), destroy + re-init. _sqlx_migrations created in both databases;
    schema_history gone.

Review follow-up

Addressed external review of the branch:

  • Pre-sqlx catalog guard (was the one merge-blocking item). A catalog from the
    old runner (has schema_history, no _sqlx_migrations) cannot be adopted in place:
    re-running 001 fails on a non-idempotent CREATE INDEX. Per ADR-0003 the upgrade
    path is destroy + init, so migrate now detects a pre-sqlx catalog up front and
    refuses with that directive (verified: it bails before touching anything, version
    untouched), instead of failing later on cryptic DDL. Added a CLI lifecycle test for it.
  • Reconciled the admin guide's version-mismatch remedy with the upgrade manual
    (destroy + init for the 0.1.0 sqlx adoption).
  • Added the pre-1.0 semver note to the upgrade manual's Version Semantics section.
  • Restored the .sql suffix on the pending-migration report; noted the checksum test's
    coupling to sqlx's SHA-384 algorithm.

Checklist

  • I have read CONTRIBUTING.md
  • All tests pass (cargo test --workspace)
  • Code is formatted (cargo fmt --check)
  • Clippy is clean (cargo clippy -- -W clippy::pedantic)
  • I have added or updated tests for new functionality
  • I have updated documentation if behavior changed
  • Breaking changes are noted below

ADR / RFC: docs/adr/0003-catalog-migration-mechanism.md

Breaking changes

Upgrading from a pre-sqlx catalog requires destroy + init, which drops both
databases (all items wiped, not just schema). Acceptable at v0.1 with no dependent
catalogs. CATALOG_VERSION 0.0.2 -> 0.1.0. Documented in the upgrade manual.

@yesyayen yesyayen changed the title Adopt sqlx::migrate for PostgreSQL catalog and data migrations (ADR-0003) feat: adopt sqlx::migrate for PostgreSQL catalog and data migrations (ADR-0003) Jul 22, 2026
yesyayen added 8 commits July 28, 2026 13:26
@yesyayen
feat(storage-postgres): adopt sqlx::migrate for catalog and data schemas
6a08596 
Replace the homegrown filename-tracked runner with sqlx's migrator for both
the catalog and data databases (ADR-0003). Each database tracks applied
migrations, with per-file checksums, in _sqlx_migrations; editing an applied
migration is now a hard error instead of a silent no-op.

- Enable the sqlx `migrate` feature.
- run_catalog_migrations / run_data_migrations call sqlx::migrate!().run().
- Delete schema_history DDL, CATALOG_MIGRATIONS/DATA_MIGRATIONS, is_migration_applied,
  record_migration; drop BEGIN/COMMIT from migration files (sqlx wraps each in a txn).
- Rehome the catalog_version write to a separate step after the catalog migrator runs.
- migrate runs both migrators unconditionally so checksum validation always fires.
- Bump CATALOG_VERSION to 0.1.0 (breaking: existing catalogs re-init).
- Add .gitattributes pinning *.sql to LF; update the upgrade manual.

Signed-off-by: Anandh Somasundaram <yesyayen@gmail.com>
@yesyayen
test(storage-postgres): tripwire + sqlx-backed CLI lifecycle tests
b933e53 
Add a tripwire pinning the embedded migration counts and CATALOG_VERSION
(ADR-0003). Rewrite the two CLI lifecycle tests that queried the removed
schema_history table to assert on sqlx's _sqlx_migrations ledger.

Signed-off-by: Anandh Somasundaram <yesyayen@gmail.com>
@yesyayen
test(storage-postgres): pin migration checksums to fail CI on edited …
77ff100 
…migrations

A stateless unit test pins each shipped migration's sqlx checksum, so
`cargo test` (the PR runner, no database) fails if an already-applied
migration file is edited. This catches in CI what sqlx otherwise only
enforces at runtime against a live catalog. ADR-0003.

Signed-off-by: Anandh Somasundaram <yesyayen@gmail.com>
@yesyayen
fix(migrate): refuse in-place upgrade of a pre-sqlx catalog
ab257f9 
A catalog created by the old filename-tracked runner (has schema_history,
no _sqlx_migrations) cannot be adopted in place: re-running 001 fails on a
non-idempotent CREATE INDEX. Per ADR-0003 the upgrade path is destroy + init,
so `migrate` now detects a pre-sqlx catalog up front and refuses with that
directive instead of failing later on cryptic DDL. Aligns the admin guide with
the upgrade manual, notes pre-1.0 semver, and restores the .sql suffix on the
pending-migration report.

Signed-off-by: Anandh Somasundaram <yesyayen@gmail.com>
@yesyayen
test(cli): migrate refuses a pre-sqlx catalog with a destroy+init dir…
f333999 
…ective

Signed-off-by: Anandh Somasundaram <yesyayen@gmail.com>
@yesyayen
docs(upgrade): fix rollback dirty-state note; label Version History a…
a130431 
…s changelog

Transactional migrations roll back completely on a mid-apply crash (no dirty
row); re-running migrate retries. Dirty state only applies to -- no-transaction
migrations, of which there are none. Also note Version History is the changelog.

Signed-off-by: Anandh Somasundaram <yesyayen@gmail.com>
@yesyayen
docs: correct ADR rollback dirty-state note; document catalog-scoped …
9d338fd 
…guard

The ADR's operational note said a mid-apply crash leaves a dirty migration;
for transactional migrations sqlx rolls back fully (no dirty row) and re-run
retries. Also note why the pre-sqlx guard checks only the catalog.

Signed-off-by: Anandh Somasundaram <yesyayen@gmail.com>
@yesyayen
test(cli): assert the pre-sqlx guard runs before the migrator connects
ca881a4 
After a refused migrate, _sqlx_migrations must not exist, proving the guard
fired before sqlx could create its ledger.

Signed-off-by: Anandh Somasundaram <yesyayen@gmail.com>
@yesyayen
yesyayen force-pushed the feat/sqlx-migrate branch from b353fd8 to ca881a4 Compare July 28, 2026 20:26
@LeeroyHannigan LeeroyHannigan mentioned this pull request Jul 29, 2026
Open
8 tasks
@robinnsc robinnsc mentioned this pull request Jul 31, 2026
Draft
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@amrith amrith Awaiting requested review from amrith amrith is a code owner
@c33howard c33howard Awaiting requested review from c33howard c33howard is a code owner
@jcshepherd jcshepherd Awaiting requested review from jcshepherd jcshepherd is a code owner
@pdf-amzn pdf-amzn Awaiting requested review from pdf-amzn pdf-amzn is a code owner
@LeeroyHannigan LeeroyHannigan Awaiting requested review from LeeroyHannigan LeeroyHannigan is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@yesyayen