hhhizzz opened a new pull request, #10735: URL: https://github.com/apache/arrow-rs/pull/10735
# Proposed title fix(parquet): prevent cached Mask reads from crossing unloaded sparse pages # Which issue does this PR close? - Closes #10733. # Rationale for this change The async Parquet reader may combine page pruning, predicate caching, and a Mask-backed row selection. `MaskCursor::next_chunk` bounds each chunk by the current loaded row range, but previously retained all trailing skipped rows up to that boundary. For example, if a loaded range starts with one selected row and the rest of the range is skipped, the cursor returned the whole loaded range as `chunk_rows`. A fixed-size cached reader can then extend that read beyond the cached segment and into a sparse page that was never loaded, producing: ```text Invalid offset in sparse column chunk data: ..., no matching page found ``` DataFusion predicate pushdown exposed this in TPC-DS queries 66, 75, and 81. Disabling the predicate cache avoided the failure, but the cache is enabled by default and should be safe with both Auto and explicit Mask selection. # What changes are included in this PR? - Track the position immediately after the last selected row in the current loaded range. - Use that position for both `MaskCursor::position` and `MaskChunk::chunk_rows`, instead of the scan position at the end of the loaded range. - Leave trailing skipped rows to the next cursor step. They become part of `initial_skip`, allowing `ArrayReader::skip_records` to cross unloaded pages. - Update the read-plan unit test to lock down the chunk boundaries. - Add an async regression test with three pages, a cached predicate column, a sparse initial selection, and both Auto and explicit Mask policies. The selected rows and output batch semantics are unchanged. The fix only avoids decoding trailing rows that are not selected. # Are these changes tested? Yes. Focused regression test: ```shell cargo test -p parquet --features async --test arrow_reader 'row_filter::r#async::test_cached_mask_reads_sparse_pages_without_error' -- --exact --nocapture ``` - Test-only commit on top of `bb1e6cd070`: failed with the sparse-column offset error. - This fix: 1 passed, 0 failed. - Mask-focused `arrow_reader` tests: 7 passed, 0 failed. I also ran one-iteration end-to-end correctness smoke tests with DataFusion `52964c2966e855f47b96a15d1aace01baee1f2c1`. For validation only, the Arrow default policy was changed to Mask on top of this PR so the workloads could not fall back to Selectors: - TPC-DS SF10: 99/99 queries succeeded. The 96 queries that had successful pre-fix results had identical row counts; the previously failing Q66, Q75, and Q81 also succeeded. - TPC-H SF10: 22/22 queries succeeded, with identical row counts to the pre-fix baseline. - ClickBench: 43/43 queries succeeded with predicate pushdown explicitly enabled, with identical row counts to the pre-fix baseline. - No `invalid offset`, `no matching page`, panic, or query failure was found in the candidate logs. These workload runs are correctness smoke tests, not performance claims. ## Suggested CI follow-up cc @alamb This bug requires an interaction between predicate pushdown, page pruning, selection representation, and predicate caching, which is difficult to cover with isolated reader tests alone. I suggest adding a CI or scheduled benchmark correctness check that runs representative TPC-DS, TPC-H, and ClickBench queries with **Parquet predicate pushdown enabled**; # Are there any user-facing changes? No API changes. Async Parquet scans using predicate caching and sparse page reads no longer fail when Auto or explicit Mask selection is used. -- 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]
