Add page-level I/O and materialization in Hybrid Scan - #23375
Add page-level I/O and materialization in Hybrid Scan#23375mhaseeb123 wants to merge 20 commits into
Conversation
Preserve page locations and variable-width offset state needed to safely reconstruct columns from a sparse subset of Parquet data pages.
Expose multifile page-range planning and consume the selected payload pages so hybrid scan avoids fetching pruned Parquet payload data.
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Part of #23362 This PR includes bug fixes and supporting features needed to enable page pruning with page-level (sparse) I/O (for payload columns) in hybrid scan, that is upcoming in #23375 and includes end to end tests. Authors: - Muhammad Haseeb (https://github.com/mhaseeb123) Approvers: - Paul Mattione (https://github.com/pmattione-nvidia) - Vukasin Milovanovic (https://github.com/vuule) - Bradley Dice (https://github.com/bdice) - Nghia Truong (https://github.com/ttnghia) URL: #23374
…sparse-page-io-hybrid
ce70f6f to
1cf8e93
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds payload page-range APIs, sparse page-data chunking, page-level pass masks, sparse Parquet preprocessing, and multifile hybrid-scan coverage for pruning, dictionaries, missing offset indexes, and row-group ordering. ChangesSparse payload hybrid scan
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
|
||
| // Must be called as soon as we create the pass | ||
| set_pass_page_mask(data_page_mask); | ||
| setup_next_pass(column_chunk_data, data_page_mask); |
There was a problem hiding this comment.
set_pass_page_mask is now moved inside the setup_next_pass
|
|
||
| // Setup page information for the chunk (which we can access without decompressing) | ||
| setup_compressed_data(column_chunk_data); | ||
| if (_sparse_page_io) { |
There was a problem hiding this comment.
Use either the dense or sparse overloads of the setup_compressed_data and set_pass_page_mask APIs.
There was a problem hiding this comment.
The sparse vs dense functions seem on the edge of similarity where it could make sense to use a template parameter and if constexpr instead of having separate functions. I'll defer to your judgment, just a thought.
There was a problem hiding this comment.
True for setup_xx_compressed_data but not really for the setup_xx_page_mask. Will defer this to a follow up if the duplicated portion seems to be growing
| * @param schema_indices Schema indices from the first source | ||
| * @return A pair indicating column-index and offset-index presence, respectively | ||
| */ | ||
| [[nodiscard]] std::pair<bool, bool> page_index_presence( |
There was a problem hiding this comment.
Simply moved from private to public scope
Part 1 of 3. Contributes to #23519 This PR improves Parquet reader's pass construction by making it column chunk aware so that it is more accurate (less conservative) when reading select columns from the file. Subsequent PRs will propagate these improvements to hybrid scan as well as enable hybrid scan to calculate at page-level (needs #23375) Authors: - Muhammad Haseeb (https://github.com/mhaseeb123) Approvers: - Bradley Dice (https://github.com/bdice) - Qi Chen (https://github.com/qbacpey) URL: #23446
vyasr
left a comment
There was a problem hiding this comment.
Some minor thoughts but nothing blocking, LGTM!
|
|
||
| // Setup page information for the chunk (which we can access without decompressing) | ||
| setup_compressed_data(column_chunk_data); | ||
| if (_sparse_page_io) { |
There was a problem hiding this comment.
The sparse vs dense functions seem on the edge of similarity where it could make sense to use a template parameter and if constexpr instead of having separate functions. I'll defer to your judgment, just a thought.
Description
Contributes to #23519
This PR adds new hybrid scan APIs to support page-level (sparse) I/O for payload columns. This includes:
Checklist