Add a .stream() terminal with server-side parameter binding to Layer 3#59
Conversation
be68c63 to
fb55d05
Compare
|
@chibugai, review it |
There was a problem hiding this comment.
Pull request overview
Adds a Layer 3 .stream() terminal for fluent SELECT queries so large result sets can be consumed lazily (row-by-row) via Layer 1’s streaming cursor, while preserving server-side parameter binding for injection safety.
Changes:
- Add
SelectQueryBuilder.stream()returningAsyncIterableIterator<Row>via a newexecuteSelectStreamterminal. - Add Layer 1
Session.queryStreamBind()and route native streaming throughchdb_stream_query_with_params_nwhen params are provided. - Add Layer 3 streaming tests covering parity, server-side binding, 64-bit precision, no-session error, and abort.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/v3/layer3/stream.test.ts | New Layer 3 .stream() behavioral tests (parity, binding, precision, abort). |
| src/layer3/runtime.ts | Extend runtime session typing with queryStreamBind and RuntimeRowStream. |
| src/layer3/execute/terminal.ts | Implement executeSelectStream / streaming row generator; enforce “bound session” requirement. |
| src/layer3/builder/select.ts | Add .stream() terminal to SelectQueryBuilder. |
| lib/chdb_node.cpp | Extend native StreamQuery wrapper to optionally bind params server-side. |
| index.js | Add Session.queryStreamBind and refactor stream startup through a shared #startStream. |
| index.d.ts | Add TypeScript declaration for Session.queryStreamBind. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Reviewed — OCR deep pass (opus-4-8, 6 files) plus a manual read of the native and Layer 3 paths. No correctness or safety issues found; looks good to me. Things I specifically checked and was happy with:
One minor, non-blocking note: that |
The fluent read path buffered the whole result (parseRows over the full text), so a forgotten .limit() on a big OLAP scan peaked at ~3-4x the result size and risked OOM. Layer 1 already had a streaming cursor but the fluent builder never used it, and its param-less C entry could not carry the compiler's bound values. libchdb already ships chdb_stream_query_with_params_n (a streaming + server-side binding entry), so no upstream change is needed: - Native StreamQuery now takes an optional pre-formatted params object and routes to chdb_stream_query_with_params_n, mirroring the one-shot QueryWithParams path; the param-less path is unchanged. - Layer 1 gains Session.queryStreamBind (queryStream refactored to share a single #startStream), binding values exactly like queryBindAsync. - Layer 3 SelectQueryBuilder.stream() returns an AsyncIterableIterator of rows via executeSelectStream. Values stay bound server-side, so a streamed read is as injection-safe as .execute(). It requires a bound session (the default connection has no streaming cursor) and honors an AbortSignal. chDB builds a streaming query's output format from the connection's session settings at open time, not from a query's trailing SETTINGS clause, so the row view SETs output_format_json_quote_64bit_integers before opening the stream — without it, 64-bit ints would stream as lossy JS numbers and streamed rows would differ from executed rows. Tests: 5 new .stream() cases (parity with execute, bound filter, 64-bit precision, no-session throw, abort); full v3 suite green (373 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fb55d05 to
f6457ee
Compare
What
Adds a
.stream()terminal to the Layer 3 fluent SELECT builder so large resultsets arrive lazily, one row at a time, through Layer 1's streaming cursor instead
of buffering the whole result.
Why
The fluent read path buffered the entire result (
parseRowsover the full text),so a forgotten
.limit()on a big OLAP scan peaked at ~3–4× the result size andrisked OOM. Layer 1 already had a streaming cursor, but the fluent builder never
used it, and its param-less streaming C entry could not carry the compiler's bound
values.
libchdbalready shipschdb_stream_query_with_params_n(streaming + server-sidebinding), so no upstream change is needed.
How
lib/chdb_node.cpp):StreamQuerynow takes an optionalpre-formatted params object and routes to
chdb_stream_query_with_params_n,mirroring the one-shot
QueryWithParamspath; the param-less path is unchanged.index.js/index.d.ts): newSession.queryStreamBind;queryStreamrefactored to share a single#startStream. Values are boundexactly like
queryBindAsync.builder/select.ts,execute/terminal.ts,runtime.ts):SelectQueryBuilder.stream()returns anAsyncIterableIterator<O>viaexecuteSelectStream. Values stay bound server-side, so a streamed read is asinjection-safe as
.execute(). It requires a bound session (the defaultconnection has no streaming cursor) and honors an
AbortSignal.Note on 64-bit precision
chDB builds a streaming query's output format from the connection's session
settings at open time, not from a query's trailing
SETTINGSclause (whichexecuteSelectrides in SQL). So the row viewSETsoutput_format_json_quote_64bit_integers = 1before opening the stream —without it, 64-bit ints would stream as lossy JS numbers and streamed rows would
differ from executed rows.
Tests
.stream()cases: parity with.execute(), bound filter, 64-bitprecision, no-session throw, abort.
streaming tests.
🤖 Generated with Claude Code