Foreign keys apply to every connection, not just the lucky one (#163) #165

Merged

qwc merged 1 commit from fix/sqlite-pragmas-per-connection into main

2026-09-09 14:26:11 +02:00

qwc commented

2026-09-09 14:09:01 +02:00

Owner

Closes #163.

The root cause of the production instance that would not start after the 0.9.0 upgrade.

What was wrong

db.MustExec("PRAGMA foreign_keys=ON")

db is a *sqlx.DB — a pool. The pragma reached whichever connection served it and every connection opened afterwards kept SQLite's default of foreign keys off. Since foreign_keys is per-connection, enforcement was a coin toss.

ProjectStore.Delete is a bare DELETE FROM projects WHERE id = ? that relies entirely on cascade. Served by one of those connections, it deleted the project and silently left its versions, upload logs, access rows and API tokens behind. Nothing reads orphan rows, so this was invisible for months — until migration 011 had to rebuild project_access, and rebuilds do enforce constraints. The INSERT failed, the transaction rolled back, and the database was left marked dirty at 11 with the instance unable to start.

The fix

Both pragmas move into the DSN. The driver applies them in its connection-open path (applyQueryParams, vendor/modernc.org/sqlite/conn.go:74), so they hold for every connection the pool ever creates. A DSN that already sets one keeps the operator's value.

Startup now also asserts foreign keys are genuinely on and refuses to run otherwise. That is deliberate: the failure being replaced was silent for months and only surfaced as a broken upgrade months later. A loud startup error is much the better trade.

journal_mode is a property of the database file rather than the connection, so repeating it per connection is a no-op after the first. It travels along to keep both settings in one place.

The regression test

TestSQLiteCascadesOnEveryConnection holds four connections open at once and asserts a cascade actually fires on each — the real behaviour, not the pragma value read back.

Confirmed to fail against the old code before being kept. It reproduces production exactly:

connection 2: cascade did not fire, 1 orphaned child rows left behind
connection 3: cascade did not fire, 1 orphaned child rows left behind
connection 4: cascade did not fire, 1 orphaned child rows left behind

Connection 1 — the one that got the pragma — passes. That is the whole bug in three lines of output.

TestSQLiteDSN covers the DSN rewriting, including leaving an operator's own pragma alone and handing an unparseable query to the driver untouched rather than burying its error message.

Why staging did not catch it

Staging took the same upgrade and ran through it without trouble. That is consistent with the diagnosis rather than against it: the failure needs data, not just code. A database only carries these orphans if a project was deleted on one of the connections that happened to have foreign keys off, and staging had no such deletion history — so 011 found nothing to violate the constraint and applied cleanly.

Worth remembering for the next migration that rebuilds a table: a green staging run says nothing about production when the failure depends on accumulated data.

Notes for review

This does not repair existing databases. Production was fixed by hand (the SQL is in #163). Any other SQLite install that ran 0.8.x and has deleted a project may carry the same orphans; PRAGMA foreign_key_check; reports them. A migration cannot help, because a database stuck dirty at 11 never reaches one.
Built-in docs updated in the same PR: the SQLite DSN section of reference/configuration.md.
#164 covers the other half found along the way — deleting a project never removed its files from disk.

🤖 Written with Claude Code

Closes #163. The root cause of the production instance that would not start after the 0.9.0 upgrade. ## What was wrong ```go db.MustExec("PRAGMA foreign_keys=ON") ``` `db` is a `*sqlx.DB` — a pool. The pragma reached whichever connection served it and every connection opened afterwards kept SQLite's default of foreign keys **off**. Since `foreign_keys` is per-connection, enforcement was a coin toss. `ProjectStore.Delete` is a bare `DELETE FROM projects WHERE id = ?` that relies entirely on cascade. Served by one of those connections, it deleted the project and silently left its versions, upload logs, access rows and API tokens behind. Nothing reads orphan rows, so this was invisible for months — until migration 011 had to rebuild `project_access`, and rebuilds *do* enforce constraints. The INSERT failed, the transaction rolled back, and the database was left marked dirty at 11 with the instance unable to start. ## The fix Both pragmas move into the DSN. The driver applies them in its connection-open path (`applyQueryParams`, `vendor/modernc.org/sqlite/conn.go:74`), so they hold for every connection the pool ever creates. A DSN that already sets one keeps the operator's value. Startup now also asserts foreign keys are genuinely on and refuses to run otherwise. That is deliberate: the failure being replaced was silent for months and only surfaced as a broken upgrade months later. A loud startup error is much the better trade. `journal_mode` is a property of the database file rather than the connection, so repeating it per connection is a no-op after the first. It travels along to keep both settings in one place. ## The regression test `TestSQLiteCascadesOnEveryConnection` holds four connections open at once and asserts a cascade actually fires on each — the real behaviour, not the pragma value read back. Confirmed to fail against the old code before being kept. It reproduces production exactly: ``` connection 2: cascade did not fire, 1 orphaned child rows left behind connection 3: cascade did not fire, 1 orphaned child rows left behind connection 4: cascade did not fire, 1 orphaned child rows left behind ``` Connection 1 — the one that got the pragma — passes. That is the whole bug in three lines of output. `TestSQLiteDSN` covers the DSN rewriting, including leaving an operator's own pragma alone and handing an unparseable query to the driver untouched rather than burying its error message. ## Why staging did not catch it Staging took the same upgrade and ran through it without trouble. That is consistent with the diagnosis rather than against it: the failure needs *data*, not just code. A database only carries these orphans if a project was deleted on one of the connections that happened to have foreign keys off, and staging had no such deletion history — so 011 found nothing to violate the constraint and applied cleanly. Worth remembering for the next migration that rebuilds a table: a green staging run says nothing about production when the failure depends on accumulated data. ## Notes for review - **This does not repair existing databases.** Production was fixed by hand (the SQL is in #163). Any other SQLite install that ran 0.8.x *and has deleted a project* may carry the same orphans; `PRAGMA foreign_key_check;` reports them. A migration cannot help, because a database stuck dirty at 11 never reaches one. - Built-in docs updated in the same PR: the SQLite DSN section of `reference/configuration.md`. - #164 covers the other half found along the way — deleting a project never removed its files from disk. 🤖 Written with [Claude Code](https://claude.com/claude-code)

qwc added 1 commit

2026-09-09 14:09:01 +02:00

Foreign keys apply to every connection, not just the lucky one (#163 )

CI / test (pull_request) Successful in 1m30s

Details

CI / build (pull_request) Successful in 53s

Details

CI / docker (pull_request) Has been skipped

Details

fc67d20b89

`PRAGMA foreign_keys=ON` was executed against the *sqlx.DB returned by
Open. That is a pool, not a connection: the pragma reached whichever
connection happened to serve it, and every connection the pool opened
afterwards kept SQLite's default of foreign keys OFF.

So enforcement was a coin toss. A project deleted on one of those
connections ran `DELETE FROM projects` with nothing to cascade, and its
versions, upload logs, access rows and API tokens stayed behind. The rows
were invisible until a migration had to rebuild a table -- rebuilds do
enforce the constraints -- at which point the upgrade failed with
FOREIGN KEY constraint failed and rolled back, leaving the database
marked dirty and the instance unable to start.

Both pragmas move into the DSN, which the driver applies in its
connection-open path, so they hold for every connection the pool ever
creates. A DSN that already sets one keeps the operator's value.

Startup now also asserts that foreign keys are actually on and refuses to
run if they are not. The failure this replaces was silent for months and
only surfaced as a broken upgrade; a loud startup error is the right
trade.

The regression test holds four connections open at once and checks that a
cascade fires on each. Against the old code connections 2, 3 and 4 all
leave orphans behind.

Assisted-by: Claude Opus 5

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Marcel M. Otte <marcel.otte@mmo.to>

qwc referenced this pull request

2026-09-09 14:12:18 +02:00

Deleting a project takes its documentation with it (#164) #166

qwc merged commit f9ed31d02b into main

2026-09-09 14:26:11 +02:00

qwc deleted branch fix/sqlite-pragmas-per-connection

2026-09-09 14:26:11 +02:00

qwc referenced this pull request from a commit

2026-09-09 14:26:12 +02:00

Merge pull request 'Foreign keys apply to every connection, not just the lucky one (#163)' (#165) from fix/sqlite-pragmas-per-connection into main