Enable hybrid scan page pruning for page level IO

[mhaseeb123]

Description

Contributes to #23519

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.

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[copy-pr-bot]

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.

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Parquet preprocessing now decodes headers from offset indexes or explicit page spans, tracks optional value metadata, skips decoding pruned pages, and reconstructs string/list offsets through a dedicated GPU path.

Changes

Parquet preprocessing updates

Layer / File(s)	Summary
Optional page metadata cpp/src/io/parquet/reader_impl_helpers.cpp, cpp/src/io/parquet/reader_impl_preprocess_utils.*, cpp/src/io/parquet/parquet_gpu.hpp	Page metadata tolerates missing column indexes and records whether value-derived fields are available.
Sparse page-header decoding cpp/src/io/parquet/page_hdr.cu, cpp/src/io/parquet/parquet_gpu.hpp, cpp/src/io/parquet/reader_impl_preprocess_utils.*, cpp/src/io/parquet/experimental/hybrid_scan_preprocess.cu	Header decoding supports offset-index and explicit per-page spans, including empty spans, with validation and span-based APIs.
Offset-index reader integration cpp/src/io/parquet/reader_impl*.{hpp,cu}, cpp/src/io/parquet/experimental/*	Reader and hybrid-scan state, chunking, page spans, and string preprocessing now use offset-index and value-information availability.
Pruned page offset handling cpp/src/io/parquet/decode_pruned_pages.cu, cpp/src/io/parquet/page_data.cu, cpp/src/io/parquet/page_delta_decode.cu, cpp/src/io/parquet/decode_fixed.cu, cpp/src/io/parquet/reader_impl.cpp, cpp/CMakeLists.txt	Pruned decode paths return before page writes, while dedicated GPU processing fills string and list offsets.
Hybrid scan validation cpp/tests/io/experimental/hybrid_scan_test.cpp	Tests validate filtered nested outputs and monotonic offsets across consecutive pruned pages.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• rapidsai/cudf#22532: Both modify the fixed-page Parquet decoding path.
• rapidsai/cudf#23090: Both modify decode_page_data_generic.
• rapidsai/cudf#23386: Both switch Hybrid Scan index gating to offset indexes.

Suggested labels: improvement

Suggested reviewers: vuule, davidwendt, pmattione-nvidia, qbacpey, pointkernel

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main change: enabling hybrid scan page pruning for page-level IO.
Description check	✅ Passed	The description directly describes the PR’s hybrid scan page-pruning support and tests.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (2)

cpp/src/io/parquet/page_hdr.cu (1)

523-561: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

decode_page_header's page parameter is unused/dead.

The function only ever writes to bs->page, never dereferences page. Both call sites (decode_using_page_index_fn, decode_from_page_data_fn) rely on a manual pages[page_idx] = bs.page; copy afterward. This is easy to misread as "decodes directly into *page", and a future refactor that drops the manual copy (assuming the function already did it) would silently regress page decoding.

♻️ Either write directly to `*page` or drop the unused parameter

-__device__ void decode_page_header(byte_stream_s* bs,
-                                   cudf::size_type chunk_idx,
-                                   PageInfo* page,
-                                   kernel_error::pointer error_code)
+__device__ void decode_page_header(byte_stream_s* bs,
+                                   cudf::size_type chunk_idx,
+                                   kernel_error::pointer error_code)
 {
   bs->page.chunk_idx      = chunk_idx;

(and update both call sites to drop the &pages[page_idx] argument, keeping their existing manual copy-back.)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/io/parquet/page_hdr.cu` around lines 523 - 561, Remove the unused
page parameter from decode_page_header and update both
decode_using_page_index_fn and decode_from_page_data_fn call sites to stop
passing &pages[page_idx]. Preserve the existing manual
bs.page-to-pages[page_idx] copy-back behavior.

cpp/src/io/parquet/parquet_gpu.hpp (1)

1040-1056: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Doc tag: pages is mutated, should be [in,out].

fill_pruned_offsets writes output offsets into pages (per fill_pruned_offsets_kernel), but is tagged @param[in] pages. Other mutating PageInfo span params in this file use [in,out] (e.g. decode_page_data, compute_page_string_sizes_pass1).

📝 Doc tag fix

- * `@param`[in] pages All pages to be processed
+ * `@param`[in,out] pages All pages to be processed

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/io/parquet/parquet_gpu.hpp` around lines 1040 - 1056, Update the
Doxygen annotation for the pages parameter of fill_pruned_offsets from [in] to
[in,out], reflecting that the function writes output offsets into the PageInfo
span. Leave the other parameter documentation unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cpp/src/io/parquet/page_hdr.cu`:
- Around line 523-561: Remove the unused page parameter from decode_page_header
and update both decode_using_page_index_fn and decode_from_page_data_fn call
sites to stop passing &pages[page_idx]. Preserve the existing manual
bs.page-to-pages[page_idx] copy-back behavior.

In `@cpp/src/io/parquet/parquet_gpu.hpp`:
- Around line 1040-1056: Update the Doxygen annotation for the pages parameter
of fill_pruned_offsets from [in] to [in,out], reflecting that the function
writes output offsets into the PageInfo span. Leave the other parameter
documentation unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1c8aaf44-8571-4402-bb5d-88977a7a14f7

📥 Commits

Reviewing files that changed from the base of the PR and between cf9226a and cebbcd5.

📒 Files selected for processing (12)

• cpp/CMakeLists.txt
• cpp/src/io/parquet/decode_pruned_pages.cu
• cpp/src/io/parquet/experimental/hybrid_scan_preprocess.cu
• cpp/src/io/parquet/page_hdr.cu
• cpp/src/io/parquet/parquet_gpu.hpp
• cpp/src/io/parquet/reader_impl.cpp
• cpp/src/io/parquet/reader_impl.hpp
• cpp/src/io/parquet/reader_impl_helpers.cpp
• cpp/src/io/parquet/reader_impl_preprocess.cu
• cpp/src/io/parquet/reader_impl_preprocess_utils.cu
• cpp/src/io/parquet/reader_impl_preprocess_utils.cuh
• cpp/tests/io/experimental/hybrid_scan_test.cpp

🚧 Files skipped from review as they are similar to previous changes (3)

• cpp/src/io/parquet/reader_impl.cpp
• cpp/src/io/parquet/reader_impl.hpp
• cpp/src/io/parquet/reader_impl_helpers.cpp

[mhaseeb123]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

🧹 Nitpick comments (2)

cpp/tests/io/experimental/hybrid_scan_test.cpp (1)

444-546: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider adding a fully-pruned (empty result) scenario.

All three pruning scenarios in this test leave multiple pages of rows surviving; none exercises the case where all pages/rows are pruned (empty filter/payload result). Per coding guidelines, tests should cover empty-input/empty-result edge cases in addition to boundary and multi-page scenarios — worth adding a validate call with an unsatisfiable filter to ensure hybrid_scan and the offset-reconstruction path handle a fully-empty result correctly.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/tests/io/experimental/hybrid_scan_test.cpp` around lines 444 - 546,
Extend the validate-based scenarios in ConsecutivePrunedPageOffsets with an
unsatisfiable filter that prunes every page and produces empty filter and
payload tables. Reuse the existing col_ref and validate helper, and provide
empty expected_slices to exercise hybrid_scan and nested offset reconstruction
for a fully empty result.

Source: Coding guidelines

cpp/src/io/parquet/reader_impl_preprocess_utils.cu (1)

443-465: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid computing d_chunk_page_info for the accelerated decode paths.

chunk_page_offsets/d_chunk_page_info are computed unconditionally, but d_chunk_page_info is only consumed by the slow COLUMN_CHUNKS fallback branch (line ~544-548); the OFFSET_INDEX and PAGE_SPANS paths never read it. Since this function is the accelerated path this PR is optimizing for hybrid-scan sparse I/O, the extra rmm::device_uvector allocation and thrust::for_each are wasted work on every call for those paths.

♻️ Proposed fix

-  rmm::device_uvector<chunk_page_info> d_chunk_page_info(pass.chunks.size(), stream);
-  thrust::for_each(rmm::exec_policy_nosync(stream, cudf::get_current_device_resource_ref()),
-                   iter,
-                   iter + pass.chunks.size(),
-                   [cpi                = d_chunk_page_info.begin(),
-                    chunk_page_offsets = chunk_page_offsets.begin(),
-                    unsorted_pages     = unsorted_pages.begin()] __device__(size_t i) {
-                     cpi[i].pages = &unsorted_pages[chunk_page_offsets[i]];
-                   });
+  rmm::device_uvector<chunk_page_info> d_chunk_page_info(0, stream);
+  if constexpr (data_source_type == page_data_source_type::COLUMN_CHUNKS) {
+    d_chunk_page_info = rmm::device_uvector<chunk_page_info>(pass.chunks.size(), stream);
+    thrust::for_each(rmm::exec_policy_nosync(stream, cudf::get_current_device_resource_ref()),
+                     iter,
+                     iter + pass.chunks.size(),
+                     [cpi                = d_chunk_page_info.begin(),
+                      chunk_page_offsets = chunk_page_offsets.begin(),
+                      unsorted_pages     = unsorted_pages.begin()] __device__(size_t i) {
+                       cpi[i].pages = &unsorted_pages[chunk_page_offsets[i]];
+                     });
+  }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/io/parquet/reader_impl_preprocess_utils.cu` around lines 443 - 465,
Move the `chunk_page_offsets` and `d_chunk_page_info` allocation and
initialization into the slow `COLUMN_CHUNKS` fallback branch, immediately before
its consumption. Keep the `OFFSET_INDEX` and `PAGE_SPANS` paths free of these
allocations and the associated `thrust::for_each`, while preserving the existing
page-offset setup for `COLUMN_CHUNKS`.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cpp/src/io/parquet/reader_impl_preprocess_utils.cu`:
- Around line 443-465: Move the `chunk_page_offsets` and `d_chunk_page_info`
allocation and initialization into the slow `COLUMN_CHUNKS` fallback branch,
immediately before its consumption. Keep the `OFFSET_INDEX` and `PAGE_SPANS`
paths free of these allocations and the associated `thrust::for_each`, while
preserving the existing page-offset setup for `COLUMN_CHUNKS`.

In `@cpp/tests/io/experimental/hybrid_scan_test.cpp`:
- Around line 444-546: Extend the validate-based scenarios in
ConsecutivePrunedPageOffsets with an unsatisfiable filter that prunes every page
and produces empty filter and payload tables. Reuse the existing col_ref and
validate helper, and provide empty expected_slices to exercise hybrid_scan and
nested offset reconstruction for a fully empty result.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 44db8c07-3531-44f7-b288-d576ae5f6a06

📥 Commits

Reviewing files that changed from the base of the PR and between 00b6b81 and cebbcd5.

📒 Files selected for processing (12)

• cpp/CMakeLists.txt
• cpp/src/io/parquet/decode_pruned_pages.cu
• cpp/src/io/parquet/experimental/hybrid_scan_preprocess.cu
• cpp/src/io/parquet/page_hdr.cu
• cpp/src/io/parquet/parquet_gpu.hpp
• cpp/src/io/parquet/reader_impl.cpp
• cpp/src/io/parquet/reader_impl.hpp
• cpp/src/io/parquet/reader_impl_helpers.cpp
• cpp/src/io/parquet/reader_impl_preprocess.cu
• cpp/src/io/parquet/reader_impl_preprocess_utils.cu
• cpp/src/io/parquet/reader_impl_preprocess_utils.cuh
• cpp/tests/io/experimental/hybrid_scan_test.cpp

[vuule]

Finally done, but not much to show for :D

[bdice]

Approving CMake.

[mhaseeb123]

/merge