Skip to content

GH-343: Fix ListVector offset buffer not properly serialized for nested empty arrays - #967

Merged
jbonofre merged 6 commits into
apache:mainfrom
Yicong-Huang:fix/343-empty-nested-list-offset-buffer
Jan 23, 2026
Merged

GH-343: Fix ListVector offset buffer not properly serialized for nested empty arrays#967
jbonofre merged 6 commits into
apache:mainfrom
Yicong-Huang:fix/343-empty-nested-list-offset-buffer

Conversation

@Yicong-Huang

@Yicong-HuangYicong-Huang commented Jan 17, 2026

Copy link
Copy Markdown
Contributor

What's Changed

Fix ListVector/LargeListVector IPC serialization when valueCount is 0.

Problem

When valueCount == 0, setReaderAndWriterIndex() was setting offsetBuffer.writerIndex(0), which means readableBytes() == 0. IPC serializer uses readableBytes() to determine buffer size, so 0 bytes were written to the IPC stream. This crashes IPC readers in other libraries because Arrow spec requires offset buffer to have at least one entry [0].

@viirya:

The offset buffers are allocated properly. But during IPC serialization, they are ignored.

 public long readableBytes() {
return writerIndex - readerIndex;
}

So when ListVector.setReaderAndWriterIndex() sets writerIndex(0) and readerIndex(0), readableBytes() returns 0 - 0 = 0.

Then when MessageSerializer.writeBatchBuffers() calls WriteChannel.write(buffer), it writes 0 bytes.

So the flow is:

valueCount=0 → ListVector.setReaderAndWriterIndex() sets offsetBuffer.writerIndex(0)
VectorUnloader.getFieldBuffers() returns the buffer with writerIndex=0
MessageSerializer.writeBatchBuffers() writes the buffer
WriteChannel.write(buffer) checks buffer.readableBytes() which is 0
0 bytes are written to the IPC stream
PyArrow read the batch with the missing buffer → crash when other libraries to read

Fix

Simplify setReaderAndWriterIndex() to always use (valueCount + 1) * OFFSET_WIDTH for offset buffer's writerIndex. When valueCount == 0, this correctly sets writerIndex to OFFSET_WIDTH, ensuring offset[0] is included in serialization.

Testing

Added tests for nested empty lists verifying offset buffer has correct readableBytes().

Closes#343.

@Yicong-HuangYicong-Huang changed the title GH-343 Fix ListVector offset buffer not allocated for nested empty arraysGH-343: Fix ListVector offset buffer not allocated for nested empty arraysJan 17, 2026
@github-actions

This comment has been minimized.

@lidavidmlidavidm added the bug-fix PRs that fix a big. label Jan 18, 2026
@github-actionsgithub-actionsBot added this to the 19.0.0 milestone Jan 18, 2026
@jbonofre

Copy link
Copy Markdown
Member

@Yicong-Huang can you please rebase the PR ? Thanks !

@Yicong-Huang
Yicong-Huangforce-pushed the fix/343-empty-nested-list-offset-buffer branch from 8b09237 to 7dbdcc4CompareJanuary 20, 2026 18:21
Comment threadvector/src/main/java/org/apache/arrow/vector/complex/LargeListVector.java Outdated
Comment threadvector/src/main/java/org/apache/arrow/vector/complex/LargeListVector.java Outdated
Comment threadvector/src/main/java/org/apache/arrow/vector/complex/LargeListVector.java Outdated
@viirya

Copy link
Copy Markdown
Member

When outer array is empty, nested writers are never invoked, so child list's offset buffer remains unallocated (capacity = 0). This violates Arrow spec which requires offset[0] = 0.

I think you are referring the writers in Spark. It is out of context here and not related to the root cause. We should update the description to explain the issue clearly.

The offset buffers are actually allocated properly. But during IPC serialization, they are ignored.

publiclongreadableBytes() {
returnwriterIndex - readerIndex;
}

So when ListVector.setReaderAndWriterIndex() sets writerIndex(0) and readerIndex(0), readableBytes() returns 0 - 0 = 0.

Then when MessageSerializer.writeBatchBuffers() calls WriteChannel.write(buffer), it writes 0 bytes.

