adriangb commented on PR #11165: URL: https://github.com/apache/arrow-rs/pull/11165#issuecomment-5804756410
@alamb thanks, good questions The annotations must give information that the order cannot give. Here is the use case for each one: - `first_row` / `last_row`: when to release a buffer. The order tells the caller what to fetch next. It does not tell the caller when it can drop bytes that it fetched. With row tags, the caller releases a page when the decode cursor passes `last_row`. This keeps memory below a byte budget. In https://github/com/apache/datafusion/pulls/24086 this changed peak memory on one 300 MB row group from 283 MB to 100 MB. The caller can also set its window in rows, for example "fetch 4 batches ahead". <link to the code in #24086 that uses the row tags> - `conditional`: this give us the invariant that you wanted. With filter pushdown, a returned range will not always come back as `NeedsData`. That is correct for the full plan. But for entries with `conditional == false`, the invariant stays true: the decoder will request those bytes. Only entries that an earlier filter stage can remove get `conditional == true`. The caller can then use a policy that agrees with its storage: - Archival storage, high cost per request: fetch all entries. - Object storage: fetch certain entries first, then conditional entries while budget remains. - SSD: skip anything with `conditional = true` The order alone cannot give this, because decode order mixes certain and conditional pages. - `row_group`: fall back to row-group prefetch. Older files do not have a page index, namely Clickbench files. For these files the plan has one entry for each column chunk. The caller uses `row_group` to put the column chunks of one row group together and fetch them in one request. This gives the same behavior as row-group readahead today, but it comes from the same plan. A caller can also select this granularity for files that have a page index, for example if its storage has a high cost for each request, if it wants to prioritize efficiency over memory use and latency, etc. - `column` / `kind`: I agree these may be over-engineered, we can remove them for now. If we add `#[non_exhaustive]` we can expand these later. That leaves us with: ```rust #[non_exhaustive] pub struct PlannedRange { pub range: Range<u64>, pub first_row: u64, pub last_row: u64, pub row_group: usize, pub conditional: bool, } ``` **About the buffer-independent plan:** you are right that both approaches keep fetch policy outside Parquet. For me the better reason is stability. The plan does not change when data is pushed or cleared. So we do not need rules for when a preview becomes out of date, and the duplicate row group case that @viirya found goes away. Comparing to the metadata based approach in https://github.com/apache/datafusion/24395: I prefer that the plan comes from the decoder. Then offset/limit and row selections use the same code as `NeedsData`, and the plan cannot drift from what the decoder reads. For the tests, I propose this invariant: for each row group, the bytes in the non-conditional entries are the same as the bytes that `NeedsData` requests. -- 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]
