Skip to content

MINOR: Fix IOOBE in ListVector/LargeListVector.getFieldBuffers() for never-allocated offset buffer #27

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

Closed
prashanthbdremio wants to merge 11 commits into from
Closed

MINOR: Fix IOOBE in ListVector/LargeListVector.getFieldBuffers() for never-allocated offset buffer #27

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7637194
Fix IOOBE in ListVector/LargeListVector.setReaderAndWriterIndex()
prashanthbdremio Jun 18, 2026
81ac711
Add regression tests for never-allocated ListVector/LargeListVector
prashanthbdremio Jun 18, 2026
3c64cb2
Narrow realloc guard to capacity==0 only
prashanthbdremio Jun 18, 2026
1a009d1
Move fix from setReaderAndWriterIndex() to getFieldBuffers()
prashanthbdremio Jun 18, 2026
360a456
Fix regression tests to match getFieldBuffers() approach
prashanthbdremio Jun 18, 2026
9a861c9
Fix spotless formatting in test files
prashanthbdremio Jun 18, 2026
18b45a6
Fix temp buffer leak: assign to this.offsetBuffer in getFieldBuffers()
prashanthbdremio Jun 18, 2026
0d89f0a
Use (valueCount + 1) * OFFSET_WIDTH instead of offsetAllocationSizeIn…
prashanthbdremio Jun 18, 2026
e0979e8
Allocate default-sized offset buffer to avoid validate-then-write reg…
prashanthbdremio Jun 18, 2026
2cd5472
MINOR: Scope empty list offset buffer fix to empty vectors
prashanthbdremio Jun 18, 2026
00b69a2
MINOR: Reset shared empty validity buffer for empty lists
prashanthbdremio Jun 18, 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
7 changes: 6 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 @@ -277,8 +277,13 @@ public List<ArrowBuf> getFieldBuffers() {
List<ArrowBuf> result = new ArrayList<>(2);
setReaderAndWriterIndex();
result.add(validityBuffer);
if (offsetBuffer.capacity() == 0 && offsetBuffer.writerIndex() > 0) {

@bodduv bodduv Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The only further tightening I could think of is to check offsetBuffer.capacity() < OFFSET_WIDTH, but I think the main issue was indeed with offsetBuffer.capacity() == 0.

@selvaganesang selvaganesang Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If we are dealing with emptyVector issue, wouldn't be better to check if valueCount == 0 here. This seems to be fixing the unrelated issue where offset.capacity == 0 but with writerIndex was set incorrectly somewhere else.

@prashanthbdremio prashanthbdremio Jun 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 2cd5472. The guard is now scoped to valueCount == 0, so this only handles the empty-vector serialization case. For non-empty vectors with a zero-capacity offset buffer we no longer synthesize zero offsets, since that would be an ambiguous/corrupt state rather than this bug.

@bodduv bodduv Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this approach may not fix the issue if writeIndex was set incorrectly.
🤔 were there profiles of failed queries other than capacity = 0, writerIndex = 4?

Edit:
If writerIndex is set incorrectly and capacity is not zero (but say lesser than writerIndex), then I am not sure what else we could do here.

@selvaganesang selvaganesang Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Exactly. Then it is a different issue in Apache Arrow. That I believe is beyond the scope of this fix

@prashanthbdremio prashanthbdremio Jun 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

From the customer profiles, I see this signature..
IndexOutOfBoundsException: readerIndex: 0, writerIndex: 4
Have to check in observe if all the profiles have it.. I checked in 10 profiles which Dmitry had given us

@prashanthbdremio prashanthbdremio Jun 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Looks like this route is not the right way to fix.. there are more and more edge cases that we might hit.

long writerIdx = offsetBuffer.writerIndex();
offsetBuffer.getReferenceManager().release();
offsetBuffer = allocateOffsetBuffer(offsetAllocationSizeInBytes);
Comment thread
prashanthbdremio marked this conversation as resolved.
Outdated
offsetBuffer.writerIndex(writerIdx);
}
result.add(offsetBuffer);

return result;
}

