vbhanuchander-lang commented on PR #17645: URL: https://github.com/apache/iceberg/pull/17645#issuecomment-5288306162
Pushed `3873922`, folding in the fixed-width sites I had flagged above as out of scope — on reflection they are the same units mistake in the same method, and splitting them would have meant two PRs touching adjacent lines. `FIXED_LEN_BYTE_ARRAY` and `INT96` passed `batchSize * typeWidth` to the single-argument overload. Fixed-width vectors size their data buffer as `valueCount * typeWidth`, so a byte count reserves `typeWidth` times too many values. Measured for a UUID column (byteWidth 16) at the default batch size of 5000: | | valueCapacity | data buffer | | --- | --- | --- | | `setInitialCapacity(5000 * 16)` — current | 130 055 | 2 080 880 bytes | | `setInitialCapacity(5000)` — by value count | 8 128 | 130 048 bytes | Shrinking these is safe for a different reason than the variable-width ones, and it is worth being explicit since fixed-width writes use the unchecked `set(...)`, not a `*Safe` setter: `batchSize` is already the per-batch upper bound the reader depends on. It sizes `NullabilityHolder` with `batchSize` in the same method, and reuses one vector across batches, so a batch can never present more than `batchSize` values. `setInitialCapacity(5000)` yields a capacity of 8128, comfortably above that. `iceberg-arrow`: **37 tests, 0 failures**; `checkstyleMain`, `checkstyleTest`, `spotlessCheck` clean. -- 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]
