You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "zhuqi-lucas (via GitHub)" <gi...@apache.org> on 2026/09/21 13:51:10 UTC

[I] Parquet: `skip_records` eagerly decompresses dictionary pages for column chunks it never decodes [arrow-rs]

zhuqi-lucas opened a new issue, #11154:
URL: https://github.com/apache/arrow-rs/issues/11154

   ### Is your feature request related to a problem or challenge?
   
   `GenericColumnReader::skip_records` decompresses the dictionary page of a column chunk as soon as it encounters it, even when every remaining page of that chunk is subsequently skipped at page level and no value from the chunk is ever decoded:
   
   https://github.com/apache/arrow-rs/blob/e3a21bf9f4e1c9f9432a082a1a6cbba9dbc02e1c/parquet/src/column/reader.rs#L320-L324
   
   ```rust
   // If dictionary, we must read it
   if metadata.is_dict {
       self.read_dictionary_page()?;
       continue;
   }
   ```
   
   `read_dictionary_page` goes through `get_next_page` → `decode_page`, which fully decompresses the page. For zstd-compressed files with large dictionaries (wide string columns such as tickers/symbols), this shows up prominently in production profiles.
   
   **Production evidence.** We run DataFusion-based query servers over zstd-compressed Parquet with `pushdown_filters = true` and the page index disabled (`SerializedPageReaderState::Values`). 30-second CPU profiles of the two hottest data-server classes:
   
   | pod | total samples | `skip_records` (cum) | of which `read_dictionary_page` |
   |---|---|---|---|
   | stocks aggregates | 40.65s | 22.8% | 17.9% (7.29s, 78% of skip cost) |
   | stocks quotes | 28.77s | 15.6% | 13.6% (3.91s, 87% of skip cost) |
   
   All `read_dictionary_page` samples were reached from `skip_records`. The quotes profile also shows `CachedArrayReader` at 18.4%: when the predicate cache replays batches, the inner reader consumes whole row groups purely via `skip_records`, so every dictionary decompressed on that path is pure waste.
   
   The waste is largest exactly where skipping is otherwise cheapest: for flat columns (`num_levels == num_rows` fallback) whole pages are skipped without decompression even with V1 headers, so a fully-skipped chunk's *only* remaining decompression cost today is its dictionary page.
   
   Related but distinct: #6454 discusses V1 data pages being decompressed to count rows during skip; the dictionary page cost is unconditional and hits even the well-behaved page-level skip path. #10655 (forward skipping on a retained reader) would make skip-dominated chunk consumption more common and increase the benefit.
   
   ### Describe the solution you'd like
   
   Defer dictionary page decompression until the chunk actually needs it, i.e. record the dictionary page location on the skip path instead of decoding it, and decode on the first `read_new_page`/`set_data` that references a dictionary-encoded data page. All the pieces already line up:
   
   1. `skip_next_page` already skips any page type without decompressing, and already tracks `require_dictionary` state:
   https://github.com/apache/arrow-rs/blob/e3a21bf9f4e1c9f9432a082a1a6cbba9dbc02e1c/parquet/src/file/serialized_reader.rs#L1104-L1146
   2. Skipping *values* inside a dictionary-encoded page does not need the dictionary contents: `ByteArrayDecoderDictionary::skip` only advances the index decoder (the dict is used for an all-NULL emptiness check), and the generic `DictDecoder` path likewise skips indices. The current requirement that the dictionary exist before `set_data` ("Decoder for dict should have been set") is plumbing, not an algorithmic need.
   3. The read path already installs the dictionary itself (`read_new_page` handles `Page::DictionaryPage` via `set_dict`), so a deferred dictionary decode has a natural home.
   
   The change is never worse than today: at most it moves the decompression to the first real read of the chunk, and it eliminates it entirely for chunks that are opened but fully skipped (predicate-cache replay, selective `RowSelection`s, early-terminated limits).
   
   **Prior art.** Velox's `PageReader::seekToPage` skips dictionary pages with a raw `skipBytes` (no decompression) in its rep/def-only pass and only calls `prepareDictionary` on the value pass. DuckDB byte-skips fully-filtered data pages, though it intentionally decodes dictionaries eagerly because it evaluates predicates against the dictionary itself. That is worth keeping in mind for the design here: the deferral should not bake in an assumption that the dictionary is never needed before value decode, in case dictionary-level predicate evaluation is added later.
   
   I am happy to submit a PR if this direction sounds reasonable.
   
   ### Describe alternatives you've considered
   
   - Enabling the page index / V2 page headers helps the *data page* skip cost (#6454) but does not avoid the dictionary decompression, which happens before any page-level skip decision.
   - Caching decompressed dictionaries across readers helps repeated opens but not the first, and adds memory pressure; deferral is strictly cheaper.
   
   ### Additional context
   
   Profiles collected with pprof (99Hz, 30s) on production servers; numbers above are cumulative shares of total samples.
   


-- 
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: github-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Parquet: `skip_records` eagerly decompresses dictionary pages for column chunks it never decodes [arrow-rs]

Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on issue #11154:
URL: https://github.com/apache/arrow-rs/issues/11154#issuecomment-5761589933

   take


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Parquet: `skip_records` eagerly decompresses dictionary pages for column chunks it never decodes [arrow-rs]

Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on issue #11154:
URL: https://github.com/apache/arrow-rs/issues/11154#issuecomment-5790634238

   Implemented in #11168. One constraint worth recording here, because it is not visible from the problem statement and it changed the shape of the fix.
   
   The description above says the "Decoder for dict should have been set" requirement is plumbing rather than an algorithmic need. That holds only while the deferral is guaranteed to fire. `PageReader` is a public trait, and `skip_next_page` is where the skip path now passes a dictionary page — but only `SerializedPageReader` retains one. Any other implementation drops it, the defaulted `take_deferred_dictionary` then returns `None`, and the first dictionary-encoded data page reaches `set_data` with no decoder and panics. So the assert is load-bearing for exactly the readers the change does not control.
   
   The fix is a second defaulted trait method, `supports_deferred_dictionary`, defaulting to `false`. The skip path asks before it skips, and a reader that has not opted in keeps the eager `read_dictionary_page` it had before. A new trait method can only be non-breaking if its default lands on the conservative side, not merely on a side that looks harmless.
   
   That probe also closes a second hole. In the offset-index state a dictionary is only representable when the metadata recorded a `dictionary_page_offset`: `dictionary_page` is synthesised from the gap between `byte_range().0` and the first page location, and those coincide when the offset is absent, so a dictionary inlined ahead of the first data page is indistinguishable from a data page location. `SerializedPageReader` answers the probe per state and declines there.
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Parquet: `skip_records` eagerly decompresses dictionary pages for column chunks it never decodes [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #11154: Parquet: `skip_records` eagerly decompresses dictionary pages for column chunks it never decodes
URL: https://github.com/apache/arrow-rs/issues/11154


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org