viirya commented on PR #11393: URL: https://github.com/apache/arrow-rs/pull/11393#issuecomment-6046251410
Late LGTM. A couple of optional nits that could go into a follow-up: - The new doc on `compute_cache_projection_inner` explains why it's an associated fn but not what it returns — maybe note it returns `None` when the cache is disabled, there are no predicates, or no top-level primitive column remains. - The moved comment "Exclude leaves belonging to roots that span multiple parquet leaves" is a bit stale: `without_nested_types` also excludes single-leaf groups now. Possible follow-up: `compute_cache_projection` still recomputes the same mask on every row group (clone + union + intersect + `without_nested_types`, which allocates). Since `projection`/`metadata`/`max_predicate_cache_size` never change and `filter` is only ever put back unchanged, it always equals `self.stages`' cache projection, so it could read that instead (falling back to `ProjectionMask::none(num_columns)`). That's the per-row-group allocation, rather than the one-time placeholder here. -- 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]
