From 757166b43a2b066e55a3468ab483bb0afedfcfff Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 27 Sep 2026 07:47:20 +0200 Subject: [PATCH] test(database): require one SQL statement per exec during initialization The performance capture counts one exec call as one statement, because SQL cannot be split reliably in the counter (trigger bodies contain semicolons). The historical-upgrade driver now wraps exec on every connection initDatabase opens and fails on a batch, so that counting assumption holds for the fresh profile and all historical schemas. Documents the definition in the counter and the architecture docs. Co-Authored-By: Claude Opus 5.5 --- .../database-worker-sql-statement-count.ts | 9 ++++-- docs/architecture/performance-journeys.md | 4 ++- docs/architecture/sqlite-db-worker.md | 4 ++- .../src/lib/testing/connection-upgrade.ts | 32 +++++++++++++++++++ 4 files changed, 45 insertions(+), 4 deletions(-) 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'); }