Remove shared prepared statement cache (Refs #76)

src/lib/server/db/index.ts

@@ -645,21 +645,15 @@ export function db(): Database.Database {
         throw err;
     }
 
-    // Cache only runtime queries, after migrations have finished. Statements
-    // belong to this connection, so reopening the DB starts with an empty cache.
-    // Callers share statement options (e.g. raw()); use consistent options for
-    // identical SQL and pass values to run/get/all rather than binding permanently.
-    const statements = new Map<string, Database.Statement>();
+    // Instrument runtime queries after migrations. Each prepare must return a
+    // fresh statement: modes, permanent bindings, and active iterators are mutable
+    // state owned by the caller, even when another caller uses identical SQL.
     const slowMs = Number(process.env.EC_SLOW_SQL_MS ?? 0);
-    const prepare = handle.prepare.bind(handle);
-    handle.prepare = ((sql: string) => {
-        let statement = statements.get(sql);
-        if (!statement) {
-            statement = timeStatement(prepare(sql), sql, slowMs);
-            statements.set(sql, statement);
-        }
-        return statement;
-    }) as typeof handle.prepare;
+    if (Number.isFinite(slowMs) && slowMs > 0) {
+        const prepare = handle.prepare.bind(handle);
+        handle.prepare = ((sql: string) =>
+            timeStatement(prepare(sql), sql, slowMs)) as typeof handle.prepare;
+    }
 
     _db = handle;
     optimizeTimer = setInterval(() => {

tests/unit/db-statements.test.ts

@@ -8,9 +8,10 @@ beforeEach(async () => {
 });
 afterEach(async () => {
     await fixture.cleanup();
+    vi.unstubAllEnvs();
 });
 
-describe('prepared statement caching', () => {
+describe('database statements', () => {
     it('tunes the controller connection and stops periodic optimization when closed', async () => {
         const { db, closeDb } = await import('../../src/lib/server/db/index.js');
         vi.useFakeTimers({ toFake: ['setInterval', 'clearInterval'] });
@@ -30,7 +31,7 @@ describe('prepared statement caching', () => {
             vi.useRealTimers();
         }
     });
-    it('reuses statements with fresh parameters and remains usable after rollback', async () => {
+    it('keeps caller-owned statements reusable with fresh parameters after rollback', async () => {
         const { db } = await import('../../src/lib/server/db/index.js');
         const insertSql = 'INSERT INTO settings (key, value) VALUES (?, ?)';
         const selectSql = 'SELECT value FROM settings WHERE key = ?';
@@ -45,8 +46,8 @@ describe('prepared statement caching', () => {
         ).toThrow('rollback');
         db().prepare(insertSql).run('cache-second', 'two');
 
-        expect(db().prepare(insertSql)).toBe(insert);
-        expect(db().prepare(selectSql)).toBe(select);
+        expect(db().prepare(insertSql)).not.toBe(insert);
+        expect(db().prepare(selectSql)).not.toBe(select);
         expect(select.get('cache-first')).toEqual({ value: 'one' });
         expect(select.get('cache-second')).toEqual({ value: 'two' });
         expect(select.get('cache-rollback')).toBeUndefined();
@@ -65,6 +66,56 @@ describe('prepared statement caching', () => {
         expect(statement).not.toBe(oldStatement);
         expect(statement.database).not.toBe(oldStatement.database);
         expect(statement.get('cache-persisted')).toEqual({ value: 'saved' });
-        expect(db().prepare(sql)).toBe(statement);
+        expect(db().prepare(sql)).not.toBe(statement);
+    });
+
+    describe.each([0, 20])('statement isolation with EC_SLOW_SQL_MS=%i', (slowMs) => {
+        beforeEach(() => vi.stubEnv('EC_SLOW_SQL_MS', String(slowMs)));
+
+        it.each([
+            ['raw', [1]],
+            ['pluck', 1],
+            ['expand', { $: { value: 1 } }],
+            ['safeIntegers', { value: 1n }]
+        ] as const)('keeps %s mode local to its statement', async (mode, expected) => {
+            const { db } = await import('../../src/lib/server/db/index.js');
+            const sql = 'SELECT 1 AS value';
+            const customized = db().prepare(sql)[mode]();
+            const standard = db().prepare(sql);
+
+            expect(standard.get()).toEqual({ value: 1 });
+            expect(customized.get()).toEqual(expected);
+            customized[mode](false);
+            standard[mode]();
+            expect(customized.get()).toEqual({ value: 1 });
+            expect(standard.get()).toEqual(expected);
+            expect(db().prepare(sql).get()).toEqual({ value: 1 });
+        });
+
+        it('keeps permanent bindings local to their statement', async () => {
+            const { db } = await import('../../src/lib/server/db/index.js');
+            const sql = 'SELECT ? AS value';
+            const bound = db().prepare(sql).bind('first');
+
+            expect(db().prepare(sql).get('second')).toEqual({ value: 'second' });
+            expect(bound.get()).toEqual({ value: 'first' });
+            expect(db().prepare(sql).bind('third').get()).toEqual({ value: 'third' });
+            expect(bound.get()).toEqual({ value: 'first' });
+        });
+
+        it('allows identical queries while another statement is being iterated', async () => {
+            const { db } = await import('../../src/lib/server/db/index.js');
+            const sql = 'SELECT 1 AS value UNION ALL SELECT 2 AS value';
+            const iterator = db().prepare(sql).iterate();
+            try {
+                expect(iterator.next()).toEqual({ value: { value: 1 }, done: false });
+                expect(db().prepare(sql).get()).toEqual({ value: 1 });
+                expect([...db().prepare(sql).iterate()]).toEqual([{ value: 1 }, { value: 2 }]);
+                expect(iterator.next()).toEqual({ value: { value: 2 }, done: false });
+                expect(iterator.next().done).toBe(true);
+            } finally {
+                iterator.return?.();
+            }
+        });
     });
 });

tests/unit/host-observation-retention.test.ts

@@ -328,9 +328,9 @@ describe('routine host observation retention', () => {
         ).toEqual({ count: 2000 });
     });
 
-    it('reuses the same three delete statements across different batch sizes', async () => {
+    it('uses the same three delete query shapes across different batch sizes', async () => {
         const api = await setup();
-        const statements = new Map<string, ReturnType<typeof api.handle.prepare>>();
+        const deleteQueries = new Set<string>();
         for (const count of [1, 2, 7, 50, 500]) {
             api.handle.transaction(() => {
                 const insert = api.handle.prepare(`INSERT INTO ec_host_operations
@@ -346,13 +346,9 @@ describe('routine host observation retention', () => {
                 .filter((sql) => sql.startsWith('DELETE FROM ec_host_'));
             prepare.mockRestore();
             expect(deletes).toHaveLength(3);
-            for (const sql of deletes) {
-                const statement = api.handle.prepare(sql);
-                if (statements.has(sql)) expect(statement).toBe(statements.get(sql));
-                statements.set(sql, statement);
-            }
+            for (const sql of deletes) deleteQueries.add(sql);
         }
-        expect(statements.size).toBe(3);
+        expect(deleteQueries.size).toBe(3);
         expect(api.handle.pragma('foreign_key_check')).toEqual([]);
     });