diff --git a/src/workerd/api/sql-test.js b/src/workerd/api/sql-test.js index 6f66014c570..6f436ff733c 100644 --- a/src/workerd/api/sql-test.js +++ b/src/workerd/api/sql-test.js @@ -1294,6 +1294,40 @@ export class DurableObjectExample extends DurableObject { } async alarm() {} + + async testMultiStatement() { + // Performing this PRAGMA will cause sqlite to invalidate prepared statements and re-compile + // them the next time they are executed. (Probably, many other pragmas would have the same + // effect, but this is the one that we observed causing issues.) + // + // In particular, the prepared statement ActorSqlite::beginTxn, which is simply + // `BEGIN TRANSACTION`, will be invalidated and recomplied on the next invocation. + // + // When we perform our multi-statement exec below, the first line will invoke the + // `ActorSqlite::onWrite` callback, which will invoke `beginTxn`. Because `BEGIN TRANSACTION` + // must be recompiled, the SQLite authorizer callback will be invoked to check if it is + // authorized. But we use the authorizer callback to detect when SQLite has parsed a statement + // as a transaction statement. At one point, we had a bug where we incorrectly thought that + // the authorizer was being called on behalf of the statement we were trying to parse and + // execute, namely, `CREATE TABLE items...`. We therefore incorrectly made note that this + // statement was beginning a transaction. This led the transaction state tracking to become + // all wrong! + // + // This only turned out to be an issue when performing a multi-statement exec(), because in + // this case all statements except the last are executed inside the parse loop, which is why + // we misinterpreted the authorizer callback. + this.state.storage.sql.exec("PRAGMA case_sensitive_like = TRUE"); + + let cursor = this.state.storage.sql.exec(` + CREATE TABLE items(i INTEGER, s TEXT); + CREATE INDEX itemsIdx ON items(s); + INSERT INTO items VALUES (123, "abc"); + INSERT INTO items VALUES (456, "def"); + SELECT i FROM items WHERE s = "abc"; + `); + + assert.deepEqual([...cursor], [{i: 123}]); + } } export default { @@ -1380,6 +1414,13 @@ export let testRollbackKvInit = { }, }; +export let testMultiStatement = { + async test(ctrl, env, ctx) { + let stub = env.ns.get(env.ns.idFromName('multi-statement-test')); + await stub.testMultiStatement(); + }, +}; + const INSERT_36_ROWS = ['a', 'b', 'c', 'd', 'e', 'f'] .map( (prefix) => diff --git a/src/workerd/util/sqlite.c++ b/src/workerd/util/sqlite.c++ index f0f2e451bd8..43cf226e308 100644 --- a/src/workerd/util/sqlite.c++ +++ b/src/workerd/util/sqlite.c++ @@ -613,9 +613,11 @@ SqliteDatabase::StatementAndEffect SqliteDatabase::prepareSql( KJ_IF_SOME(cb, onWriteCallback) { if (!sqlite3_stmt_readonly(result)) { // The callback is allowed to invoke queries of its own, so we have to un-set the - // regulator while we call it. + // regulator and parse context while we call it. currentRegulator = kj::none; KJ_DEFER(currentRegulator = regulator); + currentParseContext = kj::none; + KJ_DEFER(currentParseContext = parseContext); cb(); } }