Skip to content

Commit f7b3146

Browse files
fix(sqlite): run multi-statement batches in source order (#470)
The SQLite connector split a batch into writes and reads and ran all writes before any reads, so `SELECT ...; DELETE ...; SELECT ...` read the table after the DELETE and returned an empty first result. Run each statement in the order written instead, matching the PostgreSQL connector. The per-statement read-only backstop and maxRows probe are unchanged.
1 parent 116cfdd commit f7b3146

2 files changed

Lines changed: 50 additions & 48 deletions

File tree

‎src/connectors/__tests__/sqlite.integration.test.ts‎

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,7 @@ describe('SQLite Connector Integration Tests', () => {
252252
SELECT COUNT(*) as total FROM users WHERE email LIKE 'multi%';
253253
`, {});
254254

255-
// Two writes then one read, matching both source order and SQLite's
256-
// writes-then-reads execution order here.
255+
// Two writes then one read, in source order.
257256
expect(result.resultSets).toHaveLength(3);
258257
expect(result.resultSets[0]).toEqual({
259258
sql: "INSERT INTO users (name, email, age) VALUES ('Multi User 1', '[email protected]', 30)",
@@ -269,6 +268,30 @@ describe('SQLite Connector Integration Tests', () => {
269268
expect(Number(result.resultSets[2].rows[0].total)).toBe(2);
270269
});
271270

271+
it('should execute multi-statement batches in source order', async () => {
272+
await sqliteTest.connector.executeSQL(`
273+
CREATE TABLE order_check (a INTEGER);
274+
INSERT INTO order_check VALUES (1), (2), (3);
275+
`, {});
276+
277+
const result = await sqliteTest.connector.executeSQL(`
278+
SELECT a AS before_delete FROM order_check ORDER BY a;
279+
DELETE FROM order_check;
280+
SELECT COUNT(*) AS after_delete FROM order_check;
281+
`, {});
282+
283+
expect(result.resultSets.map((rs) => rs.sql)).toEqual([
284+
'SELECT a AS before_delete FROM order_check ORDER BY a',
285+
'DELETE FROM order_check',
286+
'SELECT COUNT(*) AS after_delete FROM order_check',
287+
]);
288+
expect(result.resultSets[0].rows.map((row) => Number(row.before_delete))).toEqual([1, 2, 3]);
289+
expect(result.resultSets[1].rowCount).toBe(3);
290+
expect(Number(result.resultSets[2].rows[0].after_delete)).toBe(0);
291+
292+
await sqliteTest.connector.executeSQL('DROP TABLE order_check', {});
293+
});
294+
272295
it('should handle SQLite foreign key constraints', async () => {
273296
// Enable foreign key constraints
274297
await sqliteTest.connector.executeSQL('PRAGMA foreign_keys = ON', {});
@@ -412,25 +435,23 @@ describe('SQLite Connector Integration Tests', () => {
412435
INSERT INTO users (name, email, age) VALUES ('Multi Test 2', '[email protected]', 35);
413436
`, { maxRows: 1 });
414437

415-
// SQLite's connector executes all write statements first, then all
416-
// read statements, regardless of source order - so resultSets order
417-
// here is [INSERT 'Multi Test 1', INSERT 'Multi Test 2', SELECT],
418-
// not the source order [INSERT, SELECT, INSERT]. Both inserts have
419-
// already run by the time the SELECT executes.
438+
// Statements run in source order [INSERT, SELECT, INSERT], so only the
439+
// first insert has run by the time the SELECT executes.
420440
expect(result.resultSets).toHaveLength(3);
421441
expect(result.resultSets[0]).toEqual({
422442
sql: "INSERT INTO users (name, email, age) VALUES ('Multi Test 1', '[email protected]', 30)",
423443
rows: [],
424444
rowCount: 1,
425445
});
426-
expect(result.resultSets[1]).toEqual({
446+
// Should return only 1 row from the SELECT statement
447+
expect(result.resultSets[1].sql).toBe("SELECT name FROM users WHERE email LIKE '%@test.com' ORDER BY name");
448+
expect(result.resultSets[1].rows).toHaveLength(1);
449+
expect(result.resultSets[1].rows[0].name).toBe('Multi Test 1');
450+
expect(result.resultSets[2]).toEqual({
427451
sql: "INSERT INTO users (name, email, age) VALUES ('Multi Test 2', '[email protected]', 35)",
428452
rows: [],
429453
rowCount: 1,
430454
});
431-
// Should return only 1 row from the SELECT statement
432-
expect(result.resultSets[2].rows).toHaveLength(1);
433-
expect(result.resultSets[2].rows[0].name).toBe('Multi Test 1');
434455
});
435456

436457
});

