Replace run methods’ Regulator option with QueryOptions struct

I’m going to want to pass an additional option to the various run
methods.  The Regulator is already optional, so I might as well
prepare the code to take all of the options as a bundle.

Author: justin-mp

src/workerd/api/actor-state.c++

@@ -712,7 +712,8 @@ jsg::JsRef<jsg::JsValue> DurableObjectStorage::transactionSync(
     //   depth to each savepoint name like I originally thought. We should refactor this -- and use
     //   prepared statements.
 
-    sqlite.run(SqliteDatabase::TRUSTED, kj::str("SAVEPOINT _cf_sync_savepoint_", depth));
+    sqlite.run(
+        {.regulator = SqliteDatabase::TRUSTED}, kj::str("SAVEPOINT _cf_sync_savepoint_", depth));
     return js.tryCatch([&]() {
       auto result = callback(js);
 
@@ -721,14 +722,17 @@ jsg::JsRef<jsg::JsValue> DurableObjectStorage::transactionSync(
       JSG_REQUIRE(!sqlite.observedCriticalError(), Error,
           "Cannot commit transaction due to an earlier SQL critical error");
 
-      sqlite.run(SqliteDatabase::TRUSTED, kj::str("RELEASE _cf_sync_savepoint_", depth));
+      sqlite.run(
+          {.regulator = SqliteDatabase::TRUSTED}, kj::str("RELEASE _cf_sync_savepoint_", depth));
       return kj::mv(result);
     }, [&](jsg::Value exception) -> jsg::JsRef<jsg::JsValue> {
       // If a critical error forced an automatic rollback, we skip the rollback and release
       // attempt, because savepoints should already be released.
       if (!sqlite.observedCriticalError()) {
-        sqlite.run(SqliteDatabase::TRUSTED, kj::str("ROLLBACK TO _cf_sync_savepoint_", depth));
-        sqlite.run(SqliteDatabase::TRUSTED, kj::str("RELEASE _cf_sync_savepoint_", depth));
+        sqlite.run({.regulator = SqliteDatabase::TRUSTED},
+            kj::str("ROLLBACK TO _cf_sync_savepoint_", depth));
+        sqlite.run(
+            {.regulator = SqliteDatabase::TRUSTED}, kj::str("RELEASE _cf_sync_savepoint_", depth));
       }
       js.throwException(kj::mv(exception));
     });

src/workerd/api/sql.c++

@@ -83,7 +83,7 @@ SqlStorage::IngestResult SqlStorage::ingest(jsg::Lock& js, kj::String querySql)
 
 void SqlStorage::setMaxPageCountForTest(jsg::Lock& js, int count) {
   auto& db = getDb(js);
-  db.run(SqliteDatabase::TRUSTED, kj::str("PRAGMA max_page_count = ", count));
+  db.run({.regulator = SqliteDatabase::TRUSTED}, kj::str("PRAGMA max_page_count = ", count));
 }
 
 jsg::Ref<SqlStorage::Statement> SqlStorage::prepare(jsg::Lock& js, jsg::JsString query) {
@@ -156,7 +156,7 @@ SqlStorage::Cursor::State::State(SqliteDatabase& db,
     kj::StringPtr sqlCode,
     kj::Array<BindingValue> bindingsParam)
     : bindings(kj::mv(bindingsParam)),
-      query(db.run(regulator, sqlCode, mapBindings(bindings).asPtr())) {}
+      query(db.run({.regulator = regulator}, sqlCode, mapBindings(bindings).asPtr())) {}
 
 SqlStorage::Cursor::State::State(
     kj::Rc<CachedStatement> cachedStatementParam, kj::Array<BindingValue> bindingsParam)

src/workerd/io/actor-sqlite.c++

@@ -115,7 +115,8 @@ ActorSqlite::ExplicitTxn::ExplicitTxn(ActorSqlite& actorSqlite): actorSqlite(act
   // Unfortunately this means we cannot prepare the statement, unless we prepare a series of
   // statements for each depth. (Actually, it could be reasonable to prepare statements for
   // depth 0 specifically, but I'm not going to try it for now.)
-  actorSqlite.db->run(SqliteDatabase::TRUSTED, kj::str("SAVEPOINT _cf_savepoint_", depth));
+  actorSqlite.db->run(
+      {.regulator = SqliteDatabase::TRUSTED}, kj::str("SAVEPOINT _cf_savepoint_", depth));
 }
 ActorSqlite::ExplicitTxn::~ExplicitTxn() noexcept(false) {
   [&]() noexcept {
@@ -158,7 +159,8 @@ kj::Maybe<kj::Promise<void>> ActorSqlite::ExplicitTxn::commit() {
     precommitAlarmState = actorSqlite.startPrecommitAlarmScheduling();
   }
 
-  actorSqlite.db->run(SqliteDatabase::TRUSTED, kj::str("RELEASE _cf_savepoint_", depth));
+  actorSqlite.db->run(
+      {.regulator = SqliteDatabase::TRUSTED}, kj::str("RELEASE _cf_savepoint_", depth));
   committed = true;
 
   KJ_IF_SOME(p, parent) {
@@ -192,8 +194,10 @@ kj::Promise<void> ActorSqlite::ExplicitTxn::rollback() {
 }
 
 void ActorSqlite::ExplicitTxn::rollbackImpl() noexcept(false) {
-  actorSqlite.db->run(SqliteDatabase::TRUSTED, kj::str("ROLLBACK TO _cf_savepoint_", depth));
-  actorSqlite.db->run(SqliteDatabase::TRUSTED, kj::str("RELEASE _cf_savepoint_", depth));
+  actorSqlite.db->run(
+      {.regulator = SqliteDatabase::TRUSTED}, kj::str("ROLLBACK TO _cf_savepoint_", depth));
+  actorSqlite.db->run(
+      {.regulator = SqliteDatabase::TRUSTED}, kj::str("RELEASE _cf_savepoint_", depth));
   KJ_IF_SOME(p, parent) {
     alarmDirty = p->alarmDirty;
   } else {
@@ -483,19 +487,23 @@ kj::Maybe<kj::Promise<void>> ActorSqlite::put(kj::Array<KeyValuePair> pairs, Wri
       txn->commit();
     } else {
       // If we are in a transaction, let's just set a SAVEPOINT that we can rollback to if needed.
-      db->run(SqliteDatabase::TRUSTED, kj::str("SAVEPOINT _cf_put_multiple_savepoint"));
+      db->run(
+          {.regulator = SqliteDatabase::TRUSTED}, kj::str("SAVEPOINT _cf_put_multiple_savepoint"));
       for (const auto& pair: pairs) {
         try {
           kv.put(pair.key, pair.value);
         } catch (kj::Exception& e) {
           // We need to rollback to the putMultiple SAVEPOINT. Do it, and then release the SAVEPOINT.
-          db->run(SqliteDatabase::TRUSTED, kj::str("ROLLBACK TO _cf_put_multiple_savepoint"));
-          db->run(SqliteDatabase::TRUSTED, kj::str("RELEASE _cf_put_multiple_savepoint"));
+          db->run({.regulator = SqliteDatabase::TRUSTED},
+              kj::str("ROLLBACK TO _cf_put_multiple_savepoint"));
+          db->run({.regulator = SqliteDatabase::TRUSTED},
+              kj::str("RELEASE _cf_put_multiple_savepoint"));
           kj::throwFatalException(kj::mv(e));
         }
       }
       // We're done here. RELEASE the savepoint.
-      db->run(SqliteDatabase::TRUSTED, kj::str("RELEASE _cf_put_multiple_savepoint"));
+      db->run(
+          {.regulator = SqliteDatabase::TRUSTED}, kj::str("RELEASE _cf_put_multiple_savepoint"));
     }
   } else {
     for (auto& pair: pairs) {

src/workerd/util/sqlite-test.c++

@@ -482,7 +482,7 @@ RowCounts countRowsTouched(SqliteDatabase& db,
   uint64_t rowsFound = 0;
 
   // Runs a query; retrieves and discards all the data.
-  auto query = db.run(regulator, sqlCode, bindParams...);
+  auto query = db.run({.regulator = regulator}, sqlCode, bindParams...);
   while (!query.isDone()) {
     rowsFound++;
     query.nextRow();
@@ -749,10 +749,10 @@ KJ_TEST("SQLite row counters with triggers") {
 
   // A deletion incurs two writes: one for the row and one for the log.
   {
-    db.run(regulator, "DELETE FROM things");
-    db.run(regulator, "INSERT INTO things (id) VALUES (1)");
-    db.run(regulator, "INSERT INTO things (id) VALUES (2)");
-    db.run(regulator, "INSERT INTO things (id) VALUES (3)");
+    db.run({.regulator = regulator}, "DELETE FROM things");
+    db.run({.regulator = regulator}, "INSERT INTO things (id) VALUES (1)");
+    db.run({.regulator = regulator}, "INSERT INTO things (id) VALUES (2)");
+    db.run({.regulator = regulator}, "INSERT INTO things (id) VALUES (3)");
 
     RowCounts stats = countRowsTouched(db, regulator, "DELETE FROM things");
     KJ_EXPECT(stats.written == 6);
@@ -859,9 +859,9 @@ KJ_TEST("SQLite observer addQueryStats") {
   int rowsWrittenBefore = sqliteObserver.rowsWritten;
   constexpr int dbRowCount = 3;
   {
-    db.run(regulator, "INSERT INTO things (id) VALUES (10)");
-    db.run(regulator, "INSERT INTO things (id) VALUES (11)");
-    db.run(regulator, "INSERT INTO things (id) VALUES (12)");
+    db.run({.regulator = regulator}, "INSERT INTO things (id) VALUES (10)");
+    db.run({.regulator = regulator}, "INSERT INTO things (id) VALUES (11)");
+    db.run({.regulator = regulator}, "INSERT INTO things (id) VALUES (12)");
   }
   KJ_EXPECT(sqliteObserver.rowsRead - rowsReadBefore == dbRowCount);
   KJ_EXPECT(sqliteObserver.rowsWritten - rowsWrittenBefore == dbRowCount);
@@ -906,7 +906,7 @@ KJ_TEST("SQLite observer addQueryStats") {
   rowsReadBefore = sqliteObserver.rowsRead;
   rowsWrittenBefore = sqliteObserver.rowsWritten;
   {
-    auto query = db.run(regulator, "INSERT INTO things (id) VALUES (100)");
+    auto query = db.run({.regulator = regulator}, "INSERT INTO things (id) VALUES (100)");
     db.reset();
   }
   KJ_EXPECT(sqliteObserver.rowsRead - rowsReadBefore == 1);
@@ -1552,7 +1552,7 @@ KJ_TEST("I/O exceptions pass through SQLite") {
   SqliteDatabase::Vfs vfs(*dir);
   SqliteDatabase db(vfs, kj::Path({"db"}), kj::WriteMode::CREATE | kj::WriteMode::MODIFY);
 
-  db.run(SqliteDatabase::TRUSTED, kj::str(R"(
+  db.run({.regulator = SqliteDatabase::TRUSTED}, kj::str(R"(
     CREATE TABLE IF NOT EXISTS things (
       id INTEGER PRIMARY KEY AUTOINCREMENT,
       value INTEGER
@@ -1564,7 +1564,8 @@ KJ_TEST("I/O exceptions pass through SQLite") {
   KJ_ASSERT_NONNULL(dir->dbFile)->error = KJ_EXCEPTION(FAILED, "test-vfs-error");
 
   // It should pass through.
-  KJ_EXPECT_THROW_MESSAGE("test-vfs-error", db.run(SqliteDatabase::TRUSTED, kj::str(R"(
+  KJ_EXPECT_THROW_MESSAGE(
+      "test-vfs-error", db.run({.regulator = SqliteDatabase::TRUSTED}, kj::str(R"(
     INSERT INTO things(value) VALUES (456);
   )")));
 }
@@ -1613,9 +1614,9 @@ KJ_TEST("SQLite critical error handling for SQLITE_IOERR") {
   });
 
   // Use a small cache size to force flushing to disk on even a small write
-  db.run(SqliteDatabase::TRUSTED, "PRAGMA cache_size = 1");  // 1 page cache
+  db.run({.regulator = SqliteDatabase::TRUSTED}, "PRAGMA cache_size = 1");  // 1 page cache
 
-  db.run(SqliteDatabase::TRUSTED, kj::str(R"(
+  db.run({.regulator = SqliteDatabase::TRUSTED}, kj::str(R"(
     CREATE TABLE IF NOT EXISTS things (
       id INTEGER PRIMARY KEY AUTOINCREMENT,
       value INTEGER
@@ -1629,7 +1630,8 @@ KJ_TEST("SQLite critical error handling for SQLITE_IOERR") {
   KJ_ASSERT_NONNULL(dir->dbFile)->error = KJ_EXCEPTION(FAILED, "test-vfs-error");
 
   KJ_EXPECT(!db.observedCriticalError());
-  KJ_EXPECT_THROW_MESSAGE("test-vfs-error", db.run(SqliteDatabase::TRUSTED, kj::str(R"(
+  KJ_EXPECT_THROW_MESSAGE(
+      "test-vfs-error", db.run({.regulator = SqliteDatabase::TRUSTED}, kj::str(R"(
     INSERT INTO things(value) VALUES (456);
   )")));
 
@@ -1655,7 +1657,8 @@ KJ_TEST("SQLite critical error handling for SQLITE_FULL") {
     auto largeData = kj::heapArray<byte>(100000, 'X');  // 100KB
 
     // This should eventually trigger SQLITE_FULL
-    db.run(SqliteDatabase::TRUSTED, "INSERT INTO test_full VALUES (?, ?)", 1, largeData.asPtr());
+    db.run({.regulator = SqliteDatabase::TRUSTED}, "INSERT INTO test_full VALUES (?, ?)", 1,
+        largeData.asPtr());
   });
 }
 
@@ -1676,7 +1679,8 @@ KJ_TEST("SQLite critical error handling for SQLITE_NOMEM") {
     // Create data that will exceed the memory limit
     auto largeData = kj::heapArray<byte>(50000, 'X');  // 50KB
 
-    db.run(SqliteDatabase::TRUSTED, "INSERT INTO test_nomem VALUES (?, ?)", 2, largeData.asPtr());
+    db.run({.regulator = SqliteDatabase::TRUSTED}, "INSERT INTO test_nomem VALUES (?, ?)", 2,
+        largeData.asPtr());
   });
 }
 

src/workerd/util/sqlite.c++

@@ -847,7 +847,7 @@ SqliteDatabase::IngestResult SqliteDatabase::ingestSql(
     // Slice off the next valid statement SQL
     auto nextStatement = kj::str(sqlCode.first(statementLength));
     // Create a Query object, which will prepare & execute it
-    auto q = Query(*this, regulator, nextStatement);
+    auto q = Query(*this, QueryOptions{.regulator = regulator}, nextStatement);
 
     rowsRead += q.getRowsRead();
     rowsWritten += q.getRowsWritten();
@@ -1340,11 +1340,11 @@ void SqliteDatabase::Statement::beforeSqliteReset() {
 }
 
 SqliteDatabase::Query::Query(SqliteDatabase& db,
-    const Regulator& regulator,
+    QueryOptions options,
     Statement& statement,
     kj::ArrayPtr<const ValuePtr> bindings)
     : ResetListener(db),
-      regulator(regulator),
+      regulator(options.regulator),
       maybeStatement(statement.prepareForExecution()),
       queryEvent(this->db.sqliteObserver) {
   // If we throw from the constructor, the destructor won't run. Need to call destroy() explicitly.
@@ -1353,11 +1353,11 @@ SqliteDatabase::Query::Query(SqliteDatabase& db,
 }
 
 SqliteDatabase::Query::Query(SqliteDatabase& db,
-    const Regulator& regulator,
+    QueryOptions options,
     kj::StringPtr sqlCode,
     kj::ArrayPtr<const ValuePtr> bindings)
     : ResetListener(db),
-      regulator(regulator),
+      regulator(options.regulator),
       ownStatement(db.prepareSql(regulator, sqlCode, 0, MULTI)),
       maybeStatement(ownStatement),
       queryEvent(this->db.sqliteObserver) {

src/workerd/util/sqlite.h

@@ -72,6 +72,11 @@ class SqliteDatabase {
   class Lock;
   class LockManager;
   struct VfsOptions;
+  class Regulator;
+
+  struct QueryOptions {
+    const Regulator& regulator;
+  };
 
   struct IngestResult {
     kj::StringPtr remainder;
@@ -180,7 +185,7 @@ class SqliteDatabase {
   //   `Query` object are both associated with the last statement. This is particularly convenient
   //   for doing database initialization such as creating several tables at once.
   template <typename... Params>
-  Query run(const Regulator& regulator, kj::StringPtr sqlCode, Params&&... bindings);
+  Query run(QueryOptions options, kj::StringPtr sqlCode, Params&&... bindings);
 
   template <size_t size>
   Statement prepare(const char (&sqlCode)[size]);
@@ -630,17 +635,17 @@ class SqliteDatabase::Query final: private ResetListener {
   friend class SqliteDatabase;
 
   Query(SqliteDatabase& db,
-      const Regulator& regulator,
+      QueryOptions options,
       Statement& statement,
       kj::ArrayPtr<const ValuePtr> bindings);
   Query(SqliteDatabase& db,
-      const Regulator& regulator,
+      QueryOptions options,
       kj::StringPtr sqlCode,
       kj::ArrayPtr<const ValuePtr> bindings);
   template <typename... Params>
-  Query(SqliteDatabase& db, const Regulator& regulator, Statement& statement, Params&&... bindings)
+  Query(SqliteDatabase& db, QueryOptions options, Statement& statement, Params&&... bindings)
       : ResetListener(db),
-        regulator(regulator),
+        regulator(options.regulator),
         maybeStatement(statement.prepareForExecution()),
         queryEvent(this->db.sqliteObserver) {
     // If we throw from the constructor, the destructor won't run. Need to call destroy()
@@ -649,9 +654,9 @@ class SqliteDatabase::Query final: private ResetListener {
     bindAll(std::index_sequence_for<Params...>(), kj::fwd<Params>(bindings)...);
   }
   template <typename... Params>
-  Query(SqliteDatabase& db, const Regulator& regulator, kj::StringPtr sqlCode, Params&&... bindings)
+  Query(SqliteDatabase& db, QueryOptions options, kj::StringPtr sqlCode, Params&&... bindings)
       : ResetListener(db),
-        regulator(regulator),
+        regulator(options.regulator),
         ownStatement(db.prepareSql(regulator, sqlCode, 0, MULTI)),
         maybeStatement(ownStatement),
         queryEvent(this->db.sqliteObserver) {
@@ -952,18 +957,18 @@ class SqliteDatabase::Lock {
 
 template <typename... Params>
 SqliteDatabase::Query SqliteDatabase::run(
-    const Regulator& regulator, kj::StringPtr sqlCode, Params&&... params) {
-  return Query(*this, regulator, sqlCode, kj::fwd<Params>(params)...);
+    QueryOptions options, kj::StringPtr sqlCode, Params&&... params) {
+  return Query(*this, options, sqlCode, kj::fwd<Params>(params)...);
 }
 
 template <typename... Params>
 SqliteDatabase::Query SqliteDatabase::Statement::run(Params&&... params) {
-  return Query(db, regulator, *this, kj::fwd<Params>(params)...);
+  return Query(db, QueryOptions{.regulator = regulator}, *this, kj::fwd<Params>(params)...);
 }
 
 template <size_t size, typename... Params>
 SqliteDatabase::Query SqliteDatabase::run(const char (&sqlCode)[size], Params&&... params) {
-  return Query(*this, TRUSTED, sqlCode, kj::fwd<Params>(params)...);
+  return Query(*this, QueryOptions{.regulator = TRUSTED}, sqlCode, kj::fwd<Params>(params)...);
 }
 
 template <size_t size>