fix(parquet): keep DELTA_BYTE_ARRAY dedup for values larger than the page size limit - #10505
Open
adriangb wants to merge 2 commits into
Open
fix(parquet): keep DELTA_BYTE_ARRAY dedup for values larger than the page size limit#10505adriangb wants to merge 2 commits into
adriangb wants to merge 2 commits into
Conversation
adriangb
marked this pull request as ready for review
August 1, 2026 16:49
Contributor
Author
|
run benchmark arrow_writer |
Contributor
Author
|
run benchmark writer_overhead |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (d053bf0) to cd17899 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (d053bf0) to cd17899 (merge-base) diff Run configurationrun benchmark writer_overheadBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (d053bf0) to cd17899 (merge-base) diff Run configurationrun benchmark writer_overheadCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (d053bf0) to cd17899 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
adriangb
force-pushed
the
fix-delta-byte-array-page-split-10489
branch
from
August 3, 2026 13:58
d63a6d7 to
5b904cc
Compare
alamb
pushed a commit
that referenced
this pull request
Aug 3, 2026
…er benchmark (#10512) # Which issue does this PR close? - Part of #10511 # Rationale for this change No existing benchmark writes `DELTA_BYTE_ARRAY` through the writer: the `arrow_writer` property matrix varies writer version, compression, bloom filters, and CDC, all on the default encoding, and `encoding.rs` benches encoders in isolation. That gap matters for the large-value write path specifically: the byte-budget sub-batching in `write_batch_internal` (#9972) measures raw payload bytes, and page-boundary behavior interacts with the encoding's cross-value state (#10489, #10505). None of that is visible to any current benchmark. #10511 tracks whether making the byte budget encoded-size-aware is worthwhile; these benchmarks are the measurement for that question. # What changes are included in this PR? Two new batches and one new bench group in `parquet/benches/arrow_writer.rs`: - `large_string_shared_prefix`: 128 rows x 2 MiB, long common prefix with a short distinct suffix (the case `DELTA_BYTE_ARRAY` exists for) - `large_string_distinct`: same shape, values differing from byte 0 (the adversarial case, prefix length ~0) Values are sized so one value alone exceeds the default 1 MiB page limit, the regime of #10489. Each batch runs under `plain` and `delta_byte_array` properties (dictionary disabled), so the delta-vs-plain gap on identical data separates inherent encoding cost from writer overhead. Results on an Apple M-series laptop, current `main`: | group | `plain` | `delta_byte_array` | | --- | --- | --- | | `large_string_shared_prefix` | 59.0 ms (4.2 GiB/s) | 61.4 ms (4.1 GiB/s) | | `large_string_distinct` | 37.2 ms (6.7 GiB/s) | 44.7 ms (5.6 GiB/s) | Note these numbers shift when #10505 lands, in both directions: shared-prefix delta gets slower in CPU time because the encoder starts doing real prefix comparisons instead of degenerating to per-page `PLAIN` (while the output shrinks ~128x), and distinct delta gets faster from halving the page count. That sensitivity is the point of having the benchmark. # Are these changes tested? The change is itself a benchmark; it compiles and runs under `cargo bench -p parquet --bench arrow_writer`. # Are there any user-facing changes? No. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
adriangb
force-pushed
the
fix-delta-byte-array-page-split-10489
branch
from
August 3, 2026 16:03
5b904cc to
c284081
Compare
Contributor
Author
|
run benchmark arrow_writer writer_overhead |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark writer_overheadBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark writer_overheadCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Contributor
Author
|
run benchmark arrow_writer writer_overhead |
1 similar comment
Contributor
Author
|
run benchmark arrow_writer writer_overhead |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark writer_overheadBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark writer_overheadBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark writer_overheadCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark writer_overheadCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Contributor
Author
|
run benchmark arrow_writer |
2 similar comments
Contributor
Author
|
run benchmark arrow_writer |
Contributor
Author
|
run benchmark arrow_writer |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (6190b5a) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (6190b5a) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (6190b5a) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (6190b5a) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (6190b5a) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (6190b5a) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Contributor
Author
Local benchmark results (Apple M4 Pro, aarch64)Since the CI runner has been noisy on this PR, I ran the same benchmarks locally as base ( Real changes — delta far outside that benchmark's own noise floor:
The
Everything else is flat. Of the remaining 36 benchmarks, 33 land within ±2%, and the three that don't ( The regressions CI reported do not reproduce. Specifically:
Worth noting that Profile: where the time actually wentOn After the change that loop is gone from the profile, and the benchmark is 2.5× faster than it was before either commit. Why 32-byte blocksIt is the widest block that both aarch64 and x86-64 still expand inline: at 64 bytes x86-64 (baseline SSE2) drops to an out-of-line |
Contributor
Author
alamb
reviewed
Aug 4, 2026
alamb
left a comment
alamb
left a comment
Contributor
There was a problem hiding this comment.
Thanks @adriangb
I re-read the ticket and I am not sure that this is actually a bug -- it seems like it may be just better behavior (aka the writer is now enforcing page limits more strictly):
#10489 (comment)
I am worried about the complexity potentially introduced by this PR: not just the code, but I am not sure if it will really help @jonasdedden 's problem (maybe now the pages will have 2 rows rather than 1 ?) 🤔
If it is important to restore the old behavior, I think it would be simpler if we just added some writer option that said "only check page sizes at batch boundaries" -- that would be simpler to implement and understand
| /// `should_add_data_page` then cuts a page after every single value. For | ||
| /// `DELTA_BYTE_ARRAY` that is destructive rather than merely wasteful: | ||
| /// each page boundary discards the previous value, so a column of large | ||
| /// values sharing long prefixes degenerates to `PLAIN` (#10489). |
Contributor
There was a problem hiding this comment.
suggest adding a link to the issue rather than the #10489 style
| /// each page boundary discards the previous value, so a column of large | ||
| /// values sharing long prefixes degenerates to `PLAIN` (#10489). | ||
| /// | ||
| /// Recording the value's encoded size here makes the limit apply to what |
Contributor
There was a problem hiding this comment.
I am not sure what this paragraph is trying to say - it seems like maybe it is describing an implementation detail (so we could remove it?)
| /// Exempt a page's mandatory first value from the data page byte limit, | ||
| /// when that value alone already exceeds it. | ||
| /// | ||
| /// Parquet requires every data page to hold at least one value, so such a |
| /// value cannot be split out no matter how the limit is set. Counting it | ||
| /// against the limit makes the limit unsatisfiable, and | ||
| /// `should_add_data_page` then cuts a page after every single value. For | ||
| /// `DELTA_BYTE_ARRAY` that is destructive rather than merely wasteful: |
Contributor
There was a problem hiding this comment.
I am not sure about "destructive" here -- the point is that it is very wasteful to encode values on a single page, right?
| /// that compress against the preceding value opt in, so `PLAIN` and | ||
| /// `DELTA_LENGTH_BYTE_ARRAY` keep their tighter one-value page bound. | ||
| /// | ||
| /// Known limitation: the caller's trigger keys on a page-opening |
Contributor
There was a problem hiding this comment.
this also seems complicated -- is there any way we can fix the limitation rather than try and caveat it (in comments that are not likely to get read)?
Contributor
Author
There was a problem hiding this comment.
I think we can do it in #10554. Happy to incorporate into one larger PR or do it in stages. I think we can do it in stages since this PR doesn't make things any worse than the status quo.
etseidl
added a commit
that referenced
this pull request
Aug 4, 2026
#10549) > **Stacked on #10550 This branch is > that PR's benchmark commit plus one commit of its own, so that the numbers > below are reproducible with `cargo bench` on this branch alone. Review only > the second commit here; the diff shrinks to > `+139/-15` once #10550 merges and this rebases onto `main`. # Which issue does this PR close? None directly. Split out of #10505 so that the correctness fix there can be reviewed without an unrelated performance change attached to it. # Rationale for this change `DELTA_BYTE_ARRAY` stores each value as the number of leading bytes it shares with its predecessor plus the remaining suffix, so writing a value runs a shared-prefix scan against the previous value. Both encoder paths implement that scan as a byte-at-a-time loop: - `DeltaByteArrayEncoder::put` in `parquet/src/encodings/encoding/mod.rs` (the generic `SerializedFileWriter` path) - `FallbackEncoder::encode`'s `Delta` arm in `parquet/src/arrow/arrow_writer/byte_array.rs` (the `ArrowWriter` path) The scan runs once per value, and on exactly the data the encoding exists for — near-identical consecutive values — it covers essentially the whole value. That makes its throughput, not its per-call overhead, the thing that matters, and a byte-at-a-time loop is the slowest way to do it. # What changes are included in this PR? Extract the two duplicated loops into `crate::util::prefix::common_prefix_length` and compare a 32-byte block at a time instead of a byte at a time. 32 is the widest block that both aarch64 and x86-64 still expand inline; at 64 bytes x86-64 drops to an out-of-line `bcmp` call, which costs more than the extra width buys. Measured on aarch64, every width from 16 up performs the same, so this sits in the middle of a flat optimum rather than on a tuned peak. No behavior change: the function returns the same prefix length the byte-wise loops did, and no page layout, encoding, or file output changes. # Are these changes tested? Existing coverage: the full `parquet` suite passes unmodified (1307 tests), including the `DELTA_BYTE_ARRAY` round-trip tests. Those round trips are weaker evidence than they look, in two ways. First, they never reach the new code path: `ByteArrayType::test` and `FixedLenByteArrayType::test` feed random values, which share no prefix, and the `ArrowWriter` cases write values a handful of bytes long — so nothing in the suite writes two consecutive values sharing 32 bytes, and the block loop never runs. Second, a round trip is structurally blind to an under-counted prefix: the encoder just emits a correspondingly longer suffix and the decoder reconstructs the same bytes either way. Only over-counting shows up. `test_estimated_data_encoded_size` does assert an exact encoded size, but on 2- and 3-byte values that can never enter the block loop. New coverage, unit level, in `parquet/src/util/prefix.rs`: the boundary cases the block loop introduces — empty inputs, prefixes shorter than / equal to / longer than one block, a mismatch in the first and last byte of a block, and unequal lengths where one input is a strict prefix of the other. Plus unequal lengths combined with a mismatch past a block boundary (where truncating to the shorter length interacts with the block loop), non-zero slice start offsets (callers pass sub-slices into shared Arrow buffers, not freshly allocated `Vec`s), and a prefix ending mid-UTF-8-codepoint — byte-level prefixes may split a multi-byte character, which was true of the byte-wise scan too and is worth pinning now that the scan is wider. New coverage, end to end, for both call sites: `test_delta_byte_array_long_shared_prefix{,_fixed_len}` in `parquet/src/encodings/encoding/mod.rs` and `delta_byte_array_long_shared_prefix` in `parquet/src/arrow/arrow_writer/mod.rs`. Each writes values with a 1000-byte shared prefix — not a multiple of the 32-byte block, so the scan has to resolve a partial block — and asserts on the prefix lengths actually written, decoded back out of the page, rather than on a round trip alone. Those assertions were checked for power by mutation: dropping the sub-block tail scan from `common_prefix_length` (so it under-counts by up to 31 bytes) leaves all 156 `arrow::arrow_writer` tests passing on `main`, and fails all three new tests. Benchmarked with `parquet/benches/arrow_writer.rs`'s `bench_delta_byte_array_writers`, added in #10512. Results in a comment below. # Are there any user-facing changes? No API changes and no change to written output. `DELTA_BYTE_ARRAY` writes get faster. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Ed Seidl <etseidl@users.noreply.github.com>
adriangb
force-pushed
the
fix-delta-byte-array-page-split-10489
branch
from
August 4, 2026 20:29
6190b5a to
bd5237f
Compare
adriangb
added a commit
to pydantic/arrow-rs
that referenced
this pull request
Aug 4, 2026
The byte-budget chunker computes how many values fit in a page budget, then scaled that count to a level count by the chunk-wide level:value ratio, rounded up. For a chunk with no nulls this is exact. With one null in 17 levels, `ceil(17/16) == 2`, and `write_granular_chunk` sliced the chunk into uniform two-level windows that mostly carry two values — so a window could hold twice the values its budget allowed. Return the value count from the chunker and have `write_granular_chunk` end each window by walking definition levels until it has covered that many values. The walk runs only in the granular path, whose values are by definition large enough to overflow a page budget, and that path already makes full def-level and rep-level passes. Two effects, both on columns whose values exceed `data_page_size_limit`: - The apache#9972 per-page bound now holds on nullable columns, one value per page rather than two. Pinned by `test_column_writer_caps_page_size_with_sparse_nulls`, which uses PLAIN so it is independent of the first-value exemption. - The exemption that preserves DELTA_BYTE_ARRAY prefix dedup fires when a page opens with a single-value mini-batch, which is now guaranteed regardless of where nulls fall. 16 identical 64 KiB values with one null, against a 16 KiB limit, go from five pages storing ~5 values in full to one 17-level page storing ~1 — re-pinning the test that apache#10505 left as a marker for exactly this. Repeated columns are unchanged: a record still cannot span pages, so a record holding several over-limit values still exceeds the budget. Closes apache#10538
adriangb
added a commit
to pydantic/arrow-rs
that referenced
this pull request
Aug 5, 2026
The byte-budget chunker computes how many values fit in a page budget, then scaled that count to a level count by the chunk-wide level:value ratio, rounded up. For a chunk with no nulls this is exact. With one null in 17 levels, `ceil(17/16) == 2`, and `write_granular_chunk` sliced the chunk into uniform two-level windows that mostly carry two values — so a window could hold twice the values its budget allowed. Return the value count from the chunker and have `write_granular_chunk` end each window by walking definition levels until it has covered that many values. The walk runs only in the granular path, whose values are by definition large enough to overflow a page budget, and that path already makes full def-level and rep-level passes. Two effects, both on columns whose values exceed `data_page_size_limit`: - The apache#9972 per-page bound now holds on nullable columns, one value per page rather than two. Pinned by `test_column_writer_caps_page_size_with_sparse_nulls`, which uses PLAIN so it is independent of the first-value exemption. - The exemption that preserves DELTA_BYTE_ARRAY prefix dedup fires when a page opens with a single-value mini-batch, which is now guaranteed regardless of where nulls fall. 16 identical 64 KiB values with one null, against a 16 KiB limit, go from five pages storing ~5 values in full to one 17-level page storing ~1 — re-pinning the test that apache#10505 left as a marker for exactly this. Repeated columns are unchanged: a record still cannot span pages, so a record holding several over-limit values still exceeds the budget. Closes apache#10538
adriangb
added a commit
to pydantic/arrow-rs
that referenced
this pull request
Aug 5, 2026
Value-exact windows roughly double the mini-batch count on a nullable column, so they are worth taking only where they change the bytes written. For `DELTA_BYTE_ARRAY` they change them a great deal: 128 values at one null in 16, 2 MiB each, go from 16 MiB to 2 MiB, and from 64 MiB to 8 MiB once the values are 8 MiB. For `PLAIN` and `DELTA_LENGTH_BYTE_ARRAY` they change nothing at all — a value costs the same wherever it lands — while doubling the page count and costing ~28% throughput on a nullable column. Gate on the `compresses_against_previous_value` flag apache#10505 added, which already marks exactly the encodings whose values are cheaper next to their predecessor. What the other encodings give up is bounded and small. A ratio-scaled window spans `ceil(values * levels / values_in_chunk)` levels, so where one value already fills the page budget it covers at most two values whatever the null density, and exactly one wherever the ratio is a whole number. Against a 1 MiB limit that is a 4 MiB page for 2 MiB values instead of 2 MiB — and 2 MiB is the floor, since a page must hold a value. It does not grow with `write_batch_size`, which is the failure apache#9972 fixed. `test_column_writer_caps_page_size_with_sparse_nulls` now pins two values per page rather than one, so it fails both if this gate is dropped and if the bound is lost.
…page limit A `BYTE_ARRAY` value larger than `data_page_size_limit` exceeds the limit on its own, so the post-write `should_add_data_page` check cuts a page after every single value. Parquet requires at least one value per data page, so that value is unsplittable and the limit is simply unsatisfiable there. For `DELTA_BYTE_ARRAY` this is destructive rather than merely wasteful: each page boundary discards the encoder's previous value, so every value gets `prefix_length = 0` and the encoding degenerates to exactly `PLAIN`. Columns of large values that share long prefixes stop being deduplicated (apache#10489). Exempt a page's mandatory first value from the byte limit when that value alone already exceeds it, so the limit applies to what follows — the bytes we can still place on another page. The exemption is gated on `ColumnValueEncoder::compresses_against_previous_value`, so only `DELTA_BYTE_ARRAY` opts in; `PLAIN` and `DELTA_LENGTH_BYTE_ARRAY` cost the same wherever a value lands and keep their existing one-value page bound. Pages therefore stay bounded by the value size rather than growing with `write_batch_size`, preserving the fix from apache#9972. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The first-value exemption triggers on a page-opening mini-batch holding exactly one value. A null in a chunk changes the level:value ratio, the byte-budget chunker rounds up to two-level mini-batches, and pages that open with a two-value mini-batch miss the exemption: they are cut after two values with the first stored in full. The mini-batch pairing the null with a value has one value, so the page it opens does get the exemption and accumulates the remaining suffixes. Pin that layout ([2, 2, 2, 2, 9] for 16 identical values with a null at index 8) so the limitation is a documented decision rather than an accident, and note it on set_page_size_floor. If the trigger is later keyed on values written to the page (0 -> 1) instead of mini-batch shape, the test fails with fewer, larger pages and should be updated to pin the improved layout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
adriangb
force-pushed
the
fix-delta-byte-array-page-split-10489
branch
from
August 5, 2026 19:20
cf1b481 to
1d6ca1a
Compare
adriangb
added a commit
to pydantic/arrow-rs
that referenced
this pull request
Aug 5, 2026
…ounts `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. For a chunk with no nulls that is exact. With one null in 17 levels it gives `ceil(17/16) == 2`, and `write_granular_chunk` slices the chunk into uniform two-level windows — most of which carry two values, twice what the budget allowed. The mechanism dates to apache#9972; apache#10505 raises its cost but does not introduce it. Where the encoding compresses a value against its predecessor, that round-up costs whole values of output. 128 values of 2 MiB at one null in 16 write 16.78 MB instead of 2.10 MB; at 8 MiB values it is 64 MiB instead of 8 MiB. Have the chunker return the value count it already computed, and let `write_granular_chunk` end a window by walking definition levels until it has covered that many values. Apply it only where it changes the bytes written, because it roughly doubles the mini-batch count on a nullable column. Two conditions: - The budget must be the data page budget, a constant `data_page_size_limit`, so a one-value budget means the value itself overflows a page. The dictionary page budget is the limit minus what the dictionary already holds, so it shrinks toward zero and reaches a one-value budget on ordinary values; cutting exactly there measured +13.0% on `string/default` and +8.3% on `string/parquet_2`. - The encoding must compress against the previous value. `PLAIN` and `DELTA_LENGTH_BYTE_ARRAY` store a value identically wherever it lands, so value-exact windows leave their output byte for byte the same while doubling the page count — +27.6% on a nullable column for no reduction in output. What the other paths give up is bounded. A ratio-scaled window spans `ceil(values * levels / values_in_chunk)` levels, covering at most two values where one already fills the budget, whatever the null density, and exactly one wherever the ratio is a whole number. Against a 1 MiB limit that is a 4 MiB page for 2 MiB values rather than 2 MiB — and 2 MiB is the floor, since a page must hold a value. It does not scale with `write_batch_size`, which is the failure apache#9972 fixed. `test_column_writer_delta_byte_array_nullable_shared_prefix_partial_dedup` is re-pinned from `[2, 2, 2, 2, 9]` to `[17]` and renamed, the layout apache#10505 left a marker for. `test_column_writer_caps_page_size_with_sparse_nulls` pins two values per page under `PLAIN`, so it fails both if the encoding gate is dropped and if the bound is lost. Closes apache#10538
Contributor
Author
|
@alamb I've addressed the comment suggestions you had. Regarding the limitation with nulls: I think it's solvable, but I propose we do it as a followup (#10554) to keep the review units contained. Responding to your other questions
I think the problem with this framing is that we are trading off best efforts here. Best effort to keep file sizes low, best effort to keep page sizes in check and best effort to be fast. None of the regimes are clearly bugs, they work without errors (aside from OOMs maybe). But I would argue that an 80MB file instead of an 8MB file is buggy behavior / a regression (which is what @jonasdedden is arguing as well I think).
I think it does address the root issue. I ran the reporter's exact repro on current main and on this PR:
We can't do better than the 8.0x page limit: a single value is already 8MiB so we are forced to exceed the limit. Parquet requires at least one value per page, so an 8 MiB value against a 1 MiB limit overshoots it by 8× no matter what. What changes is only that
For the reported case it's 10 rows on 1 page. Two-per-page is the worst case, and only for values that share no prefix. In that case deduplication has nothing to save and the file size is identical either way, so the bound is acting as a safeguard rather than as the result. That's what Regarding "just raise the page size limit": it works, but only if you know your largest value up front, and it's all-or-nothing for the column. In the table above, raising the limit to 4 MiB still gives 80 MiB, because the values are 8 MiB; you have to clear the largest value you'll ever write. On complexity: the mechanism is 30 lines. One trait method w/ a default, one The "only check page sizes at batch boundaries" option is genuinely simpler but it would regress the work that #9972 was added to prevent, i.e. reverts to huge pages for the case being reported here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DELTA_BYTE_ARRAYstops deduplicating in parquet 59 for values larger than the page size limit #10489The cause is the page flush, not the mini-batch splitting
The issue attributes the regression to #9972's byte-budget sub-batching. It's actually the page flush; the sub-batching only exposes it.
should_add_data_pagefires whenestimated_data_page_size() >= data_page_size_limit. ForDELTA_BYTE_ARRAYthat estimate is the real encoded size, and the first value on a page is stored in full — so a single 8 MiB value against a 1 MiB limit puts the page over the limit by itself and triggers a flush. Flushing clears the encoder'sprevious, so the next value getsprefix_length = 0, and so on. The encoding degenerates to exactlyPLAIN.Before #9972 a 1024-row mini-batch meant the check simply didn't run until 1024 values had been written, so the dedup survived. That was an accident of
write_batch_size, not a designed property — the same column at 2000 rows already lost the prefix at the 1024-value boundary.This matters for choosing a fix: making the byte budget encoding-aware (suggestion 2 in the issue) does not fix the reported bug. However precise the budget, the first value alone still exceeds the limit and still triggers the flush.
The fix
Parquet requires at least one value per data page, so a value larger than the limit cannot be split out — the limit is unsatisfiable for it. Counting those bytes against the limit is what forces the pathological one-value-per-page cut.
Record that first value's encoded size in
PageMetrics::page_size_floorand apply the limit to what follows it, i.e. the bytes that can still go on another page.The exemption is gated on a new
ColumnValueEncoder::compresses_against_previous_value(defaultfalse,trueonly forDELTA_BYTE_ARRAYin both the generic and arrow encoders).PLAINandDELTA_LENGTH_BYTE_ARRAYcost the same wherever a value lands, so there is nothing to preserve by keeping values together and they keep their existing tighter one-value page bound. All three regression tests from #9972 pass unmodified.Why not "don't split when the split cannot help"
Suggestion 1 in the issue restores the dedup, but by removing the bound that #9972 added, for exactly the workload it was added for. Measured with
Some(1) => chunk_sizeinbyte_budget_chunker.rs, 2048 rows × 128 KiB distinct values against a 64 KiB page limit — the same regime as a 10 MB value against the 1 MiB default, scaled to fit in RAM:Page size under suggestion 1 is
write_batch_size × value_size. At 1024 × 10 MB that is a 10 GB page, which is the failure #9972 fixed.Results
The reporter's table, reproduced verbatim (10 identical 8 MiB values, raw input 80 MiB):
data_page_size_limitValues that merely share a long prefix behave the same. Values sharing no prefix are unaffected in file size and stay bounded at up to two values per page — the exempt one plus the one that trips the budget.
Tests
test_column_writer_delta_byte_array_dedups_large_shared_prefix_values— 16 identical 64 KiB values, 16 KiB page limit. Fails onmainwith 1 MiB across 16 pages (byte for byte whatPLAINproduces); passes here at ~one value's worth.test_column_writer_delta_byte_array_bounds_pages_without_shared_prefix— the same column with values differing from byte 0, asserting pages stay bounded by two values. Guards against fixing this by dropping the bound.test_large_string_delta_byte_array_shared_prefix— theArrowWriterpath from the report, as an exact page-layout assertion.Notes
One known limitation, pinned by
test_column_writer_delta_byte_array_nullable_shared_prefix_partial_dedup: the exemption triggers when a page opens with a mini-batch holding exactly one value. That is guaranteed for chunks without nulls (the byte-budget chunker gives an over-limit value its own mini-batch), but a null in a chunk makes the chunker convert values to levels by ratio and round up, so pages can open with a two-value mini-batch, miss the exemption, and store their first value in full. Dedup on such columns is partial rather than absent, and never worse thanmain, which stores every value in full. The clean fix is for the granular path to cut windows after an exact value count instead of ratio-scaling; that is independent of this change and tracked in #10538. The pinning test documents the layout to expect once it lands.A follow-up worth considering separately: the byte budget in
count_within_budget_*still measures raw payload length, soDELTA_BYTE_ARRAYcolumns sub-batch more eagerly than the encoded size warrants. That is a throughput question rather than a correctness one, and it is not what caused this regression.🤖 Generated with Claude Code