Abdallah-Afifi opened a new pull request, #11539: URL: https://github.com/apache/arrow-rs/pull/11539
# Which issue does this PR close? - Closes #10645. # Rationale for this change `parquet` only ever wrote ALP vectors at the spec's recommended 1024 values, even though the format permits a `log_vector_size` of 3 to 15. The decoder already handles any of them, since it reads the size from the page header — only the writer was fixed. # What changes are included in this PR? `AlpEncoderOptions::try_new(log_vector_size)` validates the 3..=15 range, with `set_alp_encoder_options` and `set_column_alp_encoder_options` on `WriterPropertiesBuilder`. This follows `DeltaBinaryPackedEncoderOptions` from #11051, including keeping `AlpEncoder::new()` as-is and adding `new_with_options` beside it. The vector size becomes a runtime field on `AlpEncoder` and `StreamingPage`, which turns `StreamingPage::put`'s `values.as_chunks::<VECTOR_SIZE>()` into `chunks_exact`. # Are these changes tested? Yes: - round-trip at every permitted `log_vector_size`, 3 through 15 - a non-default size through the streaming path, byte-compared against single-pass encoding - the configured size reaching the page header - the full `WriterProperties` -> `get_encoder_with_properties` -> page header chain - per-column precedence over the global default, and rejection of out-of-range sizes `cargo test --workspace` passes (7257 tests), along with `cargo fmt --all --check`, `cargo clippy --workspace --all-targets --all-features -- -D warnings`, and `typos`. # Performance Numbers are in https://github.com/apache/arrow-rs/issues/10645#issuecomment-6066040118. In short, `encode/next_page` regresses about 5-7% on `f32` and is flat or better on `f64`, measured against a same-session `main` baseline with a `main` vs `main` control to establish a ~1.5% drift floor. I also tried keeping a const-generic `as_chunks::<1024>()` fast path for the default size and it was no better than plain `chunks_exact`, so it isn't in this PR. That also rules out the chunking loop's const-ness as the cause, and I don't have a confirmed explanation for the `f32` cost or for the `f64` improvement. Both reproduced across runs. An M5 laptop is not CI, so a second opinion on the numbers would be welcome — and if 5-7% on `f32` is too much, the alternative is dispatching to const-generic instantiations for all 13 sizes, which I'm happy to look at instead. One note on ordering: #11481 makes `AlpEncoder::new()` public. This PR leaves that signature alone and adds `new_with_options`, so the two shouldn't conflict in either merge order. --- Per the AI policy in CONTRIBUTING.md: I used Claude Code for the implementation, the tests and the benchmarking. I've reviewed all of it and ran the measurements myself. -- 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]
