Skip to content

Comments

fix(ts): support streaming queries in amp query#1854

Merged
leoyvens merged 1 commit intomainfrom
fix-query-streaming-output
Feb 24, 2026
Merged

fix(ts): support streaming queries in amp query#1854
leoyvens merged 1 commit intomainfrom
fix-query-streaming-output

Conversation

@leoyvens
Copy link
Collaborator

The query command collected all record batches into a single Arrow Table before formatting, which meant streaming queries would hang forever waiting for a stream that never ends. This switches to decoding rows as each batch arrives, letting jsonl and pretty formats emit output immediately while table/json still collect but respect --limit to bound the buffering.

A process.exit(0) at the end ensures gRPC connections don't keep the event loop alive after a streaming query is interrupted.

Tested manually.

@fubhy this is AI code so I'd appreciate a review of Effect usage.

@leoyvens leoyvens requested a review from fubhy February 24, 2026 12:34
@leoyvens leoyvens changed the title fix(ts): stream query output row-by-row instead of collecting fix(ts): support streaming queries in amp query Feb 24, 2026
@LNSD
Copy link
Contributor

LNSD commented Feb 24, 2026

@leoyvens wdyt if we instead of extending the Amp TS CLI, we add crates/bin/amp-query CLI that uses the flight client? All in Rust

@leoyvens
Copy link
Collaborator Author

@LNSD would be great, but a larger scope than this PR

Copy link
Contributor

@fubhy fubhy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leoyvens @LNSD we are working on a leaner typescript sdk here: https://github.com/edgeandnode/amp-typescript/blob/main/packages/amp

That one also has the correct streamign semantics because it doesn't use the paritally broken (but official) apache arrow (arrow-js) package and instead uses a hand-rolled implementation of arrow purely for decoding to JS (which is what you would normally in JS as we are considering it a leaf end consumer, not an arrow data pipeline processor).

I am aligned with eventually removing the TS/JS implementation of amp from this repository and instead going all in on Rust for CLI, built-in client, etc.

But agreed with Leo that that's a larger scope and for now this is actually reasonable.


// Force exit to close any open gRPC connections (streaming queries never
// send EOS so the event loop would otherwise hang indefinitely).
process.exit(0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is slop.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I just remove it, streaming queries with a --limit option hang and the CLI command doesn't terminate.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have suggestions here I'm happy to try them in a follow up

The query command collected all record batches into a single Arrow Table before formatting, which meant streaming (infinite) queries would hang forever waiting for a stream that never ends. This switches to decoding rows as each batch arrives, letting jsonl and pretty formats emit output immediately while table/json still collect but respect --limit to bound the buffering. A process.exit(0) at the end ensures gRPC connections don't keep the event loop alive after a streaming query is interrupted.

Signed-off-by: Leonardo Yvens <leoyvens@gmail.com>
@leoyvens leoyvens force-pushed the fix-query-streaming-output branch from 6a48ea8 to 8e9bd30 Compare February 24, 2026 15:45
@leoyvens leoyvens merged commit 552d449 into main Feb 24, 2026
12 of 14 checks passed
@leoyvens leoyvens deleted the fix-query-streaming-output branch February 24, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants