zhuqi-lucas opened a new pull request, #11168: URL: https://github.com/apache/arrow-rs/pull/11168
# Which issue does this PR close? Closes #11154. # Rationale for this change `GenericColumnReader::skip_records` decompresses a column chunk's dictionary page as soon as it reaches it, even when every remaining page is then skipped and no value from the chunk is ever decoded. Skipping values needs only the RLE index cursor and skipping whole pages needs nothing, so dictionary contents are only ever needed by value decoding. On zstd-compressed data with large dictionaries this is a large, avoidable cost, and it is most visible with `pushdown_filters`/`RowSelection` and with predicate caching, where output columns are consumed largely by skipping. A local timing test on a zstd column whose dictionary holds 40k distinct strings (14,800 compressed bytes): | | | | --- | --- | | whole-chunk skip, dictionary never decoded | **84 µs** | | partial skip that pays the deferred dictionary + first page | 4.76 ms | # What changes are included in this PR? - The skip path records the dictionary page's location (`DeferredDictionaryPage`) instead of decoding it. A data page that turns out to need the dictionary installs it then, paying the decompression exactly once; a chunk skipped end to end never pays it. The re-read is a random `ChunkReader::get_read` at the recorded offset (already part of the trait; an in-memory slice on the async path). - `PageReader::take_deferred_dictionary`, a new trait method defaulting to `Ok(None)`, so a reader that never defers is unchanged and the eager path (a dictionary page arriving through `get_next_page`) is untouched. - Whether a page is a dictionary is judged by its actual header type, not the `dictionary_page_offset` metadata flag, so files that inline the dictionary without recording an offset (older parquet-mr writers, e.g. the one behind `test_read_nested_lists`) are handled correctly. # Are these changes tested? Yes: - whole-chunk skip never decodes the dictionary (corrupting the dictionary body leaves the skip green); - a partial skip pays it (the same corruption then fails); - skip-then-read returns correct values; - the plain read path is unchanged; - a timing test prints the avoided cost. The existing `test_read_nested_lists` (nested column + `RowSelection`) caught the offset-less-dictionary case during development and now guards it. # Are there any user-facing changes? No public behavior change and no format change. `PageReader` gains one defaulted trait method; custom implementations that wrap another `PageReader` should forward it to benefit, but are correct without doing so. -- 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]