Expand Down
7 changes: 6 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 @@ -235,8 +235,13 @@ public List<ArrowBuf> getFieldBuffers() {
List<ArrowBuf> result = new ArrayList<>(2);
setReaderAndWriterIndex();
result.add(validityBuffer);
if (offsetBuffer.capacity() == 0 && offsetBuffer.writerIndex() > 0) {

@selvaganesang selvaganesang Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same comment as above. If it is not emptyVector issue, we may not know what should be the value in the offsetBuffer. Is it ok to be 0 in all indexes.

@prashanthbdremio prashanthbdremio Jun 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 2cd5472. Same tightening applied here: valueCount == 0 && offsetBuffer.capacity() == 0 && offsetBuffer.writerIndex() > 0. That keeps the fix limited to the empty-vector trailing-offset case, where zero is the correct offset value.

long writerIdx = offsetBuffer.writerIndex();
offsetBuffer.getReferenceManager().release();
offsetBuffer = allocateOffsetBuffer(offsetAllocationSizeInBytes);
offsetBuffer.writerIndex(writerIdx);
}
result.add(offsetBuffer);

return result;
}

Expand Down
21 changes: 21 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 @@ -1120,6 +1120,27 @@ public void testEmptyLargeListOffsetBuffer() {
}
}

@Test
public void testEmptyLargeListOffsetBufferWithoutAllocate() {
// Regression test for the Arrow 19 IOOBE: a never-allocated LargeListVector must produce a
// valid offset buffer from getFieldBuffers() even when allocateNew() was never called.
try (LargeListVector list = LargeListVector.empty("list", allocator)) {
list.addOrGetVector(FieldType.nullable(MinorType.INT.getType()));
list.setValueCount(0);

List<ArrowBuf> buffers = list.getFieldBuffers();
ArrowBuf offsetBuf = buffers.get(1);
assertTrue(
offsetBuf.capacity() >= LargeListVector.OFFSET_WIDTH,
"Returned offset buffer should have capacity >= " + LargeListVector.OFFSET_WIDTH);
assertTrue(
offsetBuf.readableBytes() >= LargeListVector.OFFSET_WIDTH,
"Returned offset buffer should have readableBytes >= " + LargeListVector.OFFSET_WIDTH);
assertEquals(0L, offsetBuf.getLong(0));
// Vector owns the buffer — no manual close needed
}
}

private void writeIntValues(UnionLargeListWriter writer, int[] values) {
writer.startList();
for (int v : values) {
Expand Down
24 changes: 24 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 @@ -1399,6 +1399,30 @@ public void testEmptyListOffsetBuffer() {
}
}

@Test
public void testEmptyListOffsetBufferWithoutAllocate() {
// Regression test for the Arrow 19 IOOBE: a never-allocated ListVector must produce a valid
// offset buffer from getFieldBuffers() even when allocateNew() was never called.
// getFieldBuffers() allocates a real offset buffer when the vector's own offset buffer has
// capacity 0 but writerIndex > 0 (the inconsistent state from Arrow 19).
try (ListVector list = ListVector.empty("list", allocator)) {
list.addOrGetVector(FieldType.nullable(MinorType.INT.getType()));
list.setValueCount(0);

List<ArrowBuf> buffers = list.getFieldBuffers();
ArrowBuf offsetBuf = buffers.get(1);
assertTrue(
offsetBuf.capacity() >= BaseRepeatedValueVector.OFFSET_WIDTH,
"Returned offset buffer should have capacity >= " + BaseRepeatedValueVector.OFFSET_WIDTH);
assertTrue(
offsetBuf.readableBytes() >= BaseRepeatedValueVector.OFFSET_WIDTH,
"Returned offset buffer should have readableBytes >= "
+ BaseRepeatedValueVector.OFFSET_WIDTH);
assertEquals(0, offsetBuf.getInt(0));
// Vector owns the buffer — no manual close needed
}
}

private void writeIntValues(UnionListWriter writer, int[] values) {
writer.startList();
for (int v : values) {
Expand Down
Loading