fix(ipc): body prefix must be uncompressed length - #470
Open
kentkwu wants to merge 2 commits into
Open
Conversation
Per the Arrow columnar format spec, the eight-byte prefix on each compressed IPC body buffer must hold the buffer's uncompressed length so consumers can size the decompression destination buffer. The writer was emitting the compressed length instead, which caused PyArrow (and any other implementation that relies on the prefix for allocation) to reject Arrow JS-produced ZSTD and LZ4_FRAME streams with a "destination buffer too small" error. JS-to-JS round trips did not surface the bug because the reader sizes buffers from the codec frame rather than the prefix. Add a byte-level regression test that inspects each emitted prefix against the decompressed payload length for both allowed codecs (LZ4_FRAME and ZSTD), and factor the shared codec-registration helper out of stream-writer-tests.ts so the new test file does not have to import from another test file.
domoritz
approved these changes
Sep 3, 2026
Member
|
Could you check CI failures? |
The Closure externs generator crawls exported classes for their static
and prototype property names, so option-object properties (referenced
only inside constructor bodies) never get reserved. Closure ADVANCED
renames them, and external UMD callers passing
`{ compressionType: X }` to a writer receive silently-uncompressed
output — the writer's lookup targets the renamed property, gets
nothing, and sets `_compression = null`.
Declare `compressionType` as reserved so Closure preserves it.
`autoDestroy` is already preserved coincidentally (it exists as a
prototype property on `RecordBatchReader`); the identically-shaped
bug for `writeLegacyIpcFormat` is left for follow-up.
kentkwu
force-pushed
the
fix/ipc-writer-uncompressed-length-prefix
branch
from
September 3, 2026 14:27
e110c6b to
59fde71
Compare
Member
|
@trxcllnt Do you want to review this before we merge this? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Arrow columnar format spec requires the eight-byte prefix on each compressed IPC body buffer to hold the uncompressed length so a reader can size the decompression destination buffer.
The writer was emitting the compressed length instead, so PyArrow rejects Arrow JS-produced ZSTD and LZ4_FRAME streams. JS-to-JS round trips missed the bug because the reader sizes buffers from the codec frame, not the prefix. The compression code path is shared between the stream and file writers, so the one-line fix covers both formats.
src/ipc/writer.ts: writebyteBuf.lengthin the prefix instead ofcompressed.lengthregisterCompressionCodecsand the new prefix inspector intotest/unit/ipc/writer/compression-codecs.ts, dropping a duplicate copy fromfile-writer-tests.tsCross-language interop verified against PyArrow 25.0.1.
Test Plan
npm testnpm run buildnpm run lint:ciRelated