Skip to content

GH-1424: Write only the values a vector's offsets cover in VectorUnloader #1425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jbonofre merged 3 commits into from
Nov 2, 2026
Merged

GH-1424: Write only the values a vector's offsets cover in VectorUnloader #1425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f78cd30
fix(vector): write only the values a vector's offsets cover when unlo…
andygrove Oct 31, 2026
d811f92
fix(vector): handle unions, empty lists and split failures when re-ba…
andygrove Nov 1, 2026
bc8bd9c
fix(vector): reallocate LargeListVector offsets that cannot hold [0]
jbonofre Nov 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 100 additions & 1 deletion vector/src/main/java/org/apache/arrow/vector/VectorUnloader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,16 @@
import java.util.List;
import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.util.AutoCloseables;
import org.apache.arrow.vector.complex.FixedSizeListVector;
import org.apache.arrow.vector.complex.LargeListVector;
import org.apache.arrow.vector.complex.ListVector;
import org.apache.arrow.vector.complex.NonNullableStructVector;
import org.apache.arrow.vector.compression.CompressionCodec;
import org.apache.arrow.vector.compression.CompressionUtil;
import org.apache.arrow.vector.compression.NoCompressionCodec;
import org.apache.arrow.vector.ipc.message.ArrowFieldNode;
import org.apache.arrow.vector.ipc.message.ArrowRecordBatch;
import org.apache.arrow.vector.util.TransferPair;

