Batch Promise iterator row reads - #233
Merged
penberg merged 14 commits intoSep 18, 2026
Merged
Conversation
xoxohorses
changed the title
xoxohorses
changed the title
xoxohorses
changed the title
xoxohorses
marked this pull request as ready for review
September 14, 2026 17:50
penberg
reviewed
Sep 14, 2026
penberg
force-pushed
the
jy--codex-batched-row-reads
branch
from
September 18, 2026 09:07
3dc3a56 to
ecf467c
Compare
Contributor
|
@claude review |
penberg
added a commit
that referenced
this pull request
Sep 18, 2026
Statements and row iterators hold their own references to the libSQL connection, and libSQL only closes the SQLite handle when the last reference is dropped. `Database.close()` therefore left the database file open (and any read transaction of a partially consumed iterator active) until the garbage collector reclaimed every statement. On Windows this makes deleting the database file after `close()` fail with `EBUSY`, which is what breaks Windows CI on #233. This tracks each database's statements and iterators with weak references and releases their libSQL objects on `close()`. In-flight operations hold their own references and finish normally; an iterator closed mid-`next()` drops its rows once that call completes. Using a statement or iterator after `close()` throws `TypeError: The database connection is not open`, matching better-sqlite3. The iterator wrappers in `compat.js` and `promise.js` now run errors through `convertError` so that surfaces as the same `TypeError`. A second reference was hiding in iterator results: a local libSQL `Row` holds a clone of the statement (and so the connection), and the `Record` returned by `RowsIterator.next()` kept that row alive until GC. Records now copy the column values out when the row is produced (only the first column when plucking), so they no longer pin the connection. Unlike better-sqlite3, closing with an active iterator is still allowed (better-sqlite3 throws "This database connection is busy executing a query"), so the active-iterator sync test is libsql-only. Tests: new regression tests in `sync.test.js` and `async.test.js` fail with `SQLITE_BUSY` without the fix and pass with it.
Closing the iterator resets the statement, but a nextBatch() already running kept stepping it, which restarts the query from the first row. If the batch filled up, the statement was left active with its timeout guard dropped, holding a read lock until the statement was reused. Track closure with a flag that the batch loop checks before each step, and reset the statement after the loop whenever the iterator was closed.
The statement has already been stepped by query() when nextBatch() validates maxRows, so returning early left it active with its timeout guard registered. A rejected next() does not trigger return() in `for await`, so nothing else released it.
The synchronous API reads rows one at a time and its option parsing only
recognizes queryTimeout, so `statement.all(undefined, { batchSize: 250 })`
binds the options object and throws, and defaultBatchSize is ignored.
penberg
force-pushed
the
jy--codex-batched-row-reads
branch
from
September 18, 2026 13:12
ecf467c to
11c8ea0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds
RowsIterator.nextBatch(maxRows)in Rust and updates the Promise API's existingiterate()wrapper to fetch native row batches while returning standard one-row iterator results. ThedefaultBatchSizeconnection option controls the native row batch size, andQueryOptions.batchSizeoverrides it for oneall()oriterate()call. The default is 1, which preserves current behavior and keeps the existing method signatures unchanged.This was motivated by a production trace of a simple ordered query that returned 9,088 rows: about 280 ms of its 350 ms p50 was attributed to SQLite processing. For that result size, the default
all()path makes 9,088 asynchronous Rust-to-JavaScript round trips. Batching keeps SQLite's row-at-a-time stepping inside Rust and reduces those boundary crossings while the JavaScript iterator still returns one row at a time.Benchmark
The public benchmark prepares the table before timing, reads the same rows through each configuration, and validates every result. These averages were measured on Node 22.13.1 for x64 Linux with an Intel Xeon Platinum 8375C CPU.
all()with batch size 1all(…, { batchSize: 250 })Benchmark source: https://github.com/xoxohorses/libsql-js/blob/cb3e8e4fa0dc14811365033f1cc4bce84e8645d8/perf/perf-libsql-batched-rows.js
How was this change tested?
Built the native module, then ran the public benchmark across 1,000, 10,000, 100,000, and 1,000,000 returned rows. The benchmark validated each result set.
[written by Codex]