LuciferYang opened a new pull request, #9644:
URL: https://github.com/apache/paimon/pull/9644

   ### Purpose
   
   close #9643
   
   `HeapBytesVector.reset()` zeroed its data buffer, one line under the comment 
saying it does not:
   
   ```java
   // We don't reset buffer to avoid unnecessary copy.
   Arrays.fill(buffer, (byte) 0);
   ```
   
   Beyond costing O(buffer size) per batch on the vectorized read path, it 
silently corrupts values. `VectorizedDeltaByteArrayReader.skipBinary` 
alternates two vectors and leaves `previous` pointing at the buffer of 
whichever one it wrote last, so after an odd number of skipped values 
`previous` points into `tempBinaryValVector` and the next `skipBinary` call 
opens by resetting that same vector. The prefix it then copies out of 
`previous` is all zeros, and the value that follows the skipped range comes 
back with NUL bytes in front.
   
   Removing the fill is safe: every read goes through `getBytes(i)`, bounded by 
the `start` and `length` arrays that `reset()` still clears, and null positions 
have length 0. Nothing in the repo reads the buffer directly except the two 
Parquet readers and `ColumnVectorUtils`, and since #9275 `readValues` keeps its 
own copy rather than a view into the vector.
   
   The comment in `VectorizedDeltaByteArrayReader` that explained the copy in 
terms of "reset() zeroes the buffer" is updated to say what is actually true 
now: the vector is rewritten across batches, so a view into it cannot be kept.
   
   ### Tests
   
   `DeltaByteArrayEncodingTest.skippingAnOddNumberOfValuesKeepsThePrefix` 
writes four values sharing a prefix, calls `skipBinary(1)` twice so the second 
call resets the vector `previous` points into, and reads the next value.
   
   Against the unfixed `HeapBytesVector` it fails with `array contents differ 
at index [0], expected: <97> but was: <0>`, which is the prefix that was 
zeroed. The existing `randomStringsWithSkip` and `randomStringsWithSkipN` never 
reach it, since they skip once.
   
   `HeapBytesVectorReserveBytesTest.testResetDoesNotWipeBuffer` pins the 
vector's own behavior.
   
   `mvn -pl paimon-common,paimon-format 
-Dtest=HeapBytesVectorReserveBytesTest,DeltaByteArrayEncodingTest test` on JDK 
8: 13 and 8 tests, 0 failures. `spotless:check` and `checkstyle:check` on both 
modules are clean. The red run needs the pre-fix paimon-common installed, since 
the two modules do not share a reactor for this.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to