alamb commented on code in PR #10735:
URL: https://github.com/apache/arrow-rs/pull/10735#discussion_r3806508009
##########
parquet/tests/arrow_reader/row_filter/async.rs:
##########
@@ -169,6 +169,74 @@ async fn test_row_filter_full_page_skip_is_handled_async()
{
}
}
+#[tokio::test]
+async fn test_cached_mask_reads_sparse_pages_without_error() {
Review Comment:
I verified this test fails without the code in this pR
```
----
arrow::arrow_reader::read_plan::tests::mask_backed_plan_respects_loaded_row_ranges
stdout ----
thread
'arrow::arrow_reader::read_plan::tests::mask_backed_plan_respects_loaded_row_ranges'
(24142388) panicked at parquet/src/arrow/arrow_reader/read_plan.rs:640:9:
assertion `left == right` failed
left: 4
right: 1
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
##########
parquet/src/arrow/arrow_reader/selection/cursor.rs:
##########
@@ -176,10 +176,11 @@ impl SelectorsCursor {
/// LoadedRowRanges: [0, 4) [10, 12)
/// ```
///
-/// The first chunk decodes `[0, 4)` with mask `1000`. The next chunk skips to
-/// row 11 and decodes `[11, 12)` with mask `1`. The loaded ranges are decode
-/// boundaries, not output batch boundaries: [`ParquetRecordBatchReader`]
-/// accumulates both chunks and applies the combined mask `10001` once.
+/// The first chunk decodes `[0, 1)` with mask `1`. The next chunk skips to row
+/// 11 and decodes `[11, 12)` with mask `1`. Trimming trailing skipped rows
from
Review Comment:
I found this discussion about caching confusing here (as the iterator is not
handling the caching)
I think it would be clearer to state this as a new invariant / guarantee of
the cursor -- namely that it will not return trailing `0`s/unselected rows --
and then you can add a comment in the cache that it relies on this property
--
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]
