diff --git a/paimon-common/src/main/java/org/apache/paimon/data/columnar/heap/HeapBytesVector.java b/paimon-common/src/main/java/org/apache/paimon/data/columnar/heap/HeapBytesVector.java index 3c57c9bd8003..cbea2f364088 100644 --- a/paimon-common/src/main/java/org/apache/paimon/data/columnar/heap/HeapBytesVector.java +++ b/paimon-common/src/main/java/org/apache/paimon/data/columnar/heap/HeapBytesVector.java @@ -74,8 +74,6 @@ public void reset() { } // We don't reset buffer to avoid unnecessary copy. - Arrays.fill(buffer, (byte) 0); - this.bytesAppended = 0; } diff --git a/paimon-common/src/test/java/org/apache/paimon/data/columnar/heap/HeapBytesVectorReserveBytesTest.java b/paimon-common/src/test/java/org/apache/paimon/data/columnar/heap/HeapBytesVectorReserveBytesTest.java index b02c745968f5..ac39cded4d7d 100644 --- a/paimon-common/src/test/java/org/apache/paimon/data/columnar/heap/HeapBytesVectorReserveBytesTest.java +++ b/paimon-common/src/test/java/org/apache/paimon/data/columnar/heap/HeapBytesVectorReserveBytesTest.java @@ -179,4 +179,20 @@ void testResetClearsBytesAppended() { assertThat(bytes.len).isEqualTo(2); assertThat(vector.buffer[0]).isEqualTo((byte) 10); } + + @Test + void testResetDoesNotWipeBuffer() { + HeapBytesVector vector = new HeapBytesVector(4); + byte[] data = new byte[] {1, 2, 3}; + vector.putByteArray(0, data, 0, data.length); + + vector.reset(); + + // reset() deliberately leaves the data buffer untouched: wiping it costs + // O(buffer size) per batch in the vectorized reader hot path, and reads are + // always bounded by the start/length offsets, which reset() does clear + assertThat(vector.buffer[0]).isEqualTo((byte) 1); + assertThat(vector.start[0]).isZero(); + assertThat(vector.length[0]).isZero(); + } } diff --git a/paimon-format/src/main/java/org/apache/paimon/format/parquet/reader/VectorizedDeltaByteArrayReader.java b/paimon-format/src/main/java/org/apache/paimon/format/parquet/reader/VectorizedDeltaByteArrayReader.java index 05ca1bacbbc8..41a7cdedf066 100644 --- a/paimon-format/src/main/java/org/apache/paimon/format/parquet/reader/VectorizedDeltaByteArrayReader.java +++ b/paimon-format/src/main/java/org/apache/paimon/format/parquet/reader/VectorizedDeltaByteArrayReader.java @@ -93,11 +93,11 @@ private void readValues(int total, WritableBytesVector c, int rowId) { c.putByteArray(rowId + i, bytes, offset, length); // Keep the value we just assembled rather than a view over the output vector's - // buffer. The record reader resets that vector between batches, and reset() zeroes - // the buffer, so a view into it would hand NUL bytes to the next value's prefix - // whenever a page spans more than one batch. skipBinary avoids the same hazard by - // alternating two vectors; here the array is already a fresh copy, so wrapping it - // costs nothing. + // buffer. The record reader reuses that vector across batches, rewriting it + // from offset 0, so a view into it would hand the next batch's bytes to the + // next value's prefix whenever a page spans more than one batch. skipBinary + // avoids the same hazard by alternating two vectors; here the array is + // already a fresh copy, so wrapping it costs nothing. previous = ByteBuffer.wrap(bytes); currentRow++; } diff --git a/paimon-format/src/test/java/org/apache/paimon/format/parquet/reader/DeltaByteArrayEncodingTest.java b/paimon-format/src/test/java/org/apache/paimon/format/parquet/reader/DeltaByteArrayEncodingTest.java index f1330f2f9cd0..f21e6db87016 100644 --- a/paimon-format/src/test/java/org/apache/paimon/format/parquet/reader/DeltaByteArrayEncodingTest.java +++ b/paimon-format/src/test/java/org/apache/paimon/format/parquet/reader/DeltaByteArrayEncodingTest.java @@ -102,6 +102,23 @@ public void testNegativeSize() { Integer.MAX_VALUE - 1)); } + /** + * skipBinary alternates two vectors and leaves {@code previous} pointing into the buffer of + * whichever one it wrote last, so after an odd number of skipped values the next skip call + * starts by resetting that very vector. The prefix of the following value is copied out of it. + */ + @Test + public void skippingAnOddNumberOfValuesKeepsThePrefix() throws Exception { + String[] vals = new String[] {"aaaa", "aaab", "aaac", "aaad"}; + Utils.writeData(writer, vals); + reader.initFromPage(vals.length, writer.getBytes().toInputStream()); + + reader.skipBinary(1); + reader.skipBinary(1); + + assertArrayEquals(vals[2].getBytes(), reader.readBinary(0).getBytes()); + } + /** * The record reader resets the vector between batches, so a page that spans two batches has to * survive that reset. Every other case here reads a whole page into one vector without a reset,