FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-1196: Write NO_COMPRESSION_LENGTH sentinel for empty buffers in compress() by PG1204 · Pull Request #1248 · apache/arrow-java · GitHub

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 commented Jul 24, 2026
edited
Loading

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.

This comment has been minimized.

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 added the bug-fix PRs that fix a big. label Jul 27, 2026
github-actions Bot added this to the 20.0.0 milestone Jul 27, 2026

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Seems reasonable to me.

Copy link
Copy Markdown
Member

@PG1204 can you rebase to pick up CI fixes?

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

PG1204 commented Aug 25, 2026

Copy link
Copy Markdown
Author

@PG1204 can you rebase to pick up CI fixes?

@lidavidm rebase done.

PG1204 requested a review from lidavidm August 25, 2026 01:31
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

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


Back | FazBrowse Home | New Git URL