Rich-T-kid opened a new issue, #11748:
URL: https://github.com/apache/arrow-rs/issues/11748
### Describe the problem
I ran into this while working on preserving Parquet dictionaries in
DataFusion.
An unannotated Parquet `BYTE_ARRAY` column is inferred as Arrow `Binary`. If
the column has dictionary pages and the supplied reader schema requests
`Dictionary<Int32, Utf8>`, the Parquet reader keeps the physical `Binary`
dictionary values instead of converting them to `Utf8`.
On current `main` (`d58a0fce35`), the reader returns `Ok`, but the batch
schema and the actual column disagree:
```text
batch schema: Dictionary(Int32, Utf8)
column type: Dictionary(Int32, Binary)
```
This can be reproduced by writing a dictionary-encoded `BinaryArray`,
reading it with `ArrowReaderOptions::with_schema` set to `Dictionary(Int32,
Utf8)`, and comparing these two values:
```rust
assert_eq!(batch.schema().field(0).data_type(), &dict_utf8);
assert_eq!(batch.column(0).data_type(), &dict_binary); // current result
```
The practical result in DataFusion is that it cannot send the logical string
dictionary schema straight through to the reader. The current path expands and
rebuilds the dictionary:
```text
Parquet RLE dictionary
-> plain Binary rows
-> Utf8 rows
-> rebuild Dictionary<Int32, Utf8>
```
That produces the right logical type, but it turns an operation over the
distinct dictionary values into work over every row. This is tracked in
[DataFusion #26530](https://github.com/apache/datafusion/issues/26530).
### Expected behavior
For dictionary pages, the reader should decode the physical dictionary as
`Dictionary<K, Binary>`, convert the distinct values once, and keep the keys:
```text
Parquet RLE dictionary
-> Dictionary<Int32, Binary>
-> Dictionary<Int32, Utf8>
```
The general Arrow dictionary cast already works this way. The missing part
is applying the same idea inside the Parquet dictionary reader when the
requested value type differs from the physical value type. If the same Parquet
dictionary values are shared by several output batches, the converted values
should be shared as well rather than cast once per batch.
Plain-encoded pages would still need the existing dictionary-building
fallback. This issue is specifically about not losing a dictionary that is
already present in the file.
### Why this matters for DataFusion and ClickBench
DataFusion's [cost-based dictionary encoding
project](https://github.com/apache/datafusion/issues/24111) is trying to
preserve Parquet dictionaries when they are useful and avoid them when they are
not. That choice only pays off if a selected Parquet dictionary stays a
dictionary through schema coercion.
ClickBench is a useful real-world example. Its textual columns are stored as
unannotated `Binary`, so the DataFusion benchmark [always enables
`binary_as_string`](https://github.com/apache/datafusion/blob/6131a9813af3127c909cb6a2ff9ba363b036dc1d/benchmarks/src/clickbench.rs#L188-L193).
The arrow-rs ClickBench reader has [the same Binary-to-string
workaround](https://github.com/apache/arrow-rs/blob/d58a0fce35b67836ac9b2e20a7efe47a584e0a86/parquet/benches/arrow_reader_clickbench.rs#L939-L948).
When direct RLE dictionary reads were enabled in [DataFusion PR
#24227](https://github.com/apache/datafusion/pull/24227), ClickBench was about
3.8x slower overall and several queries were roughly 5x slower in [this
benchmark
run](https://github.com/apache/datafusion/pull/24227#issuecomment-5365010866).
Those results also include high-cardinality columns and dictionary fallback to
plain pages, so this conversion is not the only cause. However, because
ClickBench represents text as `Binary`, any selected string dictionary goes
through this exact `Binary` to `Utf8` coercion path. Expanding and hashing all
rows again adds the cost that dictionary preservation was meant to avoid.
Related issues:
-
[apache/datafusion#26530](https://github.com/apache/datafusion/issues/26530)
isolates the expansion/rebuild path and includes a focused benchmark.
-
[apache/datafusion#24111](https://github.com/apache/datafusion/issues/24111)
tracks cost-based dictionary encoding.
- [#11140](https://github.com/apache/arrow-rs/issues/11140) tracks UTF-8
validation for `Binary` to string schema overrides; this change should follow
whatever invalid-input behavior is decided there.
- [#11414](https://github.com/apache/arrow-rs/issues/11414) tracks the
dictionary reader's performance when the physical and requested value types
already match.
### Suggested regression coverage
1. Write an unannotated `Binary` column with dictionary encoding enabled.
2. Request `Dictionary<Int32, Utf8>` through
`ArrowReaderOptions::with_schema`.
3. Use a small batch size so one Parquet dictionary is used by multiple
output batches.
4. Verify the schema and actual column type are both `Dictionary<Int32,
Utf8>`.
5. Verify the keys are unchanged and the converted values array is reused
across batches.
--
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]
