This is an automated email from the ASF dual-hosted git repository.
JingsongLi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/paimon.git
The following commit(s) were added to refs/heads/master by this push:
new d9ca1bcf16 [common] Do not zero the HeapBytesVector buffer on reset
(#9644)
d9ca1bcf16 is described below
commit d9ca1bcf1646af8313c5b07527ac66299de11029
Author: YangJie <[email protected]>
AuthorDate: Thu Sep 10 02:35:50 2026 -0400
[common] Do not zero the HeapBytesVector buffer on reset (#9644)
---
.../paimon/data/columnar/heap/HeapBytesVector.java | 2 --
.../columnar/heap/HeapBytesVectorReserveBytesTest.java | 16 ++++++++++++++++
.../parquet/reader/VectorizedDeltaByteArrayReader.java | 10 +++++-----
.../parquet/reader/DeltaByteArrayEncodingTest.java | 17 +++++++++++++++++
4 files changed, 38 insertions(+), 7 deletions(-)
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 3c57c9bd80..cbea2f3640 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 class HeapBytesVector extends AbstractHeapVector
implements WritableBytes
}
// 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 b02c745968..ac39cded4d 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 @@ class HeapBytesVectorReserveBytesTest {
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 05ca1bacbb..41a7cdedf0 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 @@ public class VectorizedDeltaByteArrayReader extends
VectorizedReaderBase
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 f1330f2f9c..f21e6db870 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 class DeltaByteArrayEncodingTest {
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,
