Skip to content

Commit b0e51af

Browse files
authored
GH-1116: [Java] Fix compressed buffer prefix write and ZSTD dstCapacity (#1119)
## What Two fixes in the compression codec: 1. **`AbstractCompressionCodec.compress()`: capture `writerIndex()` once** The previous code read `uncompressedBuffer.writerIndex()` at multiple sites — for the size comparison and again after `doCompress()` to populate the 8-byte uncompressed-length prefix. Capture the value once at the top of `compress()` and reuse it for the empty-buffer check, the size comparison, and the prefix, so all three consumers see the same value. 2. **`ZstdCompressionCodec.doCompress()`: `dstCapacity` overstated by 8 bytes** `Zstd.compressUnsafe(dst, dstSize, ...)` expects `dstSize` to be the available space from `dst`. The code offsets `dst` by 8 bytes past the prefix but passed `8 + maxSize` instead of `maxSize`. The `compressBound()` headroom hides this in practice, but the parameter was semantically wrong. Pass `maxSize`. ## Tests Covered by the existing round-trip tests (`testEmptyBuffer`, `testReadWriteStream`, `testReadWriteFile`, etc.). I was not able to construct a minimal reproducer for the original `declaredUncompressed=0` symptom on the unfixed code, so both fixes are conservative correctness improvements derived from code inspection rather than failing-then-green regression tests.
1 parent 9122af2 commit b0e51af

2 files changed

Lines changed: 6 additions & 3 deletions

File tree

compression/src/main/java/org/apache/arrow/compression/ZstdCompressionCodec.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ protected ArrowBuf doCompress(BufferAllocator allocator, ArrowBuf uncompressedBu
4444
long bytesWritten =
4545
Zstd.compressUnsafe(
4646
compressedBuffer.memoryAddress() + CompressionUtil.SIZE_OF_UNCOMPRESSED_LENGTH,
47-
dstSize,
47+
maxSize,
4848
/*src*/ uncompressedBuffer.memoryAddress(),
4949
/* srcSize= */ uncompressedBuffer.writerIndex(),
5050
/* level= */ this.compressionLevel);

vector/src/main/java/org/apache/arrow/vector/compression/AbstractCompressionCodec.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ public abstract class AbstractCompressionCodec implements CompressionCodec {
2929

3030
@Override
3131
public ArrowBuf compress(BufferAllocator allocator, ArrowBuf uncompressedBuffer) {
32-
if (uncompressedBuffer.writerIndex() == 0L) {
32+
// GH-1116: capture writerIndex() once so the empty-buffer check, size
33+
// comparison, and uncompressed-length prefix all see the same value.
34+
long uncompressedLength = uncompressedBuffer.writerIndex();
35+
36+
if (uncompressedLength == 0L) {
3337
// shortcut for empty buffer
3438
ArrowBuf compressedBuffer = allocator.buffer(CompressionUtil.SIZE_OF_UNCOMPRESSED_LENGTH);
3539
compressedBuffer.setLong(0, 0);
@@ -41,7 +45,6 @@ public ArrowBuf compress(BufferAllocator allocator, ArrowBuf uncompressedBuffer)
4145
ArrowBuf compressedBuffer = doCompress(allocator, uncompressedBuffer);
4246
long compressedLength =
4347
compressedBuffer.writerIndex() - CompressionUtil.SIZE_OF_UNCOMPRESSED_LENGTH;
44-
long uncompressedLength = uncompressedBuffer.writerIndex();
4548

4649
if (compressedLength > uncompressedLength) {
4750
// compressed buffer is larger, send the raw buffer

0 commit comments

Comments
 (0)