‎src/connectors/sqlite/index.ts‎

Lines changed: 18 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -541,51 +541,32 @@ export class SQLiteConnector implements Connector {
541541
throw new Error("Parameters are not supported for multi-statement queries in SQLite");
542542
}
543543

544-
// Use native .exec() for optimal performance
545-
// Note: .exec() doesn't return results, so we need to handle SELECT statements differently
546-
const readStatements = [];
547-
const writeStatements = [];
548-
549-
// Separate read and write operations
550-
for (const statement of statements) {
551-
if (this.returnsRows(statement)) {
552-
readStatements.push(statement);
553-
} else {
554-
writeStatements.push(statement);
555-
}
556-
}
557-
558-
// Execute write statements individually to track changes. Each
559-
// statement becomes its own result set, in the order actually run -
560-
// note that's writes-then-reads (see readStatements/writeStatements
561-
// split above), not necessarily the original source order.
544+
// Execute each statement individually, in source order, so a later
545+
// statement sees the effects of earlier ones and result sets line up
546+
// with the batch as written.
562547
const resultSets: SQLResultSet[] = [];
563-
for (const statement of writeStatements) {
548+
for (const statement of statements) {
564549
// Re-assert the read-only backstop before each statement so an earlier
565550
// statement in the batch (e.g. `PRAGMA query_only = OFF` / `query_only(0)`)
566551
// cannot disable it for the ones that follow.
567552
if (options.readonly) {
568553
this.db.exec("PRAGMA query_only = ON");
569554
}
570-
const result = this.prepare(statement).run();
571-
resultSets.push({ sql: statement, rows: [], rowCount: Number(result.changes) });
572-
}
573-
574-
// Execute read statements individually to collect results
575-
for (const statement of readStatements) {
576-
if (options.readonly) {
577-
this.db.exec("PRAGMA query_only = ON");
555+
if (this.returnsRows(statement)) {
556+
// Apply maxRows limit (with a truncation probe row) to SELECT queries if specified
557+
const { sql: processedStatement, probeApplied } = SQLRowLimiter.applyMaxRowsWithTruncationProbe(
558+
statement,
559+
options.maxRows,
560+
"sqlite"
561+
);
562+
const rows = this.prepare(processedStatement).all();
563+
const resultSet: SQLResultSet = { sql: statement, rows, rowCount: rows.length };
564+
SQLRowLimiter.flagTruncation(resultSet, options.maxRows, probeApplied);
565+
resultSets.push(resultSet);
566+
} else {
567+
const result = this.prepare(statement).run();
568+
resultSets.push({ sql: statement, rows: [], rowCount: Number(result.changes) });
578569
}
579-
// Apply maxRows limit (with a truncation probe row) to SELECT queries if specified
580-
const { sql: processedStatement, probeApplied } = SQLRowLimiter.applyMaxRowsWithTruncationProbe(
581-
statement,
582-
options.maxRows,
583-
"sqlite"
584-
);
585-
const rows = this.prepare(processedStatement).all();
586-
const resultSet: SQLResultSet = { sql: statement, rows, rowCount: rows.length };
587-
SQLRowLimiter.flagTruncation(resultSet, options.maxRows, probeApplied);
588-
resultSets.push(resultSet);
589570
}
590571

591572
return { resultSets };

0 commit comments

Comments
 (0)