[common] Fix IndexOutOfBoundsException in ArrowArrayWriter when element count exceeds INITIAL_CAPACITY #2165
+77
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rionmonster
approved these changes
Dec 12, 2025
Contributor
rionmonster
left a comment
rionmonster
left a comment
There was a problem hiding this comment.
LGTM — looks pretty straightforward. Approved! 👍
vamossagar12
approved these changes
Dec 15, 2025
| for (int arrIndex = 0; arrIndex < array.size(); arrIndex++) { | ||
| int fieldIndex = offset + arrIndex; | ||
| elementWriter.write(fieldIndex, array, arrIndex, handleSafe); | ||
| // Always use safe writes for array elements because the element index (offset + |
Contributor
There was a problem hiding this comment.
We should mention in the comments on the class that the handleSafe field is ignored when writing.
Contributor
|
+1，just a minor suggestion |
…nt count exceeds INITIAL_CAPACITY (apache#2165) Signed-off-by: binary-signal <binary-signal@github.noreply.com>
…andleSafe=true with dynamic check based on fieldIndex
wuchong
force-pushed
the
fix-arrow-writer-index-out-of-bounds
branch
from
e73e61f to
dfd1166
Compare
wuchong
approved these changes
Dec 31, 2025
Member
wuchong
left a comment
wuchong
left a comment
There was a problem hiding this comment.
I appended a commit from @platinumhamburg #2287 to improve the fix.
wuchong
changed the title
platinumhamburg
approved these changes
Dec 31, 2025
| // arrIndex) can exceed INITIAL_CAPACITY even when the row count doesn't. The parent's | ||
| // handleSafe is based on row count, but array element indices grow based on the total | ||
| // number of elements across all arrays, which can be much larger. | ||
| elementWriter.write(fieldIndex, array, arrIndex, true); |
Contributor
There was a problem hiding this comment.
Suggested change
| elementWriter.write(fieldIndex, array, arrIndex, true); | |
| boolean elementHandleSafe = fieldIndex >= ArrowWriter.INITIAL_CAPACITY; | |
| elementWriter.write(fieldIndex, array, arrIndex, elementHandleSafe); |
Contributor
There was a problem hiding this comment.
Perhaps we could apply a clear conditional judgment here.
wuchong
pushed a commit
that referenced
this pull request
Dec 31, 2025
…nt count exceeds INITIAL_CAPACITY (#2165) Signed-off-by: binary-signal <binary-signal@github.noreply.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Linked issue: close #2164
Fix
IndexOutOfBoundsExceptionwhen writing rows with array columns where the total number of array elements exceedsINITIAL_CAPACITY(1024) while the row count stays below it.Brief change log
In
ArrowWriter.writeRow(), thehandleSafeflag is determined by comparing row count againstINITIAL_CAPACITY:When
handleSafe = false, Arrow writers usevector.set()which doesn't auto-grow the buffer. The bug is inArrowArrayWriter.doWrite()which passes the parent'shandleSafeflag to the element writer. However, array element indices grow based on cumulative element count, not row count.Example: 250 rows with 10-element arrays → row count (250) < 1024 so
handleSafe = false, but total elements (2500) exceeds the vector's initial capacity, causingIndexOutOfBoundsException.Fix:
Always use safe writes (
handleSafe = true) for array element writers inArrowArrayWriter.doWrite(), since element indices can exceedINITIAL_CAPACITYindependently of row count.Tests
ArrowReaderWriterTest#testArrayWriterWithManyElements: writes 200 rows with 10-element arrays (2000 total elements), verifying serialization succeeds and data can be read back correctly.API and Format
No API or storage format changes.
Documentation
No documentation changes needed. This is a bug fix.