hhhizzz commented on PR #10141:
URL: https://github.com/apache/arrow-rs/pull/10141#issuecomment-5018312161

   #10288 is about to land and changes Mask execution across page-pruned data. 
When rebasing this PR, could we preserve its `LoadedRowRanges`-aware Mask path 
and thread `loaded_row_ranges` into both cursor constructors, especially:
   
   ```rust
   new_mask_from_buffer(mask, loaded_row_ranges)
   ```
   
   #10288 only has `new_mask_from_selectors`, while this PR introduces the 
direct `BooleanBuffer -> MaskCursor` path. If the ranges are not propagated 
there, `RowSelection::from_boolean_buffer(...)` can still cross an unloaded 
page and reproduce the sparse-page offset failure.
   
   Could we also add a sparse-page regression test starting from a direct, 
preferably non-byte-aligned, `BooleanBuffer::slice(...)`? The existing #10288 
tests all start from `Vec<RowSelector>`, so they do not cover this integration 
seam.
   
   The old `should_force_selectors` / `override_selector_strategy_if_needed` 
fallback should be removed after the rebase, since #10288 makes Auto remain 
Mask safely across skipped pages.


-- 
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]

Reply via email to