So the flow is:

  1. valueCount=0 → ListVector.setReaderAndWriterIndex() sets offsetBuffer.writerIndex(0)
  2. VectorUnloader.getFieldBuffers() returns the buffer with writerIndex=0
  3. MessageSerializer.writeBatchBuffers() writes the buffer
  4. WriteChannel.write(buffer) checks buffer.readableBytes() which is 0
  5. 0 bytes are written to the IPC stream
  6. PyArrow read the batch with the missing buffer → crash when other libraries to read

@viirya

Copy link
Copy Markdown
Member

Hi @lidavidm@jbonofre, do you think this can catch up the Arrow Java 19.0.0 release?

@Yicong-HuangYicong-Huang changed the title GH-343: Fix ListVector offset buffer not allocated for nested empty arraysGH-343: Fix ListVector offset buffer not properly serialized for nested empty arraysJan 21, 2026
validityBuffer.writerIndex(BitVectorHelper.getValidityBufferSizeFromCount(valueCount));
offsetBuffer.writerIndex((valueCount + 1) * OFFSET_WIDTH);
}
validityBuffer.writerIndex(BitVectorHelper.getValidityBufferSizeFromCount(valueCount));

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 valueCount == 0, I think validity buffer writer index should be validityBuffer.writerIndex(0);.

@Yicong-HuangYicong-HuangJan 21, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I believe when valueCount==0, BitVectorHelper.getValidityBufferSizeFromCount(valueCount) also returns 0, so it is equivalent. The current version might be simpler. If you prefer an if branch to handle it separately, I can also apply it.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I added the if branch back to handle valueCount==0 case. But I still think it is not necessary?

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.

Oh, if BitVectorHelper.getValidityBufferSizeFromCount(valueCount) returns 0 for valueCount == 0, then it is okay.

Comment on lines +1114 to +1116
// Allocate outer only - simulates case where inner is never written to
outerList.allocateNew();
outerList.setValueCount(0);

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.

I think innerList should also call allocateNew? Not allocated is different to not written.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

added

Comment on lines +1395 to +1397
// Only allocate level0 - simulates case where all nested levels are empty
level0.allocateNew();
level0.setValueCount(0);

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.

ditto

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

added!

