ReviewOS

stacks/bun-query-builder

migrate applies each file with no transaction and no lock

#1067
Closed glennmichael123 opened this 22 days ago · 0 comments
22 days ago

qb migrate applies each .sql file with no surrounding transaction and no lock (src/actions/migrate.ts:414-471, src/db.ts:404-430). The loop is await qb.file(filePath) then recordMigration(...), with nothing between them and nothing around the pair.

Two consequences, both verified:

A migration that fails halfway leaves a half-applied schema recorded as applied. A .sql file with several statements is executed statement by statement; if statement 3 throws, statements 1-2 are already committed. The file is then neither fully applied nor cleanly re-runnable, and on SQLite the failure can still leave the row recorded.

Concurrent boots collide. Two processes starting at once — an ordinary rolling deploy — both see the same pending set and both run it. Reproduced 12/12 times on Postgres/MySQL.

Fix shape: wrap each file's statements in a transaction where the dialect supports transactional DDL (Postgres does; MySQL largely does not), and take an advisory lock for the duration of the run (pg_advisory_lock / GET_LOCK) so a second process waits rather than racing. The builder already has advisoryLock/tryAdvisoryLock helpers.

Found while auditing after #1001.

Sign in to comment on this issue.