[I] [Parquet] ScanPlan docs: document the read-ahead buffering contract, and deprecate redundant ParquetPushDecoder APIs [arrow-rs]

["adriangb (via GitHub)" <gi...@apache.org>]

adriangb opened a new issue, #11431:
URL: https://github.com/apache/arrow-rs/issues/11431

   Follow-up to https://github.com/apache/arrow-rs/pull/10555.
   
   **1. Document what a read-ahead caller must implement.** ([review comment](https://github.com/apache/arrow-rs/pull/10555#discussion_r4163563984))
   
   - **How to avoid a copy.** Without a row selection, the decoder requests a whole column chunk, and that chunk spans several planned ranges. A cache keyed by planned range must join those ranges for each request. A caller can instead fetch one buffer for each `(row_group, column)` and answer with `Bytes::slice`. This is zero-copy, and the decoder still releases the buffer, because the pushed range is the same as the requested range.
   - **When to drop cached ranges.** The decoder requests each planned range at most one time for each visit to its row group. It never requests a range that a predicate prunes. Row groups can be read out of order or more than once. For this reason, eviction must follow the position in the plan, not the `row_group` number.
   
   **2. Deprecate APIs that `ScanPlan` makes redundant.** ([review](https://github.com/apache/arrow-rs/pull/10555#pullrequestreview-5446401663))
   
   `ParquetPushDecoder::is_at_row_group_boundary` and `row_groups_remaining` give information that a caller can now get from `scan_plan()`. Find out whether they have other users. If not, deprecate them and point users to `ScanPlan`.
   


-- 
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: github-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Parquet] ScanPlan docs: document the read-ahead buffering contract, and deprecate redundant ParquetPushDecoder APIs [arrow-rs]

["wilhelmagren (via GitHub)" <gi...@apache.org>]

wilhelmagren commented on issue #11431:
URL: https://github.com/apache/arrow-rs/issues/11431#issuecomment-6149706653

   👀 
   
   One question about deprecating `is_at_row_group_boundary()`, from what I can see, it provides some information that `scan_plan()` doesn't?
   
   `is_at_row_group_boundary()` distinguishes between `ReadingRowGroup` and `DecodingRowGroup` states while `scan_plan()` uses `remaining_row_groups.scan_plan()`for both states.
   
   The `into_builder()` requires the decoder to be in `ReadingRowGroup` state (and to be exactly at row-group boundary, if I understand correctly), how would you know when its safe to call it using only `scan_plan()?
   
   Basically, could two decoder states have the same remaining scan plan but differ in whether `into_builder()` is callable without error?
   
   Maybe im missing something, or this is an unreachable state.. :)


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org