Comment on lines +1108 to +1112
try (LargeListVector outerList = LargeListVector.empty("outer", allocator)) {
// Setup LargeList<LargeList<Int>>
outerList.addOrGetVector(FieldType.nullable(MinorType.LARGELIST.getType()));
LargeListVector innerList = (LargeListVector) outerList.getDataVector();
innerList.addOrGetVector(FieldType.nullable(MinorType.INT.getType()));

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.

I looked these tests again. I think this nested structure is not necessary for the unit tests here. It only matter for our usage at Spark side (on the ArrowWriters). But for here, we just need to make sure that a ListVector/LargeListVector has meaningful and correct readableBytes value after they are allocated.

Maybe we can simplify these tests.

@viiryaviirya left a comment

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.

I only have some comments on the tests to simplify them. Otherwise, the fix looks correct to me.

@jbonofre@lidavidm Can you take a look and see if we can get this into the next release?

Thank you!

@jbonofre

Copy link
Copy Markdown
Member

I'm doing a new review pass.

@jbonofrejbonofre left a comment

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.

LGTM, thanks !

@jbonofre
jbonofre merged commit 0f8a080 into apache:mainJan 23, 2026
26 checks passed
@viirya

Copy link
Copy Markdown
Member

Thanks you @jbonofre!

Can we backport this fix to versions like 18.3.0 and release 18.3.1?

@jbonofre

Copy link
Copy Markdown
Member

@viirya my plan is more to release 19.0.0. Do you really need 18.3.1 ?

@viirya

Copy link
Copy Markdown
Member

@viirya my plan is more to release 19.0.0. Do you really need 18.3.1 ?

Because we are using 18.3.0 currently, it will be much safer to upgrade with 18.3.1. Is it possible to backport it to 18.3 and release 18.3.1?

@jbonofre

Copy link
Copy Markdown
Member

@viirya ok, I understand. Let me complete 19.0.0 first, I will prepare 18.3.1 after.

@Yicong-Huang

Copy link
Copy Markdown
ContributorAuthor

@viirya ok, I understand. Let me complete 19.0.0 first, I will prepare 18.3.1 after.

Thanks a lot @viirya@jbonofre!

@viirya

Copy link
Copy Markdown
Member

@viirya ok, I understand. Let me complete 19.0.0 first, I will prepare 18.3.1 after.

Thank you so much! @jbonofre

@viiryaviirya left a comment

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.

@Yicong-Huang Should we also fix setReaderAndWriterIndex at BaseVariableWidthVector?

@Yicong-Huang

Copy link
Copy Markdown
ContributorAuthor

@Yicong-Huang Should we also fix setReaderAndWriterIndex at BaseVariableWidthVector?

sure will also fix the base!

@Yicong-Huang

Copy link
Copy Markdown
ContributorAuthor

Hi @jbonofre, just wanted to check if there’s any rough ETA for the 19.0.0 or 18.3.1 release. This would help us plan our dependency upgrade. Really appreciate all the work on the releases. Thanks!

cc @viirya

@Yicong-Huang

Copy link
Copy Markdown
ContributorAuthor

Hi @jbonofre, just wanted to check if there’s any rough ETA for the 19.0.0 or 18.3.1 release. This would help us plan our dependency upgrade. Really appreciate all the work on the releases. Thanks!

cc @viirya

Just gently following up on this when you have a chance. Thanks again.

jbonofre pushed a commit that referenced this pull request Mar 12, 2026
…offset buffer serialization (#989)
## What's Changed
Fix `BaseVariableWidthVector`/`BaseLargeVariableWidthVector` IPC
serialization when `valueCount` is 0.
### Problem
When `valueCount == 0`, `setReaderAndWriterIndex()` was setting
`offsetBuffer.writerIndex(0)`, which means `readableBytes() == 0`. IPC
serializer uses `readableBytes()` to determine buffer size, so 0 bytes
were written to the IPC stream. This crashes IPC readers in other
libraries because Arrow spec requires offset buffer to have at least one
entry `[0]`.
This is a follow-up to #967 which fixed the same issue in
`ListVector`/`LargeListVector`.
### Fix
Modify `setReaderAndWriterIndex()` to always use `(valueCount + 1) *
OFFSET_WIDTH` for the offset buffer's `writerIndex`, moved outside the
if/else branch. When the offset buffer capacity is insufficient (e.g.,
empty buffer from constructor or loaded via `loadFieldBuffers()`), it
reallocates a properly sized buffer on demand.
### Testing
Added tests for empty `VarCharVector` and `LargeVarCharVector` verifying
offset buffer has correct `readableBytes()` after `setValueCount(0)`.
Closes#343
---------
Co-authored-by: Yicong Huang <yicong.huang+data@databricks.com>
@viirya

Copy link
Copy Markdown
Member

@viirya ok, I understand. Let me complete 19.0.0 first, I will prepare 18.3.1 after.

Thank you so much! @jbonofre

Hello @jbonofre, thank you for reviewing and preparing 19.0.0 release. Now as 19.0.0 was released, can you help us prepare 18.3.1 release including these fixes if you have time? Thank you!

@dmitry-chirkov-dremio

dmitry-chirkov-dremio commented Jun 18, 2026

Copy link
Copy Markdown

We are troubleshooting a regression that ends up in following stack:

(java.lang.IndexOutOfBoundsException) readerIndex: 0, writerIndex: 4 (expected: 0 <= readerIndex <= writerIndex <= capacity(0))
io.netty.buffer.AbstractByteBuf.checkIndexBounds():112
io.netty.buffer.AbstractByteBuf.writerIndex():135
io.netty.buffer.NettyArrowBuf.writerIndex():215
io.netty.buffer.NettyArrowBuf.unwrapBuffer():631

Our starting point is this.

Is it possible that merged changes break Netty's assumptions in unwrapBuffer?

edit: looks like change in #989 was more involved (and more correct?)

lriggs added a commit to lriggs/arrow-java that referenced this pull request Jun 18, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fixPRs that fix a big.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++/Java] Error when reading inner lists within a struct in empty outer lists from C++/Python in Java

5 participants

@Yicong-Huang@jbonofre@viirya@dmitry-chirkov-dremio@lidavidm