jbonofre commented on code in PR #1425:
URL: https://github.com/apache/arrow-java/pull/1425#discussion_r4438977771
##########
vector/src/main/java/org/apache/arrow/vector/VectorUnloader.java:
##########
@@ -137,8 +149,65 @@ 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) {
+ appendRebased(child, (int) coveredChildLength, nodes, buffers,
variadicBufferCounts);
Review Comment:
`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(0)` 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:
```java
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 😄
--
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]
