Select Parquet columns by field ID by mhaseeb123 · Pull Request #22955 · NVIDIA/cudf

mhaseeb123 · 2026-06-24T00:18:55Z

Description

Contributes to #22849

This PR enables the parquet readers to select columns using parquet field ID (needed for Iceberg) where column names across table files may vary.

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 · 2026-06-24T00:18:59Z

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 · 2026-06-24T01:00:12Z

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

Adds column_field_ids as a third mutually exclusive Parquet column-selection mode. Field-ID-aware schema matching, projection mapping, hybrid-scan propagation, and test coverage were added across the reader path.

Changes

Parquet column selection by field ID

Layer / File(s)	Summary
Public API: field ID option and builder method `cpp/include/cudf/io/parquet.hpp`	Adds `_column_field_ids`, the getter and setter, exclusivity checks against names and indices, and the fluent builder method.
column_path_from_index helper `cpp/src/io/parquet/column_path_helpers.hpp`, `cpp/src/io/parquet/column_path_helpers.cpp`	Declares and implements the schema-tree traversal that builds a dot-separated column path from a schema index.
expression_transform_helpers: three-mode field-ID mapping `cpp/src/io/parquet/expression_transform_helpers.hpp`, `cpp/src/io/parquet/expression_transform_helpers.cpp`	Changes `schema_tree` to `std::span`, expands validation to names, indices, and field IDs, adds index bounds checks, and maps field IDs to column paths.
reader_impl and helpers: field-ID projection and schema matching `cpp/src/io/parquet/reader_impl.hpp`, `cpp/src/io/parquet/reader_impl.cpp`, `cpp/src/io/parquet/reader_impl_helpers.hpp`, `cpp/src/io/parquet/reader_impl_helpers.cpp`	Adds field-ID-aware projection handling, mismatched-schema detection, schema-child lookup by field ID, and field-ID-aware column matching across projected paths and additional sources.
Hybrid scan field-ID propagation `cpp/src/io/parquet/experimental/hybrid_scan_helpers.hpp`, `cpp/src/io/parquet/experimental/hybrid_scan_helpers.cpp`, `cpp/src/io/parquet/experimental/hybrid_scan_impl.cpp`	Extends hybrid scan payload selection to accept `match_schema_by_field_id`, and passes field-ID selection state through all selection paths and mismatched-schema detection.
Tests: flat, nested, filtered, and mismatched-schema coverage `cpp/tests/io/parquet_common.hpp`, `cpp/tests/io/parquet_common.cpp`, `cpp/tests/io/parquet_reader_test.cpp`	Updates Parquet test helpers to write field IDs, and expands reader tests to cover field-ID selection, nested selection, filter projection, and mismatched-schema cases.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

[STORY] Parquet reader features for Apache Iceberg #22849: This PR directly implements Parquet column selection by field ID, matching the issue objective.

Suggested labels

improvement

Suggested reviewers

qbacpey
davidwendt
bdice

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 35.71% 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 clearly summarizes the main change: selecting Parquet columns by field ID.
Description check	✅ Passed	The description matches the changeset and explains the field-ID column selection feature and test coverage.
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

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@cpp/src/io/parquet/expression_transform_helpers.cpp`:
- Around line 277-281: The validation using std::cmp_less for selected_col_idx
only checks the upper bound against root.children_idx.size() but does not
validate that the index is non-negative. A negative selected_col_idx would
bypass this check and cause undefined behavior when used to index into
root.children_idx on the following line. Add an additional validation check to
ensure selected_col_idx is greater than or equal to zero, either as a separate
CUDF_EXPECTS call or by combining it with the existing upper bound check using a
logical AND condition.

In `@cpp/src/io/parquet/reader_impl_helpers.cpp`:
- Around line 1924-1930: The field-ID based lookup is currently only applied
when col_name_info has explicit child paths, but the all-children branch that
handles non-leaf field ID selection still uses ordinal-based pairing for
descendants. Locate the branch that handles the case where a non-leaf field is
selected without explicit child paths (the all-children case), and modify it to
use field-ID lookup similar to the code shown in the diff. Specifically, replace
the ordinal-based child pairing logic with calls to
find_schema_child_for_mapping (instead of just find_schema_child) when iterating
through all children of the selected field, ensuring that nested children are
matched by field ID across sources rather than by position.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c661b3f7-43fd-4b6c-870a-17ce7cc8efd7

📥 Commits

Reviewing files that changed from the base of the PR and between d7d9751 and 5781078.

📒 Files selected for processing (14)

cpp/include/cudf/io/parquet.hpp
cpp/src/io/parquet/column_path_helpers.cpp
cpp/src/io/parquet/column_path_helpers.hpp
cpp/src/io/parquet/experimental/hybrid_scan_helpers.cpp
cpp/src/io/parquet/experimental/hybrid_scan_helpers.hpp
cpp/src/io/parquet/experimental/hybrid_scan_impl.cpp
cpp/src/io/parquet/expression_transform_helpers.cpp
cpp/src/io/parquet/expression_transform_helpers.hpp
cpp/src/io/parquet/reader_impl.cpp
cpp/src/io/parquet/reader_impl_helpers.cpp
cpp/src/io/parquet/reader_impl_helpers.hpp
cpp/tests/io/parquet_common.cpp
cpp/tests/io/parquet_common.hpp
cpp/tests/io/parquet_reader_test.cpp

coderabbitai

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@cpp/tests/io/parquet_reader_test.cpp`:
- Around line 641-645: The regression test in the parquet_reader_test.cpp file
is not exercising child field-ID selection as intended because
column_field_ids({1}) only selects the top-level record struct. To fix this,
modify the column_field_ids call in the parquet_reader_options builder to select
a child field ID (such as {2} or {3} representing child fields like x or y)
instead of the top-level field, and then add assertions that verify the
projected nested output shape is correct. This ensures the test properly catches
bugs in mismatched-schema child lookup by field ID.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3eba06e6-f115-4ed3-afb5-611e4e689556

