sqlite: don't count rows written to temporary (ephemeral) tables

SQLite may generate temporary tables while executing a SELECT statement. Rows written to these tables shouldn't count as they're not under control of the user. This change is essentially tursodatabase/libsql@5d6c79e, which does this same thing in libsql; libsql is also where the original row-counting logic comes from. The test was written by glen@cloudflare.com.
cloudflare · Jan 30, 2024 · 4226541 · 4226541
1 parent e3b7a02
commit 4226541
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 6 deletions.
diff --git a/patches/sqlite/0001-row-counts-plain.patch b/patches/sqlite/0001-row-counts-plain.patch
@@ -86,7 +86,7 @@ diff -u5 -r sqlite-src-3440000.pristine/src/vdbeapi.c sqlite-src-3440000/src/vdb
    return (int)v;
 diff -u5 -r sqlite-src-3440000.pristine/src/vdbe.c sqlite-src-3440000/src/vdbe.c
 --- sqlite-src-3440000.pristine/src/vdbe.c	2023-11-01 04:31:37.000000000 -0700
-+++ sqlite-src-3440000/src/vdbe.c	2023-11-03 15:13:45.737325860 -0700
++++ sqlite-src-3440000/src/vdbe.c	2024-01-30 10:49:55.735518937 -0800
 @@ -3708,10 +3708,11 @@
    if( pOp->p3 ){
      nEntry = sqlite3BtreeRowCountEst(pCrsr);
@@ -129,7 +129,7 @@ diff -u5 -r sqlite-src-3440000.pristine/src/vdbe.c sqlite-src-3440000/src/vdbe.c
  #endif
 
    assert( (pOp->p5 & OPFLAG_LASTROWID)==0 || (pOp->p5 & OPFLAG_NCHANGE)!=0 );
-+  p->aLibsqlCounter[LIBSQL_STMTSTATUS_ROWS_WRITTEN - LIBSQL_STMTSTATUS_BASE]++;
++  if (!pC->isEphemeral) p->aLibsqlCounter[LIBSQL_STMTSTATUS_ROWS_WRITTEN - LIBSQL_STMTSTATUS_BASE]++;
    if( pOp->p5 & OPFLAG_NCHANGE ){
      p->nChange++;
      if( pOp->p5 & OPFLAG_LASTROWID ) db->lastRowid = x.nKey;
@@ -141,7 +141,7 @@ diff -u5 -r sqlite-src-3440000.pristine/src/vdbe.c sqlite-src-3440000/src/vdbe.c
 
    /* Invoke the update-hook if required. */
    if( opflags & OPFLAG_NCHANGE ){
-+    p->aLibsqlCounter[LIBSQL_STMTSTATUS_ROWS_WRITTEN - LIBSQL_STMTSTATUS_BASE]++;
++    if (!pC->isEphemeral) p->aLibsqlCounter[LIBSQL_STMTSTATUS_ROWS_WRITTEN - LIBSQL_STMTSTATUS_BASE]++;
      p->nChange++;
      if( db->xUpdateCallback && ALWAYS(pTab!=0) && HasRowid(pTab) ){
        db->xUpdateCallback(db->pUpdateArg, SQLITE_DELETE, zDb, pTab->zName,
@@ -189,7 +189,7 @@ diff -u5 -r sqlite-src-3440000.pristine/src/vdbe.c sqlite-src-3440000/src/vdbe.c
    pIn2 = &aMem[pOp->p2];
    assert( (pIn2->flags & MEM_Blob) || (pOp->p5 & OPFLAG_PREFORMAT) );
    if( pOp->p5 & OPFLAG_NCHANGE ) p->nChange++;
-+  p->aLibsqlCounter[LIBSQL_STMTSTATUS_ROWS_WRITTEN - LIBSQL_STMTSTATUS_BASE]++;
++  if (!pC->isEphemeral) p->aLibsqlCounter[LIBSQL_STMTSTATUS_ROWS_WRITTEN - LIBSQL_STMTSTATUS_BASE]++;
    assert( pC->eCurType==CURTYPE_BTREE );
    assert( pC->isTable==0 );
    rc = ExpandBlob(pIn2);

diff --git a/src/workerd/api/sql-test.js b/src/workerd/api/sql-test.js
@@ -845,6 +845,14 @@ async function testIoStats(storage) {
     assert.deepEqual(rows.rowsRead, 2)
     assert.deepEqual(colCounts, [2, 2])
   }
+
+  // Temporary tables (i.e. for IN clauses) don't contribute to rowsWritten
+  {
+    const cursor = sql.exec(`SELECT * FROM abc WHERE a IN (1,2,3,4,5,6)`)
+    const rows = Array.from(cursor)
+    assert.deepEqual(cursor.rowsRead, 2)
+    assert.deepEqual(cursor.rowsWritten, 0)
+  }
 }
 
 async function testForeignKeys(storage) {

diff --git a/src/workerd/util/sqlite-test.c++ b/src/workerd/util/sqlite-test.c++
@@ -545,7 +545,7 @@ KJ_TEST("SQLite write row counters (basic)") {
     KJ_EXPECT(stats.written == 2);
   }
 
-  // On an indexed table, each updated row is two writes. This is probably due to the index update.
+  // Same as above, but with an index.
   {
     db.run("DELETE FROM things");
     db.run("INSERT INTO things (id) VALUES (1)");
@@ -555,7 +555,7 @@ KJ_TEST("SQLite write row counters (basic)") {
 
     RowCounts stats = countRowsTouched(db, "UPDATE things SET id = id * 10 WHERE id >= 3");
     KJ_EXPECT(stats.read >= 4);  // At least one read per updated row
-    KJ_EXPECT(stats.written == 4);
+    KJ_EXPECT(stats.written == 2);
   }
 }