fix(query)!: stream results instead of buffering them by anoop-narang · Pull Request #300 · hotdata-dev/hotdata-cli

anoop-narang · 2026-09-11T09:23:11Z

Why

hotdata query held every fetched result in memory at once — the IPC bytes, the
decoded batches, and a heap-allocated JsonCell per value. Peak RSS ran to ~190
bytes per row for a single BIGINT column, and the overhead is per cell, so a
wide table is far worse.

On a 2 GB host a 10 M-row result is SIGKILLed after minutes of apparent hanging,
leaving an empty file and no error. Nothing can be printed — the kernel kills the
process — so the only fix is not to need the memory.

What

csv and json render batch by batch as they arrive. Peak memory is one
record batch whatever the result size.

Output is unchanged. The streamed renderers are held byte-for-byte against
the buffered ones in tests, over nulls, embedded commas/quotes/newlines, nested
lists and 38-digit decimals. The JSON envelope is written by hand (row_count is
only known after the rows, and it follows them in serialized order), so
streamed_json_matches_the_buffered_envelope asserts field order, indentation
and null rendering against serde_json::to_string_pretty of the equivalent
struct.

-o table fetches a bounded window via ?limit=, so the rest is never
downloaded. A window smaller than the result is reported by the existing
incomplete-preview footer and exit code — no new UI.

A result the server is still writing is now waited for. This is the part that
matters most, and it is a pre-existing gap this branch would otherwise inherit:
the CLI fetched the persisted result once and, on 202 processing, degraded to
the bounded preview with exit 3. For a large result the server is still draining
when that fetch lands, so the caller got 10,000 rows and a non-zero exit instead
of their data — data loss wearing an exit code. The wait honours the server's
Retry-After (already parsed by the SDK into ArrowError::NotReady.retry_after,
so the retry decision never depends on error text) and is bounded by the same 5
minutes the async path already allows.

Measured

Fetching a persisted 4 M-row result:

old   peak RSS 353 MB   wall 8.42 s   4,000,001 rows
new   peak RSS  23 MB   wall 4.29 s   4,000,001 rows

Twice as fast because a buffered writer replaces a syscall per row. End to end
against a server with the matching fix (runtimedb#1376), the same query runs in
20 MB.

Row order differs between any two fetches of a persisted result — verified
pre-existing by running the old binary against the same result twice. Sorted
checksums match.

Tests

523 passing, up from 521. New coverage: the byte-equality pair above, a zero-row
result (the case the hand-written envelope is most likely to get wrong), the
capped-table window, a short body being reported as incomplete, and the two
polling tests — a result that never becomes ready must not hang, and a ready one
must not pay a poll interval.

The five resolve_inline tests became plan_inline tests: the inline decision is
now a pure function returning a plan, so it is unit-testable without a server.

Merge order

sdk-rust#142 (the streaming reader), released
this, with the hotdata dep moved off the path override back to the released version
runtimedb#1376 (bounds the server's inline reply)

3 must not land before 2 is released and in users' hands. A server that hands
out tickets to a CLI that does not wait for them gives short results with exit 3.

`hotdata query` held every fetched result in memory: the IPC bytes, the decoded batches, and a heap-allocated JSON cell per value, all at once. Peak RSS ran to ~190 bytes per row for a single BIGINT column, so a 10M-row result was SIGKILLed on a 2GB host after minutes of apparent hanging, leaving an empty file and no error — the kernel kills the process, so nothing can be printed. csv and json now render batch by batch as they arrive, so peak memory is one record batch whatever the result size. Output is unchanged: the streamed renderers are held byte-for-byte against the buffered ones in tests, over nulls, embedded commas/quotes/newlines, nested lists and 38-digit decimals. Measured on a 4M-row result: 353MB -> 23MB, and twice as fast, since a buffered writer replaces a syscall per row. `-o table` fetches a bounded window with `?limit=`, so the rest is never downloaded. A window smaller than the result is reported by the existing incomplete-preview footer and exit code rather than looking complete. A result the server is still writing is now waited for. Fetching once and giving up handed back the bounded preview with a non-zero exit while the real result finished writing a moment later — data loss wearing an exit code. The wait honours the server's Retry-After and is bounded by the same 5 minutes the async path already allows.

codecov · 2026-09-15T05:41:27Z

Codecov Report

❌ Patch coverage is 86.93009% with 86 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/commands/query.rs	85.52%	77 Missing ⚠️
src/commands/results.rs	0.00%	7 Missing ⚠️
src/client/sdk.rs	98.31%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

claude

Review

Blocking Issues

src/commands/query.rs:832-834 — fetch_capped decides truncated only from total_row_count. arrow_result_to_query_response backfills total_row_count from the held rows when the server sends no X-Total-Row-Count (lines 259-262). A limited fetch with no such header therefore sets total == row_count, and a 10,000-row window of a 1,000,000-row result prints as a complete table with exit 0. That is the silent truncation this PR sets out to remove, moved to the -o table path.

Action Required

Make the -o table cap independent of X-Total-Row-Count. fetch_capped already reads stream.next_link() and discards the value; treat a present next_link, or a window filled to cap, as truncated.
Add a test that serves a full window with no X-Total-Row-Count header and asserts truncated and EXIT_INCOMPLETE_RESULT.

Six inline comments in total: one blocking, three nits, two super nits.

Note on verification: no test run is reported for this PR. CI / test was queued when this review started, so the streaming and polling tests are unverified here. The description says CI is red because of a local path override, but Cargo.toml and Cargo.lock on this head take hotdata 0.18.0 from crates.io, so that note reads as stale.

`fetch_capped` decided truncation from `total_row_count`, which `arrow_result_to_query_response` backfills from the rows in hand when the response carries no `X-Total-Row-Count`. A window then had total == row_count, so a 10,000-row slice of a million-row result printed with a plain footer and exit 0 — the silent truncation this command exists to avoid, reintroduced on the table path. The fetch now asks for one row past the window. That row's presence is the proof there is more, needs nothing from the server but the rows, and is dropped before rendering. The reported total is used only when the server actually sent one, so an unheadered window shows `?` rather than a total derived from the window itself. `stream_json` also wrote `truncated: false` unconditionally while the process exited non-zero on a short body. The field is written after the rows, so it now carries what the body turned out to be.

claude

Prior blocking issue is resolved: the table cap now derives truncation from a probe row instead of X-Total-Row-Count, and a_capped_window_is_incomplete_even_with_no_row_count_header covers it. The four prior nits are also applied. One non-blocking nit left inline.

The probe row only proves there is more when `?limit=` is honoured exactly. A server free to clamp the limit to its own maximum returns the window full and no further row, so the probe never arrives, and a window with no X-Total-Row-Count printed as a complete table again — the same silent truncation through a different door. `next_link` is the server stating there is another page, and it was already being constructed and dropped. It now counts alongside the probe row, so either signal alone is enough to mark the window a preview.

claude

All prior blocking findings are fixed. The probe row plus next_link now carry truncation for the capped table window, and the streamed JSON envelope reports truncated after the rows. One non-blocking comment inline.

CI was still queued or in progress when this review ran, so the test results are not known here.

anoop-narang added 2 commits September 11, 2026 14:52

chore(deps): take hotdata 0.18.0 from crates.io

265f015

anoop-narang marked this pull request as ready for review September 15, 2026 05:37

anoop-narang requested a review from a team as a code owner September 15, 2026 05:37

anoop-narang requested review from shefeek-jinnah and removed request for a team September 15, 2026 05:37