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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
