Skip to content

Commit 974a9a7

Browse files
committed
fix tests
1 parent 1a009d1 commit 974a9a7

4 files changed

Lines changed: 91 additions & 25 deletions

File tree

vector/src/main/java/org/apache/arrow/vector/complex/LargeListVector.java

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,13 @@ public static LargeListVector empty(String name, BufferAllocator allocator) {
105105
/** The maximum index that is actually set. */
106106
private int lastSet;
107107

108+
/**
109+
* Temporary offset buffer used only for serialization of a never-allocated vector (see {@link
110+
* #getFieldBuffers()}). Owned by this vector so it is released in {@link #clear()} rather than
111+
* leaked.
112+
*/
113+
private ArrowBuf serializationOffsetBuffer;
114+
108115
/**
109116
* Constructs a new instance.
110117
*
@@ -134,6 +141,7 @@ public LargeListVector(Field field, BufferAllocator allocator, CallBack callBack
134141
BitVectorHelper.getValidityBufferSizeFromCount(INITIAL_VALUE_ALLOCATION);
135142
this.lastSet = -1;
136143
this.offsetBuffer = allocator.getEmpty();
144+
this.serializationOffsetBuffer = allocator.getEmpty();
137145
this.vector = vector == null ? DEFAULT_DATA_VECTOR : vector;
138146
this.valueCount = 0;
139147
}
@@ -277,10 +285,23 @@ public List<ArrowBuf> getFieldBuffers() {
277285
List<ArrowBuf> result = new ArrayList<>(2);
278286
setReaderAndWriterIndex();
279287
result.add(validityBuffer);
288+
// A never-allocated vector has an empty (capacity 0) offset buffer, yet setReaderAndWriterIndex
289+
// marks OFFSET_WIDTH bytes as written so that serializers still emit offset[0] = 0 (an empty
290+
// offset buffer would crash IPC readers in other libraries). Serializers read `writerIndex`
291+
// bytes, so we must hand them a properly sized buffer. Mirror exportCDataBuffers() by
292+
// substituting a temporary buffer instead of mutating this.offsetBuffer, which validation and
293+
// subsequent writes still rely on being empty. The temporary is owned by this vector and
294+
// released in clear()/close(), so it is not leaked.
280295
if (offsetBuffer.capacity() == 0 && offsetBuffer.writerIndex() > 0) {
281-
ArrowBuf tempOffset = allocateOffsetBuffer(offsetBuffer.writerIndex());
282-
tempOffset.writerIndex(offsetBuffer.writerIndex());
283-
result.add(tempOffset);
296+
// Allocate directly rather than via allocateOffsetBuffer(), which would overwrite
297+
// offsetAllocationSizeInBytes and shrink a later allocateNew()'s offset buffer.
298+
final long size = offsetBuffer.writerIndex();
299+
serializationOffsetBuffer = releaseBuffer(serializationOffsetBuffer);
300+
serializationOffsetBuffer = allocator.buffer(size);
301+
serializationOffsetBuffer.readerIndex(0);
302+
serializationOffsetBuffer.setZero(0, serializationOffsetBuffer.capacity());
303+
serializationOffsetBuffer.writerIndex(size);
304+
result.add(serializationOffsetBuffer);
284305
} else {
285306
result.add(offsetBuffer);
286307
}
@@ -810,6 +831,7 @@ public void clear() {
810831
valueCount = 0;
811832
super.clear();
812833
validityBuffer = releaseBuffer(validityBuffer);
834+
serializationOffsetBuffer = releaseBuffer(serializationOffsetBuffer);
813835
lastSet = -1;
814836
}
815837

vector/src/main/java/org/apache/arrow/vector/complex/ListVector.java

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,13 @@ public static ListVector empty(String name, BufferAllocator allocator) {
8282
/** The maximum index that is actually set. */
8383
protected int lastSet;
8484

85+
/**
86+
* Temporary offset buffer used only for serialization of a never-allocated vector (see {@link
87+
* #getFieldBuffers()}). Owned by this vector so it is released in {@link #clear()} rather than
88+
* leaked.
89+
*/
90+
private ArrowBuf serializationOffsetBuffer;
91+
8592
/**
8693
* Constructs a new instance.
8794
*
@@ -110,6 +117,7 @@ public ListVector(Field field, BufferAllocator allocator, CallBack callBack) {
110117
this.validityAllocationSizeInBytes =
111118
BitVectorHelper.getValidityBufferSizeFromCount(INITIAL_VALUE_ALLOCATION);
112119
this.lastSet = -1;
120+
this.serializationOffsetBuffer = allocator.getEmpty();
113121
}
114122

115123
@Override
@@ -235,10 +243,23 @@ public List<ArrowBuf> getFieldBuffers() {
235243
List<ArrowBuf> result = new ArrayList<>(2);
236244
setReaderAndWriterIndex();
237245
result.add(validityBuffer);
246+
// A never-allocated vector has an empty (capacity 0) offset buffer, yet setReaderAndWriterIndex
247+
// marks OFFSET_WIDTH bytes as written so that serializers still emit offset[0] = 0 (an empty
248+
// offset buffer would crash IPC readers in other libraries). Serializers read `writerIndex`
249+
// bytes, so we must hand them a properly sized buffer. Mirror exportCDataBuffers() by
250+
// substituting a temporary buffer instead of mutating this.offsetBuffer, which validation and
251+
// subsequent writes still rely on being empty. The temporary is owned by this vector and
252+
// released in clear()/close(), so it is not leaked.
238253
if (offsetBuffer.capacity() == 0 && offsetBuffer.writerIndex() > 0) {
239-
ArrowBuf tempOffset = allocateOffsetBuffer(offsetBuffer.writerIndex());
240-
tempOffset.writerIndex(offsetBuffer.writerIndex());
241-
result.add(tempOffset);
254+
// Allocate directly rather than via allocateOffsetBuffer(), which would overwrite
255+
// offsetAllocationSizeInBytes and shrink a later allocateNew()'s offset buffer.
256+
final long size = offsetBuffer.writerIndex();
257+
serializationOffsetBuffer = releaseBuffer(serializationOffsetBuffer);
258+
serializationOffsetBuffer = allocator.buffer(size);
259+
serializationOffsetBuffer.readerIndex(0);
260+
serializationOffsetBuffer.setZero(0, serializationOffsetBuffer.capacity());
261+
serializationOffsetBuffer.writerIndex(size);
262+
result.add(serializationOffsetBuffer);
242263
} else {
243264
result.add(offsetBuffer);
244265
}
@@ -657,6 +678,7 @@ public MinorType getMinorType() {
657678
public void clear() {
658679
super.clear();
659680
validityBuffer = releaseBuffer(validityBuffer);
681+
serializationOffsetBuffer = releaseBuffer(serializationOffsetBuffer);
660682
lastSet = -1;
661683
}
662684

vector/src/test/java/org/apache/arrow/vector/TestLargeListVector.java

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,35 +1123,47 @@ public void testEmptyLargeListOffsetBuffer() {
11231123
@Test
11241124
public void testEmptyLargeListOffsetBufferWithoutAllocate() {
11251125
// Regression test for the Arrow 19 IOOBE: a never-allocated LargeListVector must still produce
1126-
// a valid offset buffer after setValueCount(0). Without the realloc guard in
1127-
// setReaderAndWriterIndex(), this sets writerIndex=8 on a capacity-0 buffer.
1126+
// a valid offset buffer for serialization after setValueCount(0). getFieldBuffers() substitutes
1127+
// a properly sized temporary offset buffer (holding offset[0] = 0) without mutating the
1128+
// vector's own capacity-0 offset buffer.
11281129
try (LargeListVector list = LargeListVector.empty("list", allocator)) {
11291130
list.addOrGetVector(FieldType.nullable(MinorType.INT.getType()));
11301131
list.setValueCount(0); // no allocateNew() — offset buffer starts at capacity 0
11311132

11321133
List<ArrowBuf> buffers = list.getFieldBuffers();
1134+
ArrowBuf offsetBuffer = buffers.get(1);
11331135
assertTrue(
1134-
buffers.get(1).readableBytes() >= LargeListVector.OFFSET_WIDTH,
1136+
offsetBuffer.readableBytes() >= LargeListVector.OFFSET_WIDTH,
11351137
"Offset buffer should have at least "
11361138
+ LargeListVector.OFFSET_WIDTH
11371139
+ " bytes for offset[0]");
1138-
assertEquals(0L, list.getOffsetBuffer().getLong(0));
1140+
assertEquals(0L, offsetBuffer.getLong(0));
1141+
// The vector's own offset buffer is left untouched so subsequent writes still work.
1142+
assertEquals(0, list.getOffsetBuffer().capacity());
11391143
}
11401144
}
11411145

11421146
@Test
11431147
public void testEmptyLargeListGetBuffersWithoutAllocate() {
1144-
// Exercises the getBuffers(false) entry point — the IPC serialization path.
1148+
// Exercises the IPC serialization entry points — getBuffers(false) and getFieldBuffers(), the
1149+
// latter being the path that produced the original Netty IOOBE.
11451150
try (LargeListVector list = LargeListVector.empty("list", allocator)) {
11461151
list.addOrGetVector(FieldType.nullable(MinorType.INT.getType()));
11471152
list.setValueCount(0);
11481153

1154+
// getBufferSize() returns 0 for valueCount==0, so getBuffers returns an empty array and must
1155+
// not crash on the never-allocated offset buffer.
11491156
ArrowBuf[] bufs = list.getBuffers(false);
1157+
assertEquals(0, bufs.length);
1158+
1159+
// getFieldBuffers() must hand serializers a readable offset buffer holding offset[0] = 0.
1160+
List<ArrowBuf> fieldBuffers = list.getFieldBuffers();
11501161
assertTrue(
1151-
list.getOffsetBuffer().capacity() >= LargeListVector.OFFSET_WIDTH,
1152-
"Offset buffer capacity should be >= "
1162+
fieldBuffers.get(1).readableBytes() >= LargeListVector.OFFSET_WIDTH,
1163+
"Offset buffer should be readable for >= "
11531164
+ LargeListVector.OFFSET_WIDTH
1154-
+ " after setReaderAndWriterIndex");
1165+
+ " bytes");
1166+
assertEquals(0L, fieldBuffers.get(1).getLong(0));
11551167
}
11561168
}
11571169

vector/src/test/java/org/apache/arrow/vector/TestListVector.java

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1402,38 +1402,48 @@ public void testEmptyListOffsetBuffer() {
14021402
@Test
14031403
public void testEmptyListOffsetBufferWithoutAllocate() {
14041404
// Regression test for the Arrow 19 IOOBE: a never-allocated ListVector must still produce
1405-
// a valid offset buffer after setValueCount(0). Without the realloc guard in
1406-
// setReaderAndWriterIndex(), this sets writerIndex=4 on a capacity-0 buffer.
1405+
// a valid offset buffer for serialization after setValueCount(0). getFieldBuffers() substitutes
1406+
// a properly sized temporary offset buffer (holding offset[0] = 0) without mutating the
1407+
// vector's own capacity-0 offset buffer.
14071408
try (ListVector list = ListVector.empty("list", allocator)) {
14081409
list.addOrGetVector(FieldType.nullable(MinorType.INT.getType()));
14091410
list.setValueCount(0); // no allocateNew() — offset buffer starts at capacity 0
14101411

14111412
List<ArrowBuf> buffers = list.getFieldBuffers();
1413+
ArrowBuf offsetBuffer = buffers.get(1);
14121414
assertTrue(
1413-
buffers.get(1).readableBytes() >= BaseRepeatedValueVector.OFFSET_WIDTH,
1415+
offsetBuffer.readableBytes() >= BaseRepeatedValueVector.OFFSET_WIDTH,
14141416
"Offset buffer should have at least "
14151417
+ BaseRepeatedValueVector.OFFSET_WIDTH
14161418
+ " bytes for offset[0]");
1417-
assertEquals(0, list.getOffsetBuffer().getInt(0));
1419+
assertEquals(0, offsetBuffer.getInt(0));
1420+
// The vector's own offset buffer is left untouched so subsequent writes still work.
1421+
assertEquals(0, list.getOffsetBuffer().capacity());
14181422
}
14191423
}
14201424

14211425
@Test
14221426
public void testEmptyListGetBuffersWithoutAllocate() {
1423-
// Exercises the getBuffers(false) entry point — the IPC serialization path that produced the
1424-
// original Netty IOOBE via VectorUnloader -> NettyArrowBuf.unwrapBuffer().
1427+
// Exercises the IPC serialization entry points — getBuffers(false) and getFieldBuffers(), the
1428+
// latter being the path that produced the original Netty IOOBE via
1429+
// VectorUnloader -> NettyArrowBuf.unwrapBuffer().
14251430
try (ListVector list = ListVector.empty("list", allocator)) {
14261431
list.addOrGetVector(FieldType.nullable(MinorType.INT.getType()));
14271432
list.setValueCount(0);
14281433

1434+
// getBufferSize() returns 0 for valueCount==0, so getBuffers returns an empty array and must
1435+
// not crash on the never-allocated offset buffer.
14291436
ArrowBuf[] bufs = list.getBuffers(false);
1430-
// getBufferSize() returns 0 for valueCount==0, so getBuffers returns empty array.
1431-
// But the offset buffer on the vector itself must have been grown to valid capacity.
1437+
assertEquals(0, bufs.length);
1438+
1439+
// getFieldBuffers() must hand serializers a readable offset buffer holding offset[0] = 0.
1440+
List<ArrowBuf> fieldBuffers = list.getFieldBuffers();
14321441
assertTrue(
1433-
list.getOffsetBuffer().capacity() >= BaseRepeatedValueVector.OFFSET_WIDTH,
1434-
"Offset buffer capacity should be >= "
1442+
fieldBuffers.get(1).readableBytes() >= BaseRepeatedValueVector.OFFSET_WIDTH,
1443+
"Offset buffer should be readable for >= "
14351444
+ BaseRepeatedValueVector.OFFSET_WIDTH
1436-
+ " after setReaderAndWriterIndex");
1445+
+ " bytes");
1446+
assertEquals(0, fieldBuffers.get(1).getInt(0));
14371447
}
14381448
}
14391449

0 commit comments

Comments
 (0)