perf: stream parameterized query results instead of buffering them

[lukekim]

Problem

Client.query_with_params() documents that it returns a streaming pa.RecordBatchReader, but it materialized the whole result before returning:

handle, _ = stmt.execute_query()
reader = pa.RecordBatchReader.from_stream(handle)
table = reader.read_all()      # <- entire result pulled into memory
return table.to_reader()
finally:
    stmt.close()

The eager read was forced by the finally: stmt.close(). The ADBC statement owns the result stream, so closing it there meant the stream had to be drained first.

The effect is that a parameterized query over a large table has no streaming behaviour at all: asking for the first batch pays the cost of the entire result, in both time and memory.

Fix

Keep the statement alive until the stream is drained — or until the returned reader is closed or garbage collected — and hand back a reader that pulls batches off the wire on demand.

Measurements

Against a local runtime serving a 10M-row (382 MiB) Arrow-accelerated table, SELECT * FROM big WHERE id >= $1:

	before	after
time to first batch	0.305 s	0.021 s
peak RSS while streaming the full result	579 MB	248 MB
peak RSS, non-parameterized streaming (reference)	157 MB	157 MB

Fully materializing the result (query_arrow(sql, params=[...])) is unchanged at ~0.22 s / ~575 MB, as expected — the win is for callers that stream.

For reference, the non-parameterized paths were already streaming correctly and are unchanged by this PR; they measure at parity with a raw pyarrow Flight client.

Tests

Adds TestADBCClientStreaming covering that:

• the result is not drained before the reader is returned (this test fails on trunk with result was buffered instead of streamed)
• draining the reader yields every batch and then releases the statement
• the statement is released when the reader is closed early or abandoned
• the statement is released when execution fails

Full suite: 390 passed, 24 skipped. ruff reports no new findings (2 pre-existing PLR0917), black clean, mypy clean, pylint 9.26/10.

tests/test_main.py::test_local_runtime_refresh fails identically before and after this change — it is an integration test needing a local runtime with a taxi_trips dataset.

---

Follow-up in this PR: reachable from query_batches, and documented

query_batches is the streaming entry point, but it only accepted a plain SQL
string — so the now-streaming parameterized path could not be reached through
it. It takes params as well:

for batch in client.query_batches(
    'SELECT * FROM taxi_trips WHERE trip_distance > $1', params=[5.0]
):
    ...

The README now also states which query_* helpers materialize the whole result
and which one to reach for when it does not fit in memory — previously neither
query_batches nor the materializing helpers were documented at all.

Additional measurements

Peak RSS while streaming a parameterized result is now flat in the result size
rather than proportional to it, measured against a local runtime over an
Arrow-accelerated table (isolated process per run, ru_maxrss):

rows	before	after
10M	559-581 MiB	248-250 MiB
20M	946-969 MiB	257-265 MiB

Time to first batch is likewise flat: 0.30 s → 0.027 s at 10M rows, 0.59 s →
0.030 s at 20M.

Verification

Run against a local runtime with an Arrow-accelerated dataset, so the
integration tests execute rather than skip:

• pytest — 392 passed, 24 skipped (cloud tests, no API key)
• mypy — clean; pylint — 9.26/10
• ruff check — 2 findings, both pre-existing on trunk and untouched here
• ruff format --check — 15 files would be reformatted, identical before and
  after this change (the committed formatting predates the configured
  line-length); left alone so it does not bury the change

tests/test_main.py::test_local_runtime_refresh is order- and timing-dependent
against a live runtime — it truncates the shared dataset via refresh_sql and
races an async refresh — so it can fail on a re-run regardless of this change.

[Copilot]

Pull request overview

This PR fixes _ADBCClient.query_with_params() to actually return a streaming pyarrow.RecordBatchReader (as documented) by keeping the underlying ADBC statement alive until the result stream is exhausted or the reader is closed/collected, avoiding eager materialization of the entire result.

Changes:

• Add _stream_until_closed() to wrap a RecordBatchReader and ensure the owning ADBC statement is closed when streaming completes or the reader is cleaned up.
• Update query_with_params() to return a streaming reader instead of buffering via read_all(), while still ensuring stmt.close() on pre-stream failures.
• Add unit tests verifying lazy streaming behavior and statement lifecycle cleanup (drain, early close/abandon, and execution error).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
spicepy/_client.py	Switch parameterized query path to true streaming and ensure statement cleanup is deferred until stream completion/close.
tests/test_client.py	Add targeted tests asserting streaming (non-buffering) and correct statement cleanup across success/early-close/error cases.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[claudespice]

The red Integration tests here aren't caused by this PR — every OS/Python combination fails at the fixture setup step, before any test runs:

spice add spiceai/quickstart
ERROR Invalid argument: Failed to extract Spicepod archive: invalid Zip archive: Could not find EOCD

The registry is returning a 152-byte JSON error body under HTTP 200 with Content-Type: application/zip, so the CLI hands it to the unzipper and reports a corrupt archive. Still reproducing as of 2026-07-28T01:12Z:

{"Message":"invalid path \"spiceaiquickstartv0.1.08bb188f7b4106571cdec9b33f44963b04f928f7f\": selected encoding not supported","Code":0,"Type":"error"}

The workflow's retry wrapper can't help — the response is a deterministic 200, not a flake. Filed as spiceai/spiceai#12116 (covers both the registry bug and the CLI's misleading error). Nothing to fix on this branch; the integration jobs should go green on a re-run once the registry is serving archives again.

[claudespice]

Update — this blocker has cleared; the red runs just predate the fix.

The spiceai/quickstart registry entry now serves a valid archive. Re-probed 2026-07-31T22:2xZ:

$ curl -sSI https://api.spicerack.org/v1/spicepods/spiceai/quickstart
HTTP/2 200
content-type: application/zip

$ unzip -t q.zip
    testing: spicepod.yaml            OK
No errors detected in compressed data of q.zip.

279 bytes, valid PK header and EOCD, and it extracts a well-formed spicepod.yaml for the taxi_trips dataset — so spice add spiceai/quickstart succeeds again and the tracking issue spiceai/spiceai#12116 is closed.

The failing Integration tests runs on this PR are from 2026-07-25/26, before the registry recovered, and CI has not re-run since. A maintainer re-run of the failed jobs should green them with no change to this branch — I do not have rerun rights here (gh run rerun → Must have admin rights to Repository).