fix(sqlite): seek to the positional resume point instead of replaying the window

CHANGELOG.md

@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ### Fixed
 
+- **SQLite: paging a chunked download no longer gets slower with every chunk.** The positional cursor on `cloudsync_payload_chunks` was meant to seek straight to where the previous call stopped, but it stated its resume point only inside `(db_version > ? OR (db_version = ? AND seq >= ?))`, whose two arms carry distinct parameters. SQLite does not derive a range from that, so the scan over `cloudsync_changes` ran with an upper bound only and re-read the window from the beginning on every call, discarding rows until it reached the resume point — making a full drain quadratic in the number of chunks, and long enough on a large tenant to hit a server-side deadline and never complete. An explicit `db_version >= ?` is now stated alongside the disjunction; it selects exactly the same rows and lets the scan seek. Locally, draining a 188-chunk window went from 6110 ms to 210 ms, and the per-chunk cost no longer depends on how large the window is. PostgreSQL was never affected.
 - **A row rewritten in one statement no longer keeps the old blocks of a shorter value.** Writing a whole row rewrites its block column from the first position, so any block past the end of the new value stayed stored. On SQLite `INSERT OR REPLACE` skips the old row's delete trigger unless `recursive_triggers` is on, so those leftovers kept their metadata and were delivered as content: replacing `AAA\nBBB\nCCC` with `ZZZ` left `ZZZ\nBBB\nCCC` on the peers and on a later local read. On PostgreSQL they carried no metadata, so peers were unaffected, but they stayed in the blocks table for the life of the row. Blocks the new value does not cover are now retired with it.
 - **SQLite: a payload whose commit fails no longer leaves its transaction open.** When `cloudsync_payload_apply` started the transaction itself and the commit then failed — a deferred foreign key violated at commit, or `SQLITE_BUSY` because a reader held the database — the transaction stayed open: the uncommitted rows remained visible on the connection and the next `BEGIN` failed. The failed transaction is now rolled back and the original error is returned. Changes from earlier source versions that were already committed are kept, the receive checkpoint does not move, and the rolled-back rows are no longer counted as applied, so delivering the payload again applies it. A transaction or savepoint opened by the caller is still left to the caller.
 - **PostgreSQL: applying a payload read from a table works at any savepoint depth.** Inside 126 or more savepoints, `SELECT cloudsync_payload_apply(payload) FROM some_table` still failed with `buffer pin ... is not owned by resource owner SubTransaction` (and a caught error at that depth could abort an assertion-enabled server): cloudsync recorded the caller's resource owner and memory context for at most 128 nesting levels, counting its own internal savepoints, and silently stopped restoring them beyond that. The fixed limit is gone; only PostgreSQL's own resource limits apply.

Makefile

@@ -357,6 +357,17 @@ sync-bench: $(TARGET) $(DIST_DIR)/sync_bench$(EXE)
 sync-bench-debug:
 	$(MAKE) NETWORK_TRACE=1 sync-bench
 
+# Time the positional /check drain on a locally generated window. Separate from
+# the test targets because timings are machine-dependent; it needs no network.
+chunk-bench: $(TARGET) $(DIST_DIR)/chunk_bench$(EXE)
+	@if [ -n "$(CHUNK_BENCH_ROWS)" ]; then export CHUNK_BENCH_ROWS="$(CHUNK_BENCH_ROWS)"; fi; \
+	if [ -n "$(CHUNK_BENCH_ROW_BYTES)" ]; then export CHUNK_BENCH_ROW_BYTES="$(CHUNK_BENCH_ROW_BYTES)"; fi; \
+	if [ -n "$(CHUNK_BENCH_TXNS)" ]; then export CHUNK_BENCH_TXNS="$(CHUNK_BENCH_TXNS)"; fi; \
+	if [ -n "$(CHUNK_BENCH_REPEATS)" ]; then export CHUNK_BENCH_REPEATS="$(CHUNK_BENCH_REPEATS)"; fi; \
+	if [ -n "$(CHUNK_BENCH_CHUNK_SIZE)" ]; then export CHUNK_BENCH_CHUNK_SIZE="$(CHUNK_BENCH_CHUNK_SIZE)"; fi; \
+	if [ -n "$(CHUNK_BENCH_VERBOSE)" ]; then export CHUNK_BENCH_VERBOSE="$(CHUNK_BENCH_VERBOSE)"; fi; \
+	./$(DIST_DIR)/chunk_bench$(EXE)
+
 OPENSSL_TARBALL = $(OPENSSL_DIR)/$(OPENSSL_VERSION).tar.gz
 
 $(OPENSSL_TARBALL):

src/sqlite/cloudsync_sqlite.c

@@ -1401,9 +1401,14 @@ static int payload_chunks_filter(sqlite3_vtab_cursor *cursor, int idxnum, const
     // (db_version, seq) >= (resume_dbv, resume_seq).
     char *sql;
     if (positional) {
+        // The redundant db_version>=? is what makes the resume a seek, and it is
+        // load-bearing: SQLite derives a range from a disjunction only when both arms
+        // compare against the same value, and these two arms carry distinct
+        // parameters. Without it cloudsync_changes' xBestIndex is offered no lower
+        // bound at all and every call replays the window from the start.
         sql = sqlite3_mprintf(
             "SELECT tbl, pk, col_name, col_value, col_version, db_version, site_id, cl, seq "
-            "FROM cloudsync_changes WHERE db_version<=? AND site_id%s? AND "
+            "FROM cloudsync_changes WHERE db_version<=? AND site_id%s? AND db_version>=? AND "
             "(db_version>? OR (db_version=? AND seq>=?)) ORDER BY db_version, seq ASC",
             site_op);
     } else {
@@ -1421,7 +1426,8 @@ static int payload_chunks_filter(sqlite3_vtab_cursor *cursor, int idxnum, const
         sqlite3_bind_blob(c->src, 2, site_id, site_id_len, SQLITE_TRANSIENT);
         sqlite3_bind_int64(c->src, 3, resume_dbv);
         sqlite3_bind_int64(c->src, 4, resume_dbv);
-        sqlite3_bind_int64(c->src, 5, resume_seq);
+        sqlite3_bind_int64(c->src, 5, resume_dbv);
+        sqlite3_bind_int64(c->src, 6, resume_seq);
     } else {
         sqlite3_bind_int64(c->src, 1, since);
         sqlite3_bind_blob(c->src, 2, site_id, site_id_len, SQLITE_TRANSIENT);