Skip to content

Commit bde43f2

Browse files
committed
GH-1386: Zero-pad inline views during split and transfer
Clear the unused inline bytes in each transferred view rather than relying on newly allocated memory being zeroed. Exercise text and binary views with deterministically non-zero allocations and boundary-length values. Generated-by: Codex and DeepSeek
1 parent 617edd5 commit bde43f2

2 files changed

Lines changed: 171 additions & 0 deletions

File tree

‎vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthViewVector.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,6 +904,11 @@ private void splitAndTransferViewBufferAndDataBuffer(
904904
readPosition += LENGTH_WIDTH;
905905
// set data by copying the required portion from the source buffer
906906
target.viewBuffer.setBytes(writePosition, viewBuffer, readPosition, stringLength);
907+
// zero the unused bytes of the inline view: the target view buffer is freshly allocated
908+
// and the format requires the bytes after the value to be zero padded.
909+
if (stringLength < INLINE_SIZE) {
910+
target.viewBuffer.setZero(writePosition + stringLength, INLINE_SIZE - stringLength);
911+
}
907912
} else {
908913
// handle non-inline buffer
909914
final int readBufIndex =

‎vector/src/test/java/org/apache/arrow/vector/TestVariableWidthViewVector.java‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ public class TestVariableWidthViewVector {
8989
private static final byte[] STR7 = "K01234567891234567K".getBytes(StandardCharsets.UTF_8);
9090
// long string (length > 12)
9191
private static final byte[] STR8 = "M012345678912345678M".getBytes(StandardCharsets.UTF_8);
92+
// short value (length <= 12) used to verify the zero padding of inline views
93+
private static final byte[] PAD1 = "a".getBytes(StandardCharsets.UTF_8);
94+
// short value (length <= 12) used to verify the zero padding of inline views
95+
private static final byte[] PAD11 = "0123456789a".getBytes(StandardCharsets.UTF_8);
9296
private static final String EMPTY_SCHEMA_PATH = "";
9397

9498
private BufferAllocator allocator;
@@ -2938,4 +2942,166 @@ public void testValidateInvalidOffsets() {
29382942
assertTrue(e.getMessage().contains("index: 1024"));
29392943
}
29402944
}
2945+
2946+
/**
2947+
* Builds the source values used by the zero padding tests: a sentinel value that stays outside
2948+
* the split range followed by an empty value, a 1 byte, an 11 byte and a 12 byte value (all
2949+
* inline views), a 13 byte value (stored in a data buffer) and a null value.
2950+
*/
2951+
private static byte[][] zeroPaddingTestData() {
2952+
return new byte[][] {STR3, new byte[0], PAD1, PAD11, STR1, STR2, null};
2953+
}
2954+
2955+
/**
2956+
* ARROW-1386: the view format documented in {@link BaseVariableWidthViewVector} requires the
2957+
* bytes after an inline value to be zero padded. This helper checks the padding of every view
2958+
* transferred by {@link BaseVariableWidthViewVector#splitAndTransferTo(int, int,
2959+
* BaseVariableWidthViewVector)}, and verifies that the split keeps the value count, the null
2960+
* count and the source vector intact. Callers pass vectors whose allocator hands out buffers
2961+
* pre-filled with 0xff, so a missing write is reported instead of being hidden by memory that
2962+
* happens to be zeroed.
2963+
*/
2964+
private void testSplitAndTransferZeroPaddingHelper(
2965+
BaseVariableWidthViewVector targetVector,
2966+
BaseVariableWidthViewVector sourceVector,
2967+
int startIndex,
2968+
int length,
2969+
byte[][] data) {
2970+
final int elementSize = BaseVariableWidthViewVector.ELEMENT_SIZE;
2971+
final int lengthWidth = BaseVariableWidthViewVector.LENGTH_WIDTH;
2972+
final int prefixWidth = BaseVariableWidthViewVector.PREFIX_WIDTH;
2973+
final int inlineSize = BaseVariableWidthViewVector.INLINE_SIZE;
2974+
2975+
sourceVector.allocateNew(1024 * 10, 1024);
2976+
2977+
for (int i = 0; i < data.length; i++) {
2978+
if (data[i] == null) {
2979+
sourceVector.setNull(i);
2980+
} else {
2981+
sourceVector.set(i, data[i]);
2982+
}
2983+
}
2984+
sourceVector.setValueCount(data.length);
2985+
2986+
final ArrowBuf sourceViewBuffer = sourceVector.getDataBuffer();
2987+
final int sourceViewLength = (int) sourceViewBuffer.capacity();
2988+
final byte[] sourceViewBefore = new byte[sourceViewLength];
2989+
sourceViewBuffer.getBytes(0, sourceViewBefore, 0, sourceViewLength);
2990+
final int sourceViewRefCnt = sourceViewBuffer.refCnt();
2991+
2992+
sourceVector.splitAndTransferTo(startIndex, length, targetVector);
2993+
2994+
assertEquals(length, targetVector.getValueCount());
2995+
// the view buffer is allocated for the target vector, it is not shared with the source
2996+
assertEquals(sourceViewRefCnt, sourceVector.getDataBuffer().refCnt());
2997+
final ArrowBuf targetViewBuffer = targetVector.getDataBuffer();
2998+
assertTrue(targetViewBuffer.capacity() >= (long) length * elementSize);
2999+
3000+
final byte[] element = new byte[elementSize];
3001+
int expectedNullCount = 0;
3002+
for (int i = startIndex; i < startIndex + length; i++) {
3003+
final int targetIndex = i - startIndex;
3004+
final int offset = targetIndex * elementSize;
3005+
final byte[] expected = data[i];
3006+
3007+
if (expected == null) {
3008+
// the view content of a null value is unspecified, only the validity matters
3009+
expectedNullCount++;
3010+
assertTrue(targetVector.isNull(targetIndex));
3011+
continue;
3012+
}
3013+
3014+
assertFalse(targetVector.isNull(targetIndex));
3015+
assertArrayEquals(expected, targetVector.get(targetIndex));
3016+
assertEquals(expected.length, targetViewBuffer.getInt(offset));
3017+
targetViewBuffer.getBytes(offset, element, 0, elementSize);
3018+
3019+
if (expected.length <= inlineSize) {
3020+
assertArrayEquals(
3021+
expected, Arrays.copyOfRange(element, lengthWidth, lengthWidth + expected.length));
3022+
// the bytes following an inline value must be zero padded
3023+
for (int b = lengthWidth + expected.length; b < elementSize; b++) {
3024+
assertEquals(0, element[b], "inline view byte " + b + " of value " + i);
3025+
}
3026+
} else {
3027+
// long views keep the first bytes of the value as a prefix; the buffer index and the
3028+
// offset are an implementation detail of the target and are covered by the get() above
3029+
assertArrayEquals(
3030+
Arrays.copyOfRange(expected, 0, prefixWidth),
3031+
Arrays.copyOfRange(element, lengthWidth, lengthWidth + prefixWidth));
3032+
}
3033+
}
3034+
assertEquals(expectedNullCount, targetVector.getNullCount());
3035+
3036+
// the split does not modify the source vector
3037+
assertEquals(data.length, sourceVector.getValueCount());
3038+
final byte[] sourceViewAfter = new byte[sourceViewLength];
3039+
sourceVector.getDataBuffer().getBytes(0, sourceViewAfter, 0, sourceViewLength);
3040+
assertArrayEquals(sourceViewBefore, sourceViewAfter);
3041+
for (int i = 0; i < data.length; i++) {
3042+
if (data[i] == null) {
3043+
assertTrue(sourceVector.isNull(i));
3044+
} else {
3045+
assertArrayEquals(data[i], sourceVector.get(i));
3046+
}
3047+
}
3048+
}
3049+
3050+
/**
3051+
* ARROW-1386: split and transfer must zero pad the inline views of the target vector even when
3052+
* the allocated target view buffer contains stale bytes.
3053+
*/
3054+
@Test
3055+
public void testSplitAndTransferZeroPadsInlineViews() {
3056+
final byte[][] data = zeroPaddingTestData();
3057+
final int startIndex = 1;
3058+
final int length = 6;
3059+
3060+
try (final DirtyingRootAllocator dirtyAllocator = new DirtyingRootAllocator()) {
3061+
try (final ViewVarCharVector targetVector =
3062+
newViewVarCharVector("split-target", dirtyAllocator);
3063+
final ViewVarCharVector sourceVector =
3064+
newViewVarCharVector(EMPTY_SCHEMA_PATH, dirtyAllocator)) {
3065+
testSplitAndTransferZeroPaddingHelper(targetVector, sourceVector, startIndex, length, data);
3066+
}
3067+
assertEquals(0, dirtyAllocator.getAllocatedMemory());
3068+
}
3069+
}
3070+
3071+
/** ARROW-1386: same as {@link #testSplitAndTransferZeroPadsInlineViews()} for binary views. */
3072+
@Test
3073+
public void testSplitAndTransferZeroPadsInlineViewsBinary() {
3074+
final byte[][] data = zeroPaddingTestData();
3075+
final int startIndex = 1;
3076+
final int length = 6;
3077+
3078+
try (final DirtyingRootAllocator dirtyAllocator = new DirtyingRootAllocator()) {
3079+
try (final ViewVarBinaryVector targetVector =
3080+
newViewVarBinaryVector("split-target", dirtyAllocator);
3081+
final ViewVarBinaryVector sourceVector =
3082+
newViewVarBinaryVector(EMPTY_SCHEMA_PATH, dirtyAllocator)) {
3083+
testSplitAndTransferZeroPaddingHelper(targetVector, sourceVector, startIndex, length, data);
3084+
}
3085+
assertEquals(0, dirtyAllocator.getAllocatedMemory());
3086+
}
3087+
}
3088+
3089+
/**
3090+
* Allocator that fills every buffer it hands out with 0xff. Tests using it do not depend on the
3091+
* content of recycled or freshly mapped memory, so bytes the vector code fails to write (for
3092+
* instance a missing inline view padding) are always visible.
3093+
*/
3094+
private static final class DirtyingRootAllocator extends RootAllocator {
3095+
3096+
DirtyingRootAllocator() {
3097+
super(Integer.MAX_VALUE);
3098+
}
3099+
3100+
@Override
3101+
public ArrowBuf buffer(final long initialRequestSize) {
3102+
final ArrowBuf buffer = super.buffer(initialRequestSize);
3103+
buffer.setOne(0L, buffer.capacity());
3104+
return buffer;
3105+
}
3106+
}
29413107
}

0 commit comments

Comments
 (0)