bun:sqlite: count only the rows a statement changes itself in changes

docs/runtime/sqlite.mdx

@@ -350,7 +350,7 @@ query.run();
 
 Internally, this calls [`sqlite3_reset`](https://www.sqlite.org/capi3ref.html#sqlite3_reset) and calls [`sqlite3_step`](https://www.sqlite.org/capi3ref.html#sqlite3_step) once. Stepping through all the rows is not necessary when you don't care about the results.
 
-The `lastInsertRowid` property is the ID of the last row inserted into the database. The `changes` property is the number of rows affected by the query.
+The `lastInsertRowid` property is the ID of the last row inserted into the database. The `changes` property is the number of rows that the query inserts, updates, or deletes itself. Rows that a trigger or a foreign key action such as `ON DELETE CASCADE` changes do not count. A schema statement such as `CREATE VIRTUAL TABLE` can report rows that a virtual table module, for example FTS5, writes to its own tables.
 
 ### `.as(Class)` - Map query results to a class
 

packages/bun-types/sqlite.d.ts

@@ -1309,7 +1309,13 @@ declare module "bun:sqlite" {
    */
   export interface Changes {
     /**
-     * The number of rows changed by the last `run` or `exec` call.
+     * The number of rows that the last `run` or `exec` call inserted, updated, or deleted itself.
+     *
+     * Rows that a trigger or a foreign key action such as `ON DELETE CASCADE` changes do not count.
+     * {@link Database.run} with several statements in one string reports the sum of the statements.
+     *
+     * One limit: a schema statement such as `CREATE VIRTUAL TABLE` can report rows that a virtual
+     * table module (for example FTS5) writes to its own tables while the statement runs.
      */
     changes: number;
 

src/jsc/bindings/sqlite/JSSQLStatement.cpp

@@ -550,6 +550,8 @@ class JSSQLStatement : public JSC::JSDestructibleObject {
     // Created by db.query(); close(false) finalizes these but leaves db.prepare() statements usable.
     bool ownedByDatabase : 1 = false;
     bool finalizedByClose : 1 = false;
+    // An INSERT, UPDATE or DELETE: every run sets sqlite3_changes64(), so run() reads no other counter.
+    bool countsChanges : 1 = false;
 
 protected:
     JSSQLStatement(JSC::Structure* structure, JSDOMGlobalObject& globalObject, sqlite3_stmt* stmt, VersionSqlite3* version_db, int64_t memorySizeChange = 0)
@@ -1469,6 +1471,12 @@ static bool isSkippedInSQLiteQuery(const char c)
     return c == ' ' || c == ';' || (c >= '\t' && c <= '\r');
 }
 
+// sqlite3_changes64() persists across statements, so only a statement that moved the total reports it (#43306).
+static sqlite3_int64 directChangesSince(sqlite3* db, sqlite3_int64 totalChangesBefore)
+{
+    return sqlite3_total_changes64(db) == totalChangesBefore ? 0 : sqlite3_changes64(db);
+}
+
 // This runs a query one-off
 // without the overhead of a long-lived statement object
 // does not return anything
@@ -1541,7 +1549,7 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementExecuteFunction, (JSC::JSGlobalObject * l
     bool strict = internalFlagsValue.isInt32() && (internalFlagsValue.asInt32() & kStrictFlag) != 0;
     bool safeIntegers = internalFlagsValue.isInt32() && (internalFlagsValue.asInt32() & kSafeIntegersFlag) != 0;
 
-    const int total_changes_before = sqlite3_total_changes(db);
+    sqlite3_int64 changes = 0;
 
     while (sqlStringHead && sqlStringHead < end) {
         if (isSkippedInSQLiteQuery(*sqlStringHead)) [[unlikely]] {
@@ -1599,10 +1607,17 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementExecuteFunction, (JSC::JSGlobalObject * l
             didSetBindings = true;
         }
 
+        // COMMIT, SAVEPOINT and RELEASE are read-only but can flush FTS5, which moves sqlite3_total_changes64().
+        const bool canChangeRows = !sqlite3_stmt_readonly(sql.stmt);
+        const sqlite3_int64 total_changes_before = canChangeRows ? sqlite3_total_changes64(db) : 0;
+
         do {
             rc = sqlite3_step(sql.stmt);
         } while (rc == SQLITE_ROW);
 
+        if (canChangeRows)
+            changes += directChangesSince(db, total_changes_before);
+
         didExecuteAny = true;
         sqlStringHead = tail;
     }
@@ -1618,9 +1633,8 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementExecuteFunction, (JSC::JSGlobalObject * l
     }
 
     if (auto* diff = dynamicDowncast<JSC::InternalFieldTuple>(diffValue)) {
-        const int total_changes_after = sqlite3_total_changes(db);
         int64_t last_insert_rowid = sqlite3_last_insert_rowid(db);
-        diff->putInternalField(vm, 0, JSC::jsNumber(total_changes_after - total_changes_before));
+        diff->putInternalField(vm, 0, JSC::jsNumber(changes));
         if (safeIntegers) {
             auto* bigInt = JSBigInt::createFrom(lexicalGlobalObject, last_insert_rowid);
             RETURN_IF_EXCEPTION(scope, {});
@@ -2630,10 +2644,13 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementExecuteStatementFunctionRun, (JSC::JSGlob
     }
 
     auto* db = sqlite3_db_handle(stmt);
-    int total_changes_before = sqlite3_total_changes(db);
+    const bool canChangeRows = !sqlite3_stmt_readonly(stmt);
+    // Each counter read locks the connection mutex, so only a statement of unknown kind reads the total.
+    const bool checkTotal = canChangeRows && !castedThis->countsChanges;
+    const sqlite3_int64 total_changes_before = checkTotal ? sqlite3_total_changes64(db) : 0;
 
     int status = sqlite3_step(stmt);
-    if (!sqlite3_stmt_readonly(stmt)) {
+    if (canChangeRows) {
         castedThis->version_db->version++;
     }
 
@@ -2656,9 +2673,16 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementExecuteStatementFunctionRun, (JSC::JSGlob
     }
 
     if (auto* diff = dynamicDowncast<JSC::InternalFieldTuple>(diffValue)) {
-        const int total_changes_after = sqlite3_total_changes(db);
         int64_t last_insert_rowid = sqlite3_last_insert_rowid(db);
-        diff->putInternalField(vm, 0, JSC::jsNumber(total_changes_after - total_changes_before));
+        sqlite3_int64 changes = 0;
+        if (castedThis->countsChanges) {
+            changes = sqlite3_changes64(db);
+        } else if (checkTotal) {
+            changes = directChangesSince(db, total_changes_before);
+            // SQLite rejects bound parameters in DDL and PRAGMA, so a statement that has them and changed rows is DML.
+            castedThis->countsChanges = changes != 0 && sqlite3_bind_parameter_count(stmt) > 0;
+        }
+        diff->putInternalField(vm, 0, JSC::jsNumber(changes));
         if (castedThis->useBigInt64) {
             JSValue lastRowIdBigInt = JSBigInt::createFrom(lexicalGlobalObject, last_insert_rowid);
             RETURN_IF_EXCEPTION(scope, {});

src/jsc/bindings/sqlite/lazy_sqlite3.h

@@ -59,6 +59,7 @@ typedef int (*lazy_sqlite3_finalize_type)(sqlite3_stmt* pStmt);
 typedef void (*lazy_sqlite3_free_type)(void*);
 typedef int (*lazy_sqlite3_get_autocommit_type)(sqlite3*);
 typedef int (*lazy_sqlite3_total_changes_type)(sqlite3*);
+typedef sqlite3_int64 (*lazy_sqlite3_total_changes64_type)(sqlite3*);
 typedef int (*lazy_sqlite3_config_type)(int, ...);
 typedef int (*lazy_sqlite3_open_v2_type)(const char* filename, sqlite3** ppDb, int flags, const char* zVfs);
 typedef int (*lazy_sqlite3_prepare_v2_type)(sqlite3* db, const char* zSql, int nByte, sqlite3_stmt** ppStmt, const char** pzTail);
@@ -185,6 +186,7 @@ inline lazy_sqlite3_extended_errcode_type lazy_sqlite3_extended_errcode;
 inline lazy_sqlite3_error_offset_type lazy_sqlite3_error_offset;
 inline lazy_sqlite3_bind_parameter_name_type lazy_sqlite3_bind_parameter_name;
 inline lazy_sqlite3_total_changes_type lazy_sqlite3_total_changes;
+inline lazy_sqlite3_total_changes64_type lazy_sqlite3_total_changes64;
 inline lazy_sqlite3_last_insert_rowid_type lazy_sqlite3_last_insert_rowid;
 inline lazy_sqlite3_exec_type lazy_sqlite3_exec;
 inline lazy_sqlite3_limit_type lazy_sqlite3_limit;
@@ -281,6 +283,7 @@ inline lazy_sqlite3changeset_apply_type lazy_sqlite3changeset_apply;
 #define sqlite3_error_offset lazy_sqlite3_error_offset
 #define sqlite3_bind_parameter_name lazy_sqlite3_bind_parameter_name
 #define sqlite3_total_changes lazy_sqlite3_total_changes
+#define sqlite3_total_changes64 lazy_sqlite3_total_changes64
 #define sqlite3_last_insert_rowid lazy_sqlite3_last_insert_rowid
 #define sqlite3_exec lazy_sqlite3_exec
 #define sqlite3_limit lazy_sqlite3_limit
@@ -433,6 +436,7 @@ inline int lazyLoadSQLiteUnlocked(WTF::String* errorMessage = nullptr)
     lazy_sqlite3_error_offset = (lazy_sqlite3_error_offset_type)dlsym(sqlite3_handle, "sqlite3_error_offset");
     lazy_sqlite3_bind_parameter_name = (lazy_sqlite3_bind_parameter_name_type)dlsym(sqlite3_handle, "sqlite3_bind_parameter_name");
     lazy_sqlite3_total_changes = (lazy_sqlite3_total_changes_type)dlsym(sqlite3_handle, "sqlite3_total_changes");
+    lazy_sqlite3_total_changes64 = (lazy_sqlite3_total_changes64_type)dlsym(sqlite3_handle, "sqlite3_total_changes64");
     lazy_sqlite3_last_insert_rowid = (lazy_sqlite3_last_insert_rowid_type)dlsym(sqlite3_handle, "sqlite3_last_insert_rowid");
     lazy_sqlite3_exec = (lazy_sqlite3_exec_type)dlsym(sqlite3_handle, "sqlite3_exec");
     lazy_sqlite3_limit = (lazy_sqlite3_limit_type)dlsym(sqlite3_handle, "sqlite3_limit");
@@ -506,6 +510,12 @@ inline int lazyLoadSQLiteUnlocked(WTF::String* errorMessage = nullptr)
             return static_cast<sqlite3_int64>(lazy_sqlite3_changes(db));
         };
     }
+    // sqlite3_total_changes64 is also 3.37.0+.
+    if (!lazy_sqlite3_total_changes64) {
+        lazy_sqlite3_total_changes64 = [](sqlite3* db) -> sqlite3_int64 {
+            return static_cast<sqlite3_int64>(lazy_sqlite3_total_changes(db));
+        };
+    }
 
     return 0;
 }

test/js/bun/sqlite/sqlite.test.js

@@ -2146,6 +2146,196 @@ it("reports changes in Statement#run", () => {
   expect(db.query(sql).run().changes).toBe(2);
 });
 
+describe("changes counts only the rows that the statement changes itself", () => {
+  const entryPoints = {
+    "Database#run": (db, sql, ...params) => db.run(sql, ...params),
+    "Database#exec": (db, sql, ...params) => db.exec(sql, ...params),
+    "Statement#run from prepare()": (db, sql, ...params) => {
+      using stmt = db.prepare(sql);
+      return stmt.run(...params);
+    },
+    "Statement#run from query()": (db, sql, ...params) => db.query(sql).run(...params),
+  };
+
+  describe.each(Object.entries(entryPoints))("%s", (_, run) => {
+    it("not the shadow tables of a virtual table", () => {
+      using db = new Database(":memory:");
+      db.exec("CREATE VIRTUAL TABLE ft USING fts5(a, b); CREATE VIRTUAL TABLE rt USING rtree(id, x0, x1);");
+
+      expect({
+        fts5Insert: run(db, "INSERT INTO ft VALUES ('alpha beta', 'gamma')").changes,
+        fts5InsertTwoRows: run(db, "INSERT INTO ft VALUES (?, ?), (?, ?)", "a", "b", "c", "d").changes,
+        fts5UpdateTwoRows: run(db, "UPDATE ft SET b = 'delta' WHERE rowid <= 2").changes,
+        fts5DeleteThreeRows: run(db, "DELETE FROM ft").changes,
+        rtreeInsert: run(db, "INSERT INTO rt VALUES (1, 0, 1)").changes,
+        rtreeDelete: run(db, "DELETE FROM rt WHERE id = 1").changes,
+      }).toEqual({
+        fts5Insert: 1,
+        fts5InsertTwoRows: 2,
+        fts5UpdateTwoRows: 2,
+        fts5DeleteThreeRows: 3,
+        rtreeInsert: 1,
+        rtreeDelete: 1,
+      });
+    });
+
+    it("not the rows that a trigger writes", () => {
+      using db = new Database(":memory:");
+      db.exec(`
+        CREATE TABLE t (a);
+        CREATE TABLE log (x);
+        CREATE TRIGGER tr AFTER INSERT ON t BEGIN
+          INSERT INTO log VALUES (new.a);
+          INSERT INTO log VALUES (-new.a);
+        END;
+      `);
+
+      expect(run(db, "INSERT INTO t VALUES (1)").changes).toBe(1);
+      expect(run(db, "INSERT INTO t VALUES (?), (?)", 2, 3).changes).toBe(2);
+      expect(db.query("SELECT x FROM log ORDER BY rowid").values()).toEqual([[1], [-1], [2], [-2], [3], [-3]]);
+    });
+
+    it("not the rows that a foreign key action deletes", () => {
+      using db = new Database(":memory:");
+      db.exec(`
+        PRAGMA foreign_keys = ON;
+        CREATE TABLE parent (id INTEGER PRIMARY KEY);
+        CREATE TABLE child (id INTEGER PRIMARY KEY, parent_id REFERENCES parent (id) ON DELETE CASCADE);
+        INSERT INTO parent VALUES (1), (2);
+        INSERT INTO child VALUES (10, 1), (11, 1), (12, 1), (20, 2);
+      `);
+
+      expect(run(db, "DELETE FROM parent WHERE id = 1").changes).toBe(1);
+      expect(db.query("SELECT id FROM child").values()).toEqual([[20]]);
+    });
+
+    // https://www.sqlite.org/c3ref/changes.html: an INSTEAD OF trigger makes the change, so the statement changes no row.
+    it("not the rows that an INSTEAD OF trigger writes for a view", () => {
+      using db = new Database(":memory:");
+      db.exec(`
+        CREATE TABLE t (a);
+        CREATE VIEW v AS SELECT a FROM t;
+        CREATE TRIGGER vi INSTEAD OF INSERT ON v BEGIN
+          INSERT INTO t VALUES (new.a);
+        END;
+      `);
+
+      expect(run(db, "INSERT INTO v VALUES (1)").changes).toBe(0);
+      expect(db.query("SELECT a FROM t").values()).toEqual([[1]]);
+    });
+
+    it("0 for a statement that changes no row, not the count of the write before it", () => {
+      using db = new Database(":memory:");
+      db.exec("CREATE TABLE t (a)");
+
+      // Each statement runs right after a write of two rows, the count that sqlite3_changes() still holds.
+      const afterWrite = sql => {
+        expect(run(db, "INSERT INTO t VALUES (1), (2)").changes).toBe(2);
+        return run(db, sql).changes;
+      };
+      expect({
+        select: afterWrite("SELECT a FROM t"),
+        createTable: afterWrite("CREATE TABLE other (a)"),
+        begin: afterWrite("BEGIN"),
+        commit: afterWrite("COMMIT"),
+        updateNoRow: afterWrite("UPDATE t SET a = 0 WHERE 0"),
+      }).toEqual({ select: 0, createTable: 0, begin: 0, commit: 0, updateNoRow: 0 });
+    });
+
+    // FTS5 keeps the rows of a transaction in memory. SAVEPOINT, RELEASE and COMMIT make it write them to its shadow tables.
+    it("0 for a SAVEPOINT, RELEASE or COMMIT that makes FTS5 write its pending rows", () => {
+      using db = new Database(":memory:");
+      db.exec("CREATE VIRTUAL TABLE ft USING fts5(a, b)");
+      const insert = () => expect(run(db, "INSERT INTO ft VALUES ('alpha beta', 'gamma')").changes).toBe(1);
+
+      run(db, "BEGIN");
+      insert();
+      const savepoint = run(db, "SAVEPOINT s1").changes;
+      insert();
+      run(db, "SAVEPOINT s2");
+      insert();
+      const release = run(db, "RELEASE s1").changes;
+      insert();
+      const commit = run(db, "COMMIT").changes;
+      expect({ savepoint, release, commit }).toEqual({ savepoint: 0, release: 0, commit: 0 });
+    });
+  });
+
+  it.each(["run", "exec"])("Database#%s does not add the FTS5 writes of a COMMIT in the same string", method => {
+    using db = new Database(":memory:");
+    db.exec("CREATE VIRTUAL TABLE ft USING fts5(a, b)");
+
+    const inserts = "INSERT INTO ft VALUES ('a', 'b'); INSERT INTO ft VALUES ('c', 'd'), ('e', 'f');";
+    expect({
+      autocommit: db[method](inserts).changes,
+      transaction: db[method](`BEGIN; ${inserts} COMMIT;`).changes,
+    }).toEqual({ autocommit: 3, transaction: 3 });
+  });
+
+  it.each(["run", "exec"])("Database#%s adds up the statements of a multi-statement string", method => {
+    using db = new Database(":memory:");
+    db.exec(`
+      CREATE TABLE t (a);
+      CREATE TABLE log (x);
+      CREATE TRIGGER tr AFTER INSERT ON t BEGIN
+        INSERT INTO log VALUES (new.a);
+      END;
+    `);
+
+    // The SELECT and the CREATE TABLE change no row, so they must not add the count of the INSERT before them again.
+    const sql = "INSERT INTO t VALUES (1); INSERT INTO t VALUES (2), (3); SELECT a FROM t; CREATE TABLE other (a);";
+    expect(db[method](sql).changes).toBe(3);
+    expect(db[method]("SELECT 1; SELECT 2;").changes).toBe(0);
+    expect(db.query("SELECT x FROM log ORDER BY rowid").values()).toEqual([[1], [2], [3]]);
+  });
+
+  // With foreign keys on, DROP TABLE runs a DELETE that SQLite counts. The second DROP is a no-op and sets no counter.
+  it.each(["prepare", "query"])("a reused DROP TABLE Statement from %s() reports 0 when it is a no-op", method => {
+    using db = new Database(":memory:");
+    db.exec(`
+      PRAGMA foreign_keys = ON;
+      CREATE TABLE parent (id INTEGER PRIMARY KEY);
+      CREATE TABLE child (id INTEGER PRIMARY KEY, parent_id REFERENCES parent (id) ON DELETE CASCADE);
+      CREATE TABLE t (a);
+      INSERT INTO parent VALUES (1), (2), (3);
+      INSERT INTO child VALUES (10, 1);
+    `);
+    using drop = db[method]("DROP TABLE IF EXISTS parent");
+
+    expect([
+      drop.run().changes,
+      db.run("INSERT INTO t VALUES (1), (2), (3), (4), (5)").changes,
+      drop.run().changes,
+    ]).toEqual([3, 5, 0]);
+  });
+
+  // A Statement with bound parameters that changed rows once reads only sqlite3_changes64() on later runs.
+  it.each(["prepare", "query"])("a reused Statement from %s() reports its own rows on every run", method => {
+    using db = new Database(":memory:");
+    db.exec(`
+      CREATE TABLE t (id INTEGER PRIMARY KEY, a);
+      CREATE TABLE log (x);
+      CREATE TRIGGER tr AFTER UPDATE ON t BEGIN
+        INSERT INTO log VALUES (new.id);
+      END;
+    `);
+    using insert = db[method]("INSERT INTO t (a) VALUES (?), (?), (?)");
+    using update = db[method]("UPDATE t SET a = ? WHERE id <= ?");
+    using select = db[method]("SELECT count(*) FROM t");
+
+    expect([
+      update.run("x", 0).changes,
+      insert.run(1, 2, 3).changes,
+      update.run("x", 2).changes,
+      select.run().changes,
+      insert.run(4, 5, 6).changes,
+      update.run("y", 0).changes,
+      select.run().changes,
+      update.run("y", 1).changes,
+    ]).toEqual([0, 3, 2, 0, 3, 0, 0, 1]);
+  });
+});
+
 it("#13082", async () => {
   async function run(op) {
     const stmt = (() => {

test/js/sql/sqlite-sql.test.ts

@@ -2727,6 +2727,27 @@ describe("Triggers and Views", () => {
     const alerts2 = await sql`SELECT * FROM reorder_alerts`;
     expect(alerts2).toHaveLength(1);
   });
+
+  test("count is the rows that the statement changes itself, not the rows that a trigger or FTS5 writes", async () => {
+    await sql`CREATE TABLE t (a)`;
+    await sql`CREATE TABLE log (x)`;
+    await sql`CREATE TRIGGER tr AFTER INSERT ON t BEGIN
+      INSERT INTO log VALUES (new.a);
+      INSERT INTO log VALUES (-new.a);
+    END`;
+    await sql`CREATE VIRTUAL TABLE ft USING fts5(a, b)`;
+
+    const inserted = await sql`INSERT INTO t VALUES (1), (2)`;
+    const indexed = await sql`INSERT INTO ft VALUES ('alpha beta', 'gamma')`;
+    // The COMMIT makes FTS5 write its pending rows. Those writes are not rows of the COMMIT.
+    const inTransaction = await sql.unsafe("BEGIN; INSERT INTO ft VALUES ('delta', 'epsilon'); COMMIT;");
+    expect({ inserted: inserted.count, indexed: indexed.count, inTransaction: inTransaction.count }).toEqual({
+      inserted: 2,
+      indexed: 1,
+      inTransaction: 1,
+    });
+    expect(await sql`SELECT x FROM log ORDER BY rowid`.values()).toEqual([[1], [-1], [2], [-2]]);
+  });
 });
 
 describe("Indexes and Query Optimization", () => {