jonasdedden opened a new issue, #10489:
URL: https://github.com/apache/arrow-rs/issues/10489
### Describe the bug
## Summary
parquet 59 added byte-budget-aware mini-batch splitting to the column writer.
The new splitting has an unintended interaction with `DELTA_BYTE_ARRAY`.
When an individual value is
larger than `data_page_size_limit`, the chunker is forced to emit one value
per page. Prefix state is
reset at every page boundary, so every value gets `prefix_length = 0` and
`DELTA_BYTE_ARRAY`
degenerates to exactly `PLAIN`. Columns of large values that share long
prefixes — or are outright
identical — stop being deduplicated entirely.
The default `data_page_size_limit` is 1 MiB, so any `BYTE_ARRAY` column with
values above 1 MiB is
affected out of the box.
**Affects:** `parquet` 59.0.0 and 59.1.0 (58.4.0 is fine)
**Impact:** 10× output size in our test corpus; write-syscall count rises by
the same factor
### To Reproduce
Ten identical 8 MiB strings, `DELTA_BYTE_ARRAY` forced, dictionary and
statistics off, no
compression. Requires only `arrow` and `parquet`.
```rust
use arrow::array::{ArrayRef, RecordBatch, StringArray};
use arrow::datatypes::{DataType, Field, Schema};
use parquet::arrow::ArrowWriter;
use parquet::basic::{Compression, Encoding};
use parquet::file::properties::{EnabledStatistics, WriterProperties};
use std::sync::Arc;
fn write(page_limit: Option<usize>) -> usize {
const NUM_ROWS: usize = 10;
const VALUE_LEN: usize = 8 * 1024 * 1024;
let value = "a".repeat(VALUE_LEN);
let schema = Arc::new(Schema::new(vec![Field::new("v", DataType::Utf8,
false)]));
let array: ArrayRef = Arc::new(StringArray::from(vec![value.as_str();
NUM_ROWS]));
let batch = RecordBatch::try_new(schema.clone(), vec![array]).unwrap();
let mut props = WriterProperties::builder()
.set_compression(Compression::UNCOMPRESSED)
.set_statistics_enabled(EnabledStatistics::None)
.set_dictionary_enabled(false)
.set_encoding(Encoding::DELTA_BYTE_ARRAY);
if let Some(limit) = page_limit {
props = props.set_data_page_size_limit(limit);
}
let mut out = Vec::new();
let mut writer = ArrowWriter::try_new(&mut out, schema,
Some(props.build())).unwrap();
writer.write(&batch).unwrap();
writer.close().unwrap();
out.len()
}
```
### Expected behavior
### Measured results
Raw input is 80.00 MiB. Ideal `DELTA_BYTE_ARRAY` output is ~8.00 MiB (one
value stored, nine
zero-length suffixes).
| `data_page_size_limit` | parquet 58.4.0 | parquet 59.1.0 |
| --- | --- | --- |
| default (1 MiB) | **8.00 MiB** | **80.00 MiB** |
| 1 MiB | 8.00 MiB | 80.00 MiB |
| 4 MiB | 8.00 MiB | 80.00 MiB |
| 8 MiB | 8.00 MiB | 80.00 MiB |
| 9 MiB | 8.00 MiB | 8.00 MiB |
| 16 MiB | 8.00 MiB | 8.00 MiB |
| 128 MiB | 8.00 MiB | 8.00 MiB |
80.00 MiB is byte-for-byte what `PLAIN` produces: the encoding is doing no
work at all.
The same result holds for values that merely *share a long prefix* rather
than being identical, so
this is not specific to exact duplicates.
### Additional context
## Suggested fixes upstream
Roughly in increasing order of ambition.
1. **Do not split when the split cannot help.** If the value at the head of
the chunk alone exceeds
the budget, a one-value page is unavoidable *for that value*, but forcing
a page boundary
afterwards is what destroys the following values' prefix compression.
Where the encoder reports
`Some(0)`, consider letting the mini-batch run to the encoder's own
page-flush decision instead of
clamping to one value — i.e. treat "cannot fit" as "budget does not apply
here" rather than "split
maximally". This restores 58's behaviour for exactly the case that
regressed, while keeping the
memory fix for the common case where values are small and a 1024-row
mini-batch is what overflows.
2. **Make the budget encoding-aware.** `count_within_budget_offsets`
measures raw payload length. For
`DELTA_BYTE_ARRAY` the encoded contribution of a value that shares a
prefix with its predecessor
is much smaller than its length, so the budget check systematically
over-estimates and splits far
more eagerly than necessary. Consulting the encoder's incremental
estimate — the same quantity
`estimated_data_page_size` already tracks — would fix both the eager
splitting and the cliff.
3. **Warn, or raise the effective floor.** At minimum,
`data_page_size_limit` silently changing a
column's effective encoding from `DELTA_BYTE_ARRAY` to `PLAIN`-equivalent
deserves a documented
note on `set_data_page_size_limit` and on `DEFAULT_PAGE_SIZE`, since the
default is what most
users hit.
Option 1 looks the most surgical: it is local to
`byte_budget_sub_batch_size` and does not require
threading encoder state into the budget calculation.
--
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]
