-
Notifications
You must be signed in to change notification settings - Fork 56
Streaming results: error appears as a result row #378
Description
Describe the bug
When using .stream() to stream rows as JSON, if an error occurs mid-query, the "exception" appears as a row in the results. This effectively means you need to scan the results for a row containing only "exception". This is costly (not much, but every bit counts) and silly.
If you have a scenario where the query legitimately returns a column named exception, then...well you get the point.
Initially I used StreamReadable.on('error', ...) expecting that listener to be invoked, but it wasn't. On one hand, I guess it makes sense since it wasn't a stream/read related error, but the fact that errors aren't surfaced in a first-class way is a design limitation.
Steps to reproduce
Run a query that will produce an error mid-way through, i.e.
SELECT
number,
if(number > 420000, throwIf(1, 'BOOM!'), number) AS value
FROM
numbers(1000000)Stream that via ClickHouseClient.query().stream(). You'll see the row with "exception" appears among the results.
Expected behaviour
Errors should be surfaced in a more first-class way. Proposals:
- Ideally each data chunk of results should have an envelope indicating whether an error occurred. That way you would only need to check for an error once per chunk, rather than once per row. Think millions+ of rows, think compression enabled -- it's only a handful of chunks. This would be a difference maker -- and much more elegant.
- Consider having the SDK intercept the error (in a performant way, one way or another) and surface it via the
StreamReadable.on('error', ...)listener. - Dirt simple: instead of
"text"you could distinguish it as"error". This would still require checking each row, far from ideal, but a step in the right direction.
Thank you!!