ryanvickr opened a new pull request, #55488: URL: https://github.com/apache/arrow/pull/55488
### Rationale for this change Fixes #55487. `DeltaLengthByteArrayEncoder` currently: 1. Overwrites caller-provided `MemoryPool*` arguments in its constructor (`pool = ::arrow::default_memory_pool()` instead of a default parameter), causing both `DELTA_LENGTH_BYTE_ARRAY` and `DELTA_BYTE_ARRAY`'s `suffix_encoder_` to allocate on `default_memory_pool()` instead of the caller's pool. 2. Encodes Arrow binary/string lengths one element at a time in `PutBinaryArray` without pre-reserving `sink_` capacity, adding unnecessary per-value overhead and reallocations compared to `Put(const ByteArray*, int)` and other Parquet writers (such as `parquet-rs` and Polars). 3. Calls `BufferBuilder::UnsafeAppend(nullptr, 0)` on empty `ByteArray(0, nullptr)` values, triggering undefined behavior (`memcpy` with null source pointer) under UBSan. ### What changes are included in this PR? - Fix `DeltaLengthByteArrayEncoder`'s constructor signature to use `MemoryPool* pool = ::arrow::default_memory_pool()` as a default argument rather than reassigning `pool` in the member initializer list. - Batch length encoding in 256-value chunks in `PutBinaryArray`, pre-reserve `sink_` capacity, bulk-copy contiguous non-null offset-based `BinaryArray`/`LargeBinaryArray` payloads in a single `UnsafeAppend`, and skip zero-byte appends. - Guard `Put(const ByteArray*, int)` against calling `UnsafeAppend` on zero-length items. ### Are these changes tested? Yes, added unit tests in `cpp/src/parquet/encoding_test.cc`: - `DeltaLengthByteArrayEncodingAdHoc.UsesProvidedMemoryPool` verifying allocations across `StringArray`, nullable `StringArray`, sliced `LargeStringArray`, `StringViewArray`, raw `ByteArray`, and `DELTA_BYTE_ARRAY` are tracked on a caller-supplied `ProxyMemoryPool`. - `DeltaLengthByteArrayEncodingAdHoc.BatchedLengthsSlicesAndZeroLengthSafety` covering 256-element batch boundaries (`0, 1, 255, 256, 257, 512, 600`), non-zero slice offsets on `LargeStringArray`, `StringViewArray`, empty/all-null arrays, and `ByteArray(0, nullptr)` inputs. ### Are there any user-facing changes? Callers passing a custom `MemoryPool*` when writing `DELTA_LENGTH_BYTE_ARRAY` or `DELTA_BYTE_ARRAY` columns will now have encoder buffer allocations tracked on that pool. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. AI coding assistance was used to help draft the `PutBinaryArray` batching implementation, unit tests, and PR description; all changes were reviewed and verified by the author. **PR code and description written by:** - [x] Human - [x] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed -- 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]
