vbhanuchander-lang opened a new pull request, #17645: URL: https://github.com/apache/iceberg/pull/17645
Closes #17305. ### The problem `VectorizedArrowReader` sizes variable-width vectors with ```java vec.setInitialCapacity(batchSize * AVERAGE_VARIABLE_WIDTH_RECORD_SIZE); ``` but `setInitialCapacity(int)` takes a **value count**, not a byte count, and sizes the data buffer at Arrow's default of 8 bytes per value. With the default batch size of 5000 and an average width of 10 that reserves offsets for 50 000 values and a 400 KB data buffer, where the intent was 5000 values at ~10 bytes each. `BaseVariableWidthVector.setInitialCapacity(int, double)` says exactly that, so the fix is to use it. ### Why shrinking the initial allocation is safe The read path is already written for buffers that grow. `VarWidthReader#nextVal` uses `setValueLengthSafe`, whose in-code comment reads *"Calling setValueLengthSafe takes care of allocating a larger buffer if running out of space"*, and it deliberately re-reads `vector.getDataBuffer()` afterwards because *"it is possible that the data buffer was reallocated"*. Nothing in `arrow/src/main` reads `getValueCapacity()` or the data buffer capacity to make decisions — I grepped for both. So this changes how much is reserved up front, not what is read or written. ### Scope Only the two variable-width sites are changed — the `BINARY` case and `allocateVectorForEnumJsonBsonString`, which is what the issue describes. The `FIXED_LEN_BYTE_ARRAY` and `INT96` cases pass a byte count (`batchSize * len`) to the same single-argument overload on *fixed-width* vectors, which looks like the same mistake and would over-reserve by a factor of `len`. I have left them alone since they are outside this issue and fixed-width vectors have no density overload, so the correct call there is simply `batchSize`. Happy to fold that in if you would like it in the same change. I also did not implement the `ColumnChunkMetaData`-based width estimation the issue floats as a follow-on. That needs a decision about bounding and fallback behaviour, and it is separable from correcting the units, which is what this PR does. ### Testing `TestVariableWidthInitialCapacity` pins the Arrow contract the fix depends on: the density overload reserves one offset slot per row, while passing a byte count to the single-argument overload reserves offsets for `batchSize * averageWidth` rows. The second case is kept deliberately as a contrast so the reason for the density overload is not lost again. Full `iceberg-arrow` suite: **36 tests, 0 failures**. `checkstyleMain`, `checkstyleTest` and `spotlessCheck` are clean. 🤖 AI-assisted — generated with Claude Code (Opus 5) and reviewed by me before submitting. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
