diff --git a/apps/electron-backend/src/app/workers/database-worker-sql-statement-count.ts b/apps/electron-backend/src/app/workers/database-worker-sql-statement-count.ts index ff5f57c87..ddb1e7091 100644 --- a/apps/electron-backend/src/app/workers/database-worker-sql-statement-count.ts +++ b/apps/electron-backend/src/app/workers/database-worker-sql-statement-count.ts @@ -18,8 +18,13 @@ import type BetterSqlite3 from 'better-sqlite3'; * two to four times slower and would distort the import benchmarks that run * with the same flag. One call of `run`, `get`, `all` or `iterate` that * returns normally is one statement, which includes pragmas and the - * BEGIN/COMMIT that `db.transaction()` prepares internally; one `exec` call - * counts as one. Calls that throw are not counted: the SQL trace skips the + * BEGIN/COMMIT that `db.transaction()` prepares internally. One `exec` call + * also counts as one: SQL cannot be split into statements reliably here + * (trigger bodies contain semicolons), so callers pass one statement per + * call. The database worker never calls `exec`, and the shared connection's + * historical-upgrade test (`libs/shared/database/src/lib/testing/ + * connection-upgrade.ts`) fails on a batch. Calls that throw are not + * counted: the SQL trace skips the * ones that fail before execution (a migration's `ALTER TABLE` for a column * that already exists), and they did no work. */ diff --git a/docs/architecture/performance-journeys.md b/docs/architecture/performance-journeys.md index 25ae25c4d..cc06bb3f4 100644 --- a/docs/architecture/performance-journeys.md +++ b/docs/architecture/performance-journeys.md @@ -124,7 +124,9 @@ ordered against the worker's responses, not against wall-clock: statements whose count is still in flight when `ready-to-show` is dispatched are not. One call of `run`, `get`, `all`, `iterate` or `exec` that returns normally is one statement; on the launch workloads this matches the number of SQL trace -lines exactly. +lines exactly. An `exec` with several statements would count as one, so the +shared connection passes one statement per call, and its historical-upgrade +test fails on a batch. `main.sqlStatementsBeforeReadyToShow` is not yet deterministic. The main thread runs the shared connection's schema creation and migrations (about 90 diff --git a/docs/architecture/sqlite-db-worker.md b/docs/architecture/sqlite-db-worker.md index 187c8b55e..7fcae41c1 100644 --- a/docs/architecture/sqlite-db-worker.md +++ b/docs/architecture/sqlite-db-worker.md @@ -455,7 +455,9 @@ execution methods of better-sqlite3's `Statement` prototype and the connection's `exec`, not through the `verbose` callback: a callback makes better-sqlite3 expand every statement's SQL, which made bulk inserts two to four times slower. A call that throws is not counted, which matches the SQL -trace for statements that fail before execution. The main process counts its +trace for statements that fail before execution. One `exec` call counts as +one statement, so initialization passes one statement per call; the +historical-upgrade test enforces it. The main process counts its own shared connection the same way (`services/main-sql-statement-count.ts`, through the shared library's connection observer). Without the flag both connections are opened unchanged. See diff --git a/libs/shared/database/src/lib/testing/connection-upgrade.ts b/libs/shared/database/src/lib/testing/connection-upgrade.ts index 94cdaa19a..dcfaa1c47 100644 --- a/libs/shared/database/src/lib/testing/connection-upgrade.ts +++ b/libs/shared/database/src/lib/testing/connection-upgrade.ts @@ -3,6 +3,7 @@ import { readFileSync } from 'node:fs'; import Database from 'better-sqlite3'; import { getTableConfig } from 'drizzle-orm/sqlite-core'; import { closeDatabase, getDatabasePath, initDatabase } from '../connection'; +import { setDatabaseConnectionObserver } from '../connection-observer'; import * as currentSchema from '../schema'; const tables = [ @@ -38,6 +39,31 @@ function seed(sqlite: Database.Database) { } } +/** + * The performance capture counts one `exec` call as one SQL statement + * (apps/electron-backend/src/app/workers/database-worker-sql-statement-count.ts), + * so initialization must pass exactly one statement per `exec`. better-sqlite3 + * refuses to prepare a string with a second statement; other prepare errors + * are left to `exec` itself (for example an idempotent ALTER TABLE). + */ +function requireSingleStatementExec(): string[] { + const batches: string[] = []; + setDatabaseConnectionObserver((connection) => { + const exec = connection.exec; + connection.exec = function singleStatementExec(sql: string) { + try { + connection.prepare(sql); + } catch (error) { + if (error instanceof RangeError) { + batches.push(sql.replace(/\s+/g, ' ').slice(0, 120)); + } + } + return exec.call(this, sql); + }; + }); + return batches; +} + function snapshot(sqlite: Database.Database) { return tables.map((table) => { const columns = sqlite.pragma(`table_info(${table})`) as { @@ -98,6 +124,7 @@ async function main() { console.warn = (...args) => warnings.push(args); console.log = () => undefined; const databasePath = getDatabasePath(); + const execBatches = requireSingleStatementExec(); if (fixture === 'fresh') { await initDatabase(); @@ -157,6 +184,11 @@ async function main() { [], 'Initialization must not silently skip failed migrations' ); + assert.deepEqual( + execBatches, + [], + 'Initialization must pass one SQL statement per exec call' + ); process.stdout.write('upgrade verified'); }