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

[andinux]

The defect

The positional cursor on cloudsync_payload_chunks was designed to make each /check call an O(1) seek to where the previous call stopped. It does not do that today.

The resume lower bound is stated only inside the disjunction (db_version>? OR (db_version=? AND seq>=?)). SQLite will derive a range term from a disjunction like this — but only when it can see both arms compare against the same value. The two arms here carry distinct anonymous parameters, and SQLite does not assume two parameters hold the same value, so it derives nothing.

cloudsync_changes is a virtual table that string-builds its inner SQL from the constraints xBestIndex receives, so the clause it actually runs is:

WHERE db_version <= ? AND site_id != ? ORDER BY db_version, seq ASC

An upper bound and a site filter, and nothing saying where to start. Every call re-reads the window from the beginning and discards rows until it reaches the resume point — and each discarded row costs nearly as much as an emitted one, because the generated subquery evaluates cloudsync_col_value() plus two joins per row. A full drain is therefore quadratic in the number of chunks.

In production this took one tenant's change export to ~25 minutes, and before a server-side deadline was raised it never completed at all.

The distinction is invisible in the SQL text, which is why this survived review:

two distinct parameters  ->  WHERE db_version <= ? AND site_id != ?
one parameter reused     ->  WHERE db_version <= ? AND db_version >= ? AND site_id != ?

The fix

State the lower bound explicitly alongside the untouched disjunction:

WHERE db_version<=? AND site_id<>? AND db_version>=? AND (db_version>? OR (db_version=? AND seq>=?))

The term is logically implied by the disjunction, so it selects exactly the same rows. It exists only so the constraint loop emits a lower bound into the generated inner SQL and the (db_version) index can seek.

Reusing one parameter across both arms would work identically. I chose the explicit conjunct because it states the bound the code depends on, rather than relying on a planner inference that a later edit could silently undo by splitting the parameter again.

Measurements

make chunk-bench (added here) reproduces the defect and measures the fix locally — no network, no server.

rows	chunks	drain, before	drain, after	speedup	per chunk, before	per chunk, after
3000	94	1582 ms	100 ms	15.8x	16.8 ms	1.07 ms
6000	188	6110 ms	210 ms	29.2x	32.5 ms	1.12 ms

Per-chunk latency by decile of chunk index, same run:

before   2.2   5.2   9.4  14.8  14.0  18.8  20.0  24.2  29.2  31.9    last/first: 14.3x
after    1.04  1.00  1.29  1.04  1.39  1.12  1.01  1.00  1.00  1.02   last/first:  0.98x

There is no single speedup multiplier here, and that is the point: the multiplier is proportional to the window size. Doubling the chunks doubled it (15.8x -> 29.2x). Growth exponents: before 3.86x for 2x the chunks (~N^1.95), after 2.09x (~N^1.06). The invariant is the last column — per-chunk cost stops depending on the window at all. O(N^2) -> O(N).

The benchmark also asserts that all three SQL shapes select the identical row at every resume point (0 differences across 93 points), which is the correctness claim the fix rests on.

Not a PostgreSQL defect

src/postgresql/cloudsync_postgresql.c:1400-1407 writes the same disjunction with $3 in both arms, so the parameter-identity trap does not apply, and PostgreSQL's planner handles OR'd range bounds natively rather than through a vtab constraint pipeline. No change there.

Testing

• dist/unit — 156 tests, all OK
• dist/review_regressions — 0 failures
• dist/network_unit — all passed
• make chunk-bench at 94 and 188 chunks, curve flat after the fix

Notes for reviewers

• The benchmark is deliberately not on CI: timings are machine-dependent, and the shape of the curve rather than the absolute numbers is the result.
• If you re-run it, make first. make dist/chunk_bench builds the test binary but not dist/cloudsync.dylib, which is what the benchmark load_extensions — measuring against a stale extension shows the fix doing nothing.
• CHUNK_BENCH_TXNS=1 is the negative control: one db_version for the whole window, where no bound on db_version can narrow anything. Cost there is flat and high (~65 ms/chunk) both before and after, as it should be.
• The existing comments at cloudsync_sqlite.c:1049 and :1342 claiming "O(1) seek per chunk" were aspirational; they are now accurate, so they are left alone.

🤖 Generated with Claude Code