Skip to content

GH-1196: Write NO_COMPRESSION_LENGTH sentinel for empty buffers in compress() - #1248

Open
PG1204 wants to merge 1 commit into
apache:mainfrom
PG1204:gh-1196-empty-buffer-compression
Open

GH-1196: Write NO_COMPRESSION_LENGTH sentinel for empty buffers in compress()#1248
PG1204 wants to merge 1 commit into
apache:mainfrom
PG1204:gh-1196-empty-buffer-compression

Conversation

@PG1204

@PG1204 PG1204 commented Jul 24, 2026

Copy link
Copy Markdown

What's Changed

AbstractCompressionCodec.compress() wrote a length prefix of 0 for empty buffers. The Arrow IPC compression format only defines a positive length (compressed) or -1 / NO_COMPRESSION_LENGTH (stored uncompressed); a 0 prefix is undefined. As a result, C++/PyArrow reject Java-produced streams that contain empty buffers, even though Java itself accepts them on read.

This changes the empty-buffer shortcut in compress() to write CompressionUtil.NO_COMPRESSION_LENGTH (-1) instead of 0.

decompress() is intentionally left unchanged:

  • its NO_COMPRESSION_LENGTH branch handles the new output, and
  • its decompressedLength == 0 branch remains for backward-compatible reads of older Java-produced streams.

Added a parameterized test (testEmptyBufferWritesUncompressedSentinel, covering LZ4 and ZSTD) that compresses an empty buffer, asserts the prefix equals NO_COMPRESSION_LENGTH, and asserts it round-trips back to a zero-length buffer. The pre-existing testEmptyBuffer continues to pass with empty buffers now routed through the -1 path.

Closes #1196.

@github-actions

This comment has been minimized.

@PG1204

PG1204 commented Jul 24, 2026

Copy link
Copy Markdown
Author

Notes for reviewers

  • Empty-buffer path uses setLong(0, ...) directly, not the endianness-aware writeUncompressedLength(). This is safe: reverseBytes(-1) == -1, so the sentinel reads back correctly on big-endian too. It also matches the existing code, which wrote 0 the same way.
  • Test ownership: with the -1 sentinel, decompress() now routes through the NO_COMPRESSION_LENGTH branch -> extractUncompressedBuffer(), which returns a slice sharing the compressed buffer's reference manager. The test closes only the decompressed slice (which releases the shared allocation); closing the original compressed buffer as well would be a double-free. Verified leak-free via the RootAllocator check in @AfterEach.

@lidavidm lidavidm added the bug-fix PRs that fix a big. label Jul 27, 2026
@github-actions github-actions Bot added this to the 20.0.0 milestone Jul 27, 2026

@lidavidm lidavidm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems reasonable to me.

@lidavidm

Copy link
Copy Markdown
Member

@PG1204 can you rebase to pick up CI fixes?

@PG1204
PG1204 force-pushed the gh-1196-empty-buffer-compression branch from 547cec8 to 9c7c63d Compare August 25, 2026 01:30
@PG1204

PG1204 commented Aug 25, 2026

Copy link
Copy Markdown
Author

@PG1204 can you rebase to pick up CI fixes?

@lidavidm rebase done.

@PG1204
PG1204 requested a review from lidavidm August 25, 2026 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PRs that fix a big.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Java][IPC] AbstractCompressionCodec.compress() writes prefix=0 for empty buffers, incompatible with C++/Python readers

2 participants