adriangb opened a new pull request, #10554: URL: https://github.com/apache/arrow-rs/pull/10554
- closes https://github.com/apache/arrow-rs/issues/10538 > [!IMPORTANT] > **Stacked on #10505 — do not merge first.** The base commits below belong to that PR. > Review only this PR's own commit: [`fix(parquet): cut byte-budget mini-batches on exact value counts`](https://github.com/pydantic/arrow-rs/compare/fix-delta-byte-array-page-split-10489...claude/parquet-exact-value-windows-10538). > Once #10505 merges I'll rebase and this PR's diff becomes clean on its own. ## The problem `byte_budget_sub_batch_size` asks the encoder how many *values* fit in a page byte budget, then converts that to a *level* count using the chunk-wide level:value ratio, rounded up: ```rust (values_per_subbatch * chunk_size).div_ceil(vals_in_chunk).max(1) ``` For a chunk with no nulls this is exact. With one null in 17 levels it gives `ceil(17/16) == 2`, and `write_granular_chunk` then slices the chunk into uniform two-level windows — most of which carry **two** values, i.e. twice what the budget allowed. The mechanism predates #10505; it dates to #9972. #10505 raises its cost but does not introduce it. ## The fix The budget is inherently a value count — it comes from summing value byte sizes. So return it as one, and have `write_granular_chunk` end each window by walking definition levels until it has covered that many values. No ratio, no rounding. The walk runs only in the granular path, whose values are by definition larger than the page limit, so touching each of the chunk's levels once is noise next to writing those values — and that path already makes full def-level (`value_count`) and rep-level (record alignment) passes. Non-nullable and fixed-width columns are untouched: `Absent`/`Uniform` def levels take an O(1) arm. ## Results Both effects apply only to values exceeding `data_page_size_limit`. **The #9972 page bound now holds on nullable columns.** One value per page instead of two. This is independent of #10505 — pinned with `PLAIN`, where the first-value exemption never fires: | 16× 64 KiB values, one null, 16 KiB limit | max page | pages | |---|---|---| | before | ~128 KiB | 8 | | this PR | ~64 KiB | 16 | **DELTA_BYTE_ARRAY dedup becomes complete on nullable columns.** #10505's exemption fires when a page opens with a single-value mini-batch — now guaranteed regardless of where nulls fall: | same column, `DELTA_BYTE_ARRAY` | levels per page | values stored in full | |---|---|---| | `main` | 16× 1 | 16 (identical to `PLAIN`) | | #10505 | `[2, 2, 2, 2, 9]` | ~5 | | this PR | `[17]` | ~1 | That is the acceptance criterion in #10538, and it re-pins `test_column_writer_delta_byte_array_nullable_shared_prefix_partial_dedup` — the test #10505 added as a marker for exactly this change, per its comment. ## Scope Repeated columns are unchanged. Records cannot span pages, so a single record holding several over-limit values still exceeds the budget; that is inherent to the format, not to this heuristic. ## Tests - `test_column_writer_caps_page_size_with_sparse_nulls` — new. `PLAIN`, sparse nulls, asserts one value's payload per page. Guards this fix on its own, so it still holds if #10505 is ever reworked. - `test_column_writer_delta_byte_array_nullable_shared_prefix_dedup` — re-pinned from `[2, 2, 2, 2, 9]` to `[17]` and renamed (no longer partial), with the old layout recorded in the comment. Both were verified to fail against the previous ratio-scaled windowing, the DELTA one reproducing `[2, 2, 2, 2, 9]` exactly. Full `parquet` suite green (1311 lib + integration), `fmt` and `clippy -D warnings` clean. ## Note on #10505 Its `bool_to_int_with_if` in the test above trips `cargo clippy -- -D warnings`, which CI runs — worth fixing on that branch too. It's corrected here as part of editing that test. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