/**
* Helper class that handles converting a {@link VectorSchemaRoot} to a {@link ArrowRecordBatch}.
Expand Down Expand Up @@ -74,6 +79,12 @@ public VectorUnloader(
/**
* Performs the depth first traversal of the Vectors to create an {@link ArrowRecordBatch}
* suitable for serialization.
*
* <p>A variable-width, list or map vector whose offsets start past zero, or whose child vector
* holds elements past its last offset, as one imported through the C Data interface from a sliced
* array does, is written as the values between its first and last offset, with the offsets
* re-based to start at zero. Vectors with a union, view or extension vector at or below them are
* written as they are.
*/
public ArrowRecordBatch getRecordBatch() {
List<ArrowFieldNode> nodes = new ArrayList<>();
Expand Down Expand Up @@ -111,6 +122,10 @@ private void appendNodes(
List<ArrowBuf> buffers,
List<Long> variadicBufferCounts) {
FieldVector storageVector = ExtensionTypeVector.getStorageVector(vector);
if (firstOffset(storageVector) != 0 && splitKeepsValues(vector)) {
appendRebased(vector, vector.getValueCount(), nodes, buffers, variadicBufferCounts);
return;
}
nodes.add(
new ArrowFieldNode(vector.getValueCount(), includeNullCount ? vector.getNullCount() : -1));
List<ArrowBuf> fieldBuffers = vector.getFieldBuffers();
Expand All @@ -137,8 +152,92 @@ private void appendNodes(
buf.getReferenceManager().retain();
buffers.add(codec.compress(vector.getAllocator(), buf));
}
long coveredChildLength = coveredChildLength(storageVector);
for (FieldVector child : vector.getChildrenFromFields()) {
appendNodes(child, nodes, buffers, variadicBufferCounts);
if (coveredChildLength >= 0
&& child.getValueCount() > coveredChildLength
&& splitKeepsValues(child)) {
appendRebased(child, (int) coveredChildLength, nodes, buffers, variadicBufferCounts);

@jbonofre jbonofre Oct 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the covered range is empty, this hands back a list without a valid offsets buffer.

If coveredChildLength is 0 and the child is a ListVector or MapVector, this calls splitAndTransfer(0, 0). That path only runs to.clear(), so the target's offsets stay as allocator.getEmpty(), the allocator's shared zero-capacity buffer. ListVector.getFieldBuffers() then calls writerIndex(4) on that shared buffer. The inner child's buffers are the same instance, so what gets written depends on which vector set the write index last:

It also leaves the shared empty buffer with writeIndex() == 4 for everything else using that allocator.

This doesn't need a C Data import. A Java-built list hits it once the row count is lowered:

// outer: List<List<Int>> with rows [], [], [[1]], [[2]]
outer.setValueCount(4);
root.setRowCount(2); // outer now covers 0 inner lists, but the inner list still holds 2
new VectorUnloader(root).getRecordBatch(); // inner offsets buffer has 0 bytes

The List<List<Null>> case is outer offsets [0, 0, 0] over a 2-value inner list. main writes both correctly.

LargeListVector's split already allocates an offsets buffer when length == 0, and BaseVariableWidthVector.setReaderAndWriterIndex() reallocates the offsets buffer when it's too small for [0].

Doing either in ListVector would fix this, and so would handling length == 0 here without splitting.

Maybe we could add a test with an empty covered range over a nested list child.

@andygrove andygrove Nov 1, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d811f92 with the BaseVariableWidthVector approach: ListVector.getFieldBuffers() now reallocates an offsets buffer that can't hold [0], as #989 did for variable-width vectors, and MapVector inherits it. Allocating in the zero-length split instead made every zero-length split allocate, which TestSplitAndTransfer.testWithEmptyVector caught as a leak in code that doesn't close the target. Handling length == 0 in the unloader alone wouldn't cover a list nested in a re-based list either, since that empty child comes out of ListVector's own split. Your List<List<Int>> case is now emptyCoveredRangeOverNestedListKeepsItsFirstOffset (it wrote 0 bytes for the inner offsets before), and TestListVector and TestMapVector check that a zero-length split's target writes [0] without touching the shared empty buffer.

@jbonofre jbonofre Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LargeListVector needs the same change. Otherwise the empty covered range case still writes a 0-byte offsets buffer when the nested list is a LargeList.

LargeListVector's own zero-length split allocates [0], but that doesn't help when the LargeListVector is the child of a ListVector split to zero length. ListVector's split only runs to.clear() and never splits its child, so the child keep allocator.getEmpty(). LargeListVector.setReaderAndWriterIndex() then calls writerIndex(8) on the shared buffer:

  • List<List<LargeList<Int>>> with outer offsets [0, 0, 0] over a 2 value middle list: the int child sets the shared buffer back to 0, so the LargeList offsets are written with 0 bytes.
  • List<List<LargeList<Null>>> nothing resets it, so the writer tries to read 8 bytes from a zero capacity buffer.

main writes both correctly, as it doesn't split the middle list.

I believe applying the same reallocation in LargeListVector.setReadersAndWriterIndex() fixes it:

final long requiredOffsetBufferSize = (long) (valueCount + 1) * OFFSET_WIDTH;
if (offsetBuffer.capacity() < requiredOffsetBufferSize) {
  // An empty vector may still hold the allocator's shared empty buffer
  ArrowBuf newOffsetBuffer = allocator.buffer(requiredOffsetBufferSize);
  newOffsetBuffer.setZero(0, newOffsetBuffer.capacity());
  if (offsetBuffer.capacity() > 0) {
    newOffsetBuffer.setBytes(0, offsetBuffer, 0, offsetBuffer.capacity());
  }
  offsetBuffer.getReferenceManager().release();
  offsetBuffer = newOffsetBuffer;
}
offsetBuffer.writerIndex(requiredOffsetBufferSize);

Maybe we could add a LargeList variant of emptyCoveredRangeOverNestedListKeepsItsFirstOffset (the LargeList offsets should be written as 8 bytes holding 0), and a check in TestLargeListVector that an empty vector's getFieldBuffers() leaves allocator.getEmpty().writerIndex() at 0, as testSplitAndTransferZeroLength does for ListVector.

I can do that for you 😄

} else {
appendNodes(child, nodes, buffers, variadicBufferCounts);
}
}
}

/** Appends the first {@code length} values of the vector, re-based to start at offset zero. */
private void appendRebased(
FieldVector vector,
int length,
List<ArrowFieldNode> nodes,
List<ArrowBuf> buffers,
List<Long> variadicBufferCounts) {
TransferPair pair = vector.getTransferPair(vector.getField(), vector.getAllocator());
try (FieldVector rebased = (FieldVector) pair.getTo()) {
pair.splitAndTransfer(0, length);
appendNodes(rebased, nodes, buffers, variadicBufferCounts);
}
}

/**
* Whether splitting the vector keeps every value in its slot. A dense union's split does not when
* a type's offsets have gaps, so only the vector types listed here are re-based.
*/
private static boolean splitKeepsValues(FieldVector vector) {
if (vector instanceof BaseFixedWidthVector
|| vector instanceof BaseVariableWidthVector
|| vector instanceof BaseLargeVariableWidthVector
|| vector instanceof NullVector) {
return true;
}
if (vector instanceof ListVector
|| vector instanceof LargeListVector
|| vector instanceof FixedSizeListVector
|| vector instanceof NonNullableStructVector) {
for (FieldVector child : vector.getChildrenFromFields()) {
if (!splitKeepsValues(child)) {
return false;
}
}
return true;
}
return false;
}

/** The first offset of a variable-width, list or map vector, or zero for any other vector. */
private static long firstOffset(FieldVector vector) {
if (vector.getValueCount() == 0) {
return 0;
}
if (vector instanceof BaseVariableWidthVector) {
return ((BaseVariableWidthVector) vector).getStartOffset(0);
}
if (vector instanceof BaseLargeVariableWidthVector) {
return ((BaseLargeVariableWidthVector) vector).getStartOffset(0);
}
if (vector instanceof ListVector) {
return ((ListVector) vector).getElementStartIndex(0);
}
if (vector instanceof LargeListVector) {
return ((LargeListVector) vector).getElementStartIndex(0);
}
return 0;
}

/**
* The number of child elements a list or map vector's offsets cover, which is its last offset, or
* -1 for a vector whose child is not addressed by offsets.
*/
private static long coveredChildLength(FieldVector vector) {
int valueCount = vector.getValueCount();
if (valueCount == 0) {
return -1;
}
if (vector instanceof ListVector) {
return ((ListVector) vector).getElementEndIndex(valueCount - 1);
}
if (vector instanceof LargeListVector) {
return ((LargeListVector) vector).getElementEndIndex(valueCount - 1);
}
return -1;
}
}
13 changes: 12 additions & 1 deletion vector/src/main/java/org/apache/arrow/vector/complex/LargeListVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,18 @@ private void setReaderAndWriterIndex() {
// Both are set to 0 means 0 bytes are written to the IPC stream which will crash IPC readers
// in other libraries. According to Arrow spec, we should still output the offset buffer which
// is [0].
offsetBuffer.writerIndex((long) (valueCount + 1) * OFFSET_WIDTH);
final long requiredOffsetBufferSize = (long) (valueCount + 1) * OFFSET_WIDTH;
if (offsetBuffer.capacity() < requiredOffsetBufferSize) {
// An empty vector may still hold the allocator's shared empty buffer
ArrowBuf newOffsetBuffer = allocator.buffer(requiredOffsetBufferSize);
newOffsetBuffer.setZero(0, newOffsetBuffer.capacity());
if (offsetBuffer.capacity() > 0) {
newOffsetBuffer.setBytes(0, offsetBuffer, 0, offsetBuffer.capacity());
}
offsetBuffer.getReferenceManager().release();
offsetBuffer = newOffsetBuffer;
}
offsetBuffer.writerIndex(requiredOffsetBufferSize);
}

