zhuqi-lucas commented on PR #11168: URL: https://github.com/apache/arrow-rs/pull/11168#issuecomment-5788235591
cc @alamb @etseidl — this one could use your eyes when you have a moment. Short version: `skip_records` decompresses a column chunk's dictionary page the moment 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, so the dictionary is only ever needed by value decoding. This records the page's location instead and installs it on the first data page that actually needs it, so a chunk skipped end to end never pays. Most visible under `pushdown_filters` / `RowSelection` and with predicate caching, where output columns are consumed largely by skipping. Two things I would particularly like an opinion on: - `PageReader::take_deferred_dictionary` is a new trait method, defaulted to `Ok(None)` so existing implementations stay correct without changes. Whether that is the right shape for the deferral — or whether it belongs somewhere else entirely — is more your call than mine. - Dictionary-ness is judged by the page header's actual type rather than the `dictionary_page_offset` metadata flag, because some older parquet-mr files inline a dictionary without recording an offset (`test_read_nested_lists` is one, and it caught this during development). That felt like the safer reading, but it is a compatibility call and I would like a second opinion. One thing worth flagging honestly: the benefit is not covered by any existing benchmark. The `arrow_reader` skip groups alternate skip and read, and build uncompressed binary-packed pages in memory, so they never exercise a whole-chunk skip over a compressed dictionary column — running them against this PR measures only the read-path overhead, which is the +0.3~0.7% in the description. The 84 µs / 4.76 ms figures come from a local timing test. I am happy to add a bench covering whole-chunk-skip, skip-then-read, and plain read if you think that is worth having. -- 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]
