migrate.php: transaction wrapping around DDL is illusory, failed migration can wedge the runner #30

New Issue

2026-08-31T21:29:40-04:00

jared commented

2026-08-31 21:29:40 -04:00

Severity: High

migrations/migrate.php (~lines 118-160) wraps each migration file's statements in begin_transaction()/rollback(), but MySQL DDL statements (ALTER TABLE, CREATE TABLE, etc.) cause an implicit commit — so a rollback does not undo earlier DDL statements already executed within that same file if a later statement in the file fails.

Impact: A migration that fails partway leaves the DB partially altered and unrecorded in the migrations table (the INSERT INTO migrations only runs after all statements succeed). The next run then retries the whole file from statement 1 — including the already-applied DDL — which can now fail with "column already exists"/"constraint already exists" style errors that aren't on the safe-to-ignore allowlist (only Duplicate key name/index-already exists is currently allowlisted). This can wedge the runner in a permanently-failing state requiring manual DB surgery.

Fix: Either split multi-statement migrations so each DDL statement is individually idempotent/safe-to-retry (matching the 000_baseline.sql philosophy), or track applied statements at finer granularity than whole-file, or explicitly document that migrations must be written idempotently since transactional rollback can't be relied on for DDL.

**Severity:** High `migrations/migrate.php` (~lines 118-160) wraps each migration file's statements in `begin_transaction()`/`rollback()`, but MySQL DDL statements (`ALTER TABLE`, `CREATE TABLE`, etc.) cause an implicit commit — so a rollback does **not** undo earlier DDL statements already executed within that same file if a later statement in the file fails. **Impact:** A migration that fails partway leaves the DB partially altered *and* unrecorded in the `migrations` table (the `INSERT INTO migrations` only runs after all statements succeed). The next run then retries the whole file from statement 1 — including the already-applied DDL — which can now fail with "column already exists"/"constraint already exists" style errors that aren't on the safe-to-ignore allowlist (only `Duplicate key name`/index-`already exists` is currently allowlisted). This can wedge the runner in a permanently-failing state requiring manual DB surgery. **Fix:** Either split multi-statement migrations so each DDL statement is individually idempotent/safe-to-retry (matching the `000_baseline.sql` philosophy), or track applied statements at finer granularity than whole-file, or explicitly document that migrations must be written idempotently since transactional rollback can't be relied on for DDL.