📥 Commits

Reviewing files that changed from the base of the PR and between 5781078 and 4b99d07.

📒 Files selected for processing (3)

cpp/src/io/parquet/expression_transform_helpers.cpp
cpp/src/io/parquet/reader_impl_helpers.cpp
cpp/tests/io/parquet_reader_test.cpp

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

cpp/src/io/parquet/expression_transform_helpers.cpp
cpp/src/io/parquet/reader_impl_helpers.cpp

mhaseeb123 · 2026-06-25T05:06:08Z

-            {col_idx, normalize_column_path(schema_tree[schema_idx].name, case_sensitive_names)});
-        });
-    }
+    std::transform(


Mostly whitespace here (try hiding it in your view). The only interesting part is under else if (selected_column_field_ids.has_value()) { block.

mhaseeb123 · 2026-06-25T05:06:54Z

   */
  [[nodiscard]] auto const& get_column_indices() const { return _column_indices; }

+  /**


Pretty standard setters and getters

mhaseeb123 · 2026-06-25T05:07:10Z

                          decimal_type_id,
-                          case_sensitive_names);
+                          case_sensitive_names,
+                          match_schema_by_field_id);


Just plumbed in a new boolean field

these are mutually exclusive right? should you instead switch to an enum to reduce the number of arguments? the api is experimental after all.

these are mutually exclusive right?
Yes

should you instead switch to an enum to reduce the number of arguments? the api is experimental after all.

We are calling the base reader's select_columns API (being updated to take in another bool here).

sure but can we update that API also to reduce argument bloat everywhere

Done 3dfb09f

mhaseeb123 · 2026-06-25T05:07:49Z

  cudf::host_span<cudf::host_span<uint8_t const> const> footer_bytes,
  parquet_reader_options const& options)
 {
+  auto const has_cols_from_mismatched_srcs =


Separate out this logic: We can have mismatched columns if allowed AND selecting columns by names or field IDs

mhaseeb123 · 2026-06-25T05:07:54Z

 hybrid_scan_reader_impl::hybrid_scan_reader_impl(
  cudf::host_span<FileMetaData const> parquet_metadatas, parquet_reader_options const& options)
 {
+  auto const has_cols_from_mismatched_srcs =


mhaseeb123 · 2026-06-25T05:08:47Z


 namespace cudf::io::parquet::detail {

+std::string column_path_from_index(std::span<SchemaElement const> schema_tree, int schema_idx)


Given a ColumnChunk field ID, keep going to its parent until root and then construct a dot (.) separated string path

mhaseeb123 · 2026-06-25T05:17:16Z

  for (std::size_t i = 0; i < column_names.size(); ++i) {
    md.column_metadata[i].set_name(column_names[i]);
  }
+  for (std::size_t i = 0; i < field_ids.size(); ++i) {


Allow setting field_ids in the written parquet files in tests

mhaseeb123 · 2026-06-25T05:18:07Z

-                       find_schema_child(src_schema_elem, child_col_name_info.name),
-                       find_schema_child(dst_schema_elem, child_col_name_info.name, pfm_idx),
-                       pfm_idx);
+            auto const src_child_idx = find_schema_child(src_schema_elem, child_col_name_info.name);


For structs, make sure all children (not necessarily arranged in the same order though) have the same field ID as well.

mhaseeb123 · 2026-06-25T05:19:21Z

+               : -1;
+    };
+
+  auto const find_schema_child_for_mapping = [&](SchemaElement const& src_schema_elem,


Either calls find_schema_child (by name) or find_schema_child_by_field_id

mhaseeb123 · 2026-06-25T05:19:48Z

    CUDF_TEST_EXPECT_COLUMNS_EQUAL(result.tbl->view().column(3), c);
  }

+  {


Modify existing column selection tests here to test column selection with and without filters by field ID

vuule · 2026-07-09T22:09:23Z

-  CUDF_EXPECTS(
-    not(selected_column_names.has_value() and selected_column_indices.has_value()),
-    "Parquet reader encountered column selection by both names and indices simultaneously");
+  CUDF_EXPECTS(static_cast<int>(selected_column_names.has_value()) +


I feel like we should not be doing this. The options object has the invariant that only one of these is set.

mhaseeb123 · 2026-07-10T22:22:46Z

/merge

Follow up ##22955 This PR adds python bindings for Parquet column selection by field ID Authors: - Muhammad Haseeb (https://github.com/mhaseeb123) Approvers: - Matthew Murray (https://github.com/Matt711) URL: #22956

Select Parquet columns by field ID

08f1e36

This was referenced Jun 24, 2026

[STORY] Parquet reader features for Apache Iceberg #22849

Closed

Python bindings for Parquet select cols by field ID #22956

Merged

mhaseeb123 self-assigned this Jun 24, 2026

mhaseeb123 added feature request New feature or request 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. cuIO cuIO issue non-breaking Non-breaking change Velox Functionality that helps Velox-cudf labels Jun 24, 2026

Merge branch 'main' into fea/select-pq-cols-by-field-id

5781078

mhaseeb123 marked this pull request as ready for review June 24, 2026 00:52

mhaseeb123 requested a review from a team as a code owner June 24, 2026 00:52

mhaseeb123 requested review from pmattione-nvidia, qbacpey and vyasr June 24, 2026 00:52

coderabbitai Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread cpp/src/io/parquet/expression_transform_helpers.cpp Outdated

Comment thread cpp/src/io/parquet/reader_impl_helpers.cpp Outdated

mhaseeb123 added 2 commits June 24, 2026 01:34

Apply suggestion from coderabbit

4241c2d

Minor

4b99d07

coderabbitai Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread cpp/tests/io/parquet_reader_test.cpp

mhaseeb123 added 4 commits June 24, 2026 02:19

Style

43235a3

Minor bug fix

2d26e65

Merge branch 'main' into fea/select-pq-cols-by-field-id

3cc2e46

Merge branch 'main' into fea/select-pq-cols-by-field-id

5048a45

mhaseeb123 commented Jun 25, 2026

View reviewed changes

pmattione-nvidia reviewed Jun 25, 2026

View reviewed changes

Comment thread cpp/src/io/parquet/reader_impl.cpp Outdated

pmattione-nvidia reviewed Jun 25, 2026

View reviewed changes

Comment thread cpp/src/io/parquet/reader_impl_helpers.cpp Outdated

Merge branch 'main' into fea/select-pq-cols-by-field-id

04a2527

github-actions Bot assigned vyasr Jun 30, 2026

Merge branch 'main' into fea/select-pq-cols-by-field-id

fe2485d

mhaseeb123 force-pushed the fea/select-pq-cols-by-field-id branch from 0914161 to fe2485d Compare July 1, 2026 00:22

NVIDIA deleted a comment from copy-pr-bot Bot Jul 1, 2026

Merge branch 'main' into fea/select-pq-cols-by-field-id

545b82f

mhaseeb123 added 4 - Needs Review Waiting for reviewer to review or respond and removed 3 - Ready for Review Ready for review by team labels Jul 1, 2026

mhaseeb123 added 2 commits July 1, 2026 23:31

Address reviews

3dfb09f

Merge branch 'main' into fea/select-pq-cols-by-field-id

8417b0e

pmattione-nvidia approved these changes Jul 2, 2026

View reviewed changes

mhaseeb123 added 3 commits July 2, 2026 18:38

style

8073240

Minor

f920a00

Merge branch 'main' into fea/select-pq-cols-by-field-id

12cf311

mhaseeb123 requested a review from vuule July 9, 2026 21:11

vuule requested changes Jul 9, 2026

View reviewed changes

mhaseeb123 added 3 commits July 9, 2026 15:51

Merge branch 'main' into fea/select-pq-cols-by-field-id

1335b4a

Address comments

e66174a

Merge branch 'main' into fea/select-pq-cols-by-field-id

a18925b

mhaseeb123 requested a review from vuule July 10, 2026 18:33

vuule approved these changes Jul 10, 2026

View reviewed changes

Merge branch 'main' into fea/select-pq-cols-by-field-id

86a4b48

mhaseeb123 added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 4 - Needs Review Waiting for reviewer to review or respond labels Jul 10, 2026

rapids-bot Bot merged commit 3aa6568 into NVIDIA:main Jul 11, 2026
261 of 263 checks passed

mhaseeb123 deleted the fea/select-pq-cols-by-field-id branch July 13, 2026 16:05

This was referenced Jul 23, 2026

Prefetch byte ranges using kvikio #23317

Draft

Improve Parquet reader pass construction #23446

Merged


		namespace cudf::io::parquet::detail {

		std::string column_path_from_index(std::span<SchemaElement const> schema_tree, int schema_idx)

Uh oh!

Conversation

mhaseeb123 commented Jun 24, 2026

Description

Checklist

Uh oh!

copy-pr-bot Bot commented Jun 24, 2026

Uh oh!

coderabbitai Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Suggested labels

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mhaseeb123 commented Jul 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coderabbitai Bot commented Jun 24, 2026 •

edited

Loading