/**
Expand Down
13 changes: 12 additions & 1 deletion vector/src/main/java/org/apache/arrow/vector/complex/ListVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,18 @@ private void setReaderAndWriterIndex() {
// Both are set to 0 means 0 bytes are written to the IPC stream which will crash IPC readers
// in other libraries. According to Arrow spec, we should still output the offset buffer which
// is [0].
offsetBuffer.writerIndex((long) (valueCount + 1) * OFFSET_WIDTH);
final long requiredOffsetBufferSize = (long) (valueCount + 1) * OFFSET_WIDTH;
if (offsetBuffer.capacity() < requiredOffsetBufferSize) {
// An empty vector may still hold the allocator's shared empty buffer
ArrowBuf newOffsetBuffer = allocator.buffer(requiredOffsetBufferSize);
newOffsetBuffer.setZero(0, newOffsetBuffer.capacity());
if (offsetBuffer.capacity() > 0) {
newOffsetBuffer.setBytes(0, offsetBuffer, 0, offsetBuffer.capacity());
}
offsetBuffer.getReferenceManager().release();
offsetBuffer = newOffsetBuffer;
}
offsetBuffer.writerIndex(requiredOffsetBufferSize);
}

/**
Expand Down
13 changes: 13 additions & 0 deletions vector/src/test/java/org/apache/arrow/vector/TestLargeListVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1216,4 +1216,17 @@ public void testSetValueCountAcceptsMaxIntChildValueCount() {
}
}
}

@Test
public void testGetFieldBuffersOfEmptyVector() {
// An empty vector holds the allocator's shared empty buffer as its offsets until it allocates
try (LargeListVector vector = LargeListVector.empty("list", allocator)) {
vector.addOrGetVector(FieldType.nullable(MinorType.INT.getType()));
ArrowBuf offsets = vector.getFieldBuffers().get(1);
assertTrue(offsets.capacity() >= LargeListVector.OFFSET_WIDTH);
assertEquals(LargeListVector.OFFSET_WIDTH, offsets.readableBytes());
assertEquals(0L, offsets.getLong(0));
assertEquals(0, allocator.getEmpty().writerIndex());
}
}
}
23 changes: 23 additions & 0 deletions vector/src/test/java/org/apache/arrow/vector/TestListVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1406,4 +1406,27 @@ private void writeIntValues(UnionListWriter writer, int[] values) {
}
writer.endList();
}

@Test
public void testSplitAndTransferZeroLength() {
try (ListVector listVector = ListVector.empty("sourceVector", allocator)) {
listVector.addOrGetVector(FieldType.nullable(MinorType.INT.getType()));
UnionListWriter listWriter = listVector.getWriter();
listWriter.allocate();
listWriter.setPosition(0);
listWriter.startList();
listWriter.integer().writeInt(1);
listWriter.endList();
listVector.setValueCount(1);

TransferPair pair = listVector.getTransferPair(allocator);
pair.splitAndTransfer(0, 0);
try (ListVector to = (ListVector) pair.getTo()) {
ArrowBuf offsets = to.getFieldBuffers().get(1);
assertTrue(offsets.capacity() >= BaseRepeatedValueVector.OFFSET_WIDTH);
assertEquals(0, offsets.getInt(0));
assertEquals(0, allocator.getEmpty().writerIndex());
}
}
}
}
32 changes: 32 additions & 0 deletions vector/src/test/java/org/apache/arrow/vector/TestMapVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.UUID;
import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.vector.complex.BaseRepeatedValueVector;
import org.apache.arrow.vector.complex.MapVector;
import org.apache.arrow.vector.complex.StructVector;
import org.apache.arrow.vector.complex.impl.UnionMapReader;
Expand Down Expand Up @@ -1662,4 +1663,35 @@ public void testMapWithUuidKeyAndListUuidValue() throws Exception {
assertEquals(value2c, actualValue2c);
}
}

@Test
public void testSplitAndTransferZeroLength() {
try (MapVector mapVector = MapVector.empty("sourceVector", allocator, false)) {
FieldType type = new FieldType(false, ArrowType.Struct.INSTANCE, null, null);
AddOrGetResult<StructVector> addResult = mapVector.addOrGetVector(type);
FieldType keyType = new FieldType(false, MinorType.BIGINT.getType(), null, null);
FieldType valueType = FieldType.nullable(MinorType.FLOAT8.getType());
addResult.getVector().addOrGet(MapVector.KEY_NAME, keyType, BigIntVector.class);
addResult.getVector().addOrGet(MapVector.VALUE_NAME, valueType, Float8Vector.class);
UnionMapWriter mapWriter = mapVector.getWriter();
mapWriter.allocate();
mapWriter.setPosition(0);
mapWriter.startMap();
mapWriter.startEntry();
mapWriter.key().bigInt().writeBigInt(10);
mapWriter.value().float8().writeFloat8(1.0);
mapWriter.endEntry();
mapWriter.endMap();
mapVector.setValueCount(1);

TransferPair pair = mapVector.getTransferPair(allocator);
pair.splitAndTransfer(0, 0);
try (MapVector to = (MapVector) pair.getTo()) {
ArrowBuf offsets = to.getFieldBuffers().get(1);
assertTrue(offsets.capacity() >= BaseRepeatedValueVector.OFFSET_WIDTH);
assertEquals(0, offsets.getInt(0));
assertEquals(0, allocator.getEmpty().writerIndex());
}
}
}
}
Loading
Loading