Skip to content

fix(js): forward all batches in ChannelBatchReader - #38

Open
fornwall wants to merge 1 commit into
mainfrom
fix/js-channel-batch-reader-multi-batch
Open

fix(js): forward all batches in ChannelBatchReader#38
fornwall wants to merge 1 commit into
mainfrom
fix/js-channel-batch-reader-multi-batch

Conversation

@fornwall

Copy link
Copy Markdown
Owner

Bug (silent data loss)

ChannelBatchReader::next in javascript/src/client.rs built a fresh StreamReader for each IPC buffer received from the channel and returned only the first record batch it yielded. When a pushed IPC buffer contained more than one record batch, every batch after the first was silently dropped, corrupting data on bind/insert.

Fix

Buffer the in-progress StreamReader in a new current field and drain it fully across successive next() calls before pulling the next buffer off the channel:

  • If a reader is in progress, return its next batch; when it is drained, clear current and fetch the next buffer.
  • Empty buffers (readers that yield zero batches) are skipped and iteration continues.
  • Genuine decode errors from StreamReader::try_new are still surfaced as Some(Err(e)).

Verified with cd javascript && cargo build.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XNCrC87g9MkppGpL4MDgh5

`ChannelBatchReader::next` built a fresh `StreamReader` for each buffer
received from the channel and returned only the first record batch it
yielded. When a pushed IPC buffer contained more than one record batch,
every batch after the first was silently dropped, causing data loss on
bind/insert.

Buffer the in-progress `StreamReader` in a new `current` field and drain
it fully across successive `next()` calls before pulling the next buffer
off the channel. Empty buffers are skipped while genuine decode errors
are still surfaced.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNCrC87g9MkppGpL4MDgh5
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.

1 participant