Fix potential malformed headers in parquet delta decoder by mhaseeb123 · Pull Request #22275 · NVIDIA/cudf

mhaseeb123 · 2026-04-23T20:31:32Z

Description

This PR checks for potential malformed headers in Parquet delta decoder and propagates it in decoders.

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-04-23T20:31:36Z

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.

wence- · 2026-04-29T10:04:44Z

+    uint32_t const prev_src_pos      = s->src_pos;
+
    uint32_t target_pos;
    uint32_t const src_pos = s->src_pos;


src_pos and prev_src_pos play identical roles.

mhaseeb123 · 2026-05-14T21:38:08Z

  if (batch_size > max_delta_mini_block_size) {
-    set_error(static_cast<kernel_error::value_type>(decode_error::DELTA_PARAMS_UNSUPPORTED),
-              error_code);
+    if (block.thread_rank() == 0) {


Only one thread needs to set the error

coderabbitai · 2026-05-14T21:43:29Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e61079d5-4b07-4ace-87d4-e563b75ae7fb

📥 Commits

Reviewing files that changed from the base of the PR and between a50d9de and c839f1b.

📒 Files selected for processing (2)

cpp/src/io/parquet/delta_binary.cuh
cpp/src/io/parquet/page_delta_decode.cu

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

cpp/src/io/parquet/delta_binary.cuh
cpp/src/io/parquet/page_delta_decode.cu

📝 Walkthrough

Summary by CodeRabbit

Bug Fixes
- Strengthened validation of DELTA_BINARY_PACKED data to detect invalid headers earlier
- Tightened delta-decoding paths to fail fast on detected header or size errors, avoiding wasted work
- Improved error reporting and guarded error reporting to ensure clearer, single-point failure notifications

Walkthrough

Adds an error flag to the Parquet delta decoder and validates DELTA_BINARY_PACKED header invariants in init_binary_block. On malformed headers the decoder is moved to an inert state. Three CUDA decoding kernels now check this flag and propagate a DELTA_PARAMS_UNSUPPORTED error from thread 0 when present.

Changes

Delta decoder error handling

Layer / File(s)	Summary
Decoder error state and header validation `cpp/src/io/parquet/delta_binary.cuh`	Adds `bool error` to `delta_binary_decoder`. `init_binary_block` initializes `error = false`, validates `mini_block_count` and `block_size` (nonzero and divisible), and on failure sets `error = true`, zeroes/neutralizes decoding counters (`value_count`, `values_per_mb = 1`) and advances internal pointers to `d_end`, then returns. On success computes `values_per_mb = block_size / mini_block_count`.
Delta binary kernel: sanity and error propagation `cpp/src/io/parquet/page_delta_decode.cu` (`decode_delta_binary_kernel`)	After decoder init, treats `db->error` as unsupported params alongside `batch_size > max_delta_mini_block_size`; sets `DELTA_PARAMS_UNSUPPORTED` via `set_error` only when `block.thread_rank() == 0` and returns early.
Delta byte-array kernel: prefix/suffix decoder checks `cpp/src/io/parquet/page_delta_decode.cu` (`decode_delta_byte_array_kernel`)	Adds early return if `prefix_db->error` or `suffix_db->error`, setting `DELTA_PARAMS_UNSUPPORTED` only from `block.thread_rank() == 0`. Also guards the existing `batch_size > max_delta_mini_block_size` error path so `set_error` runs only on thread 0.
Delta length byte-array kernel: extended sanity check `cpp/src/io/parquet/page_delta_decode.cu` (`decode_delta_length_byte_array_kernel`)	Extends initial sanity check to include `db->error` with `batch_size` check; sets `DELTA_PARAMS_UNSUPPORTED` only for `block.thread_rank() == 0` before returning. Moves `leaf_level_index` declaration to after this check.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

wence-
vuule
pmattione-nvidia

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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 PR title accurately describes the main change: adding header validation checks to the parquet delta decoder to handle malformed headers.
Description check	✅ Passed	The PR description is directly related to the changeset, explaining that it checks for malformed headers in the Parquet delta decoder and propagates errors through decoders.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

coderabbitai

🧹 Nitpick comments (1)

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

775-781: 💤 Low value

Consider using kernel_error::value_type for consistency with other kernels.

The other delta decode kernels use static_cast<kernel_error::value_type>(...) (e.g., lines 371, 554, 577), while this one uses static_cast<int32_t>(...). The code works correctly, but using the typedef ensures consistency and avoids issues if the type changes.

Suggested fix

  if (db->error) {
    if (block.thread_rank() == 0) {
-      set_error(static_cast<int32_t>(decode_error::DELTA_PARAMS_UNSUPPORTED), error_code);
+      set_error(static_cast<kernel_error::value_type>(decode_error::DELTA_PARAMS_UNSUPPORTED), error_code);
    }
    return;
  }

🤖 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_delta_decode.cu` around lines 775 - 781, Replace the
inconsistent cast in the error path: when calling set_error(.., error_code) in
the block where db->error is checked, change the
static_cast<int32_t>(decode_error::DELTA_PARAMS_UNSUPPORTED) to
static_cast<kernel_error::value_type>(decode_error::DELTA_PARAMS_UNSUPPORTED) so
it matches other delta decode kernels (see other uses of
kernel_error::value_type) and keeps type consistency for set_error, using the
same symbols db->error, block.thread_rank(), set_error,
decode_error::DELTA_PARAMS_UNSUPPORTED, and error_code.

🤖 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_delta_decode.cu`:
- Around line 775-781: Replace the inconsistent cast in the error path: when
calling set_error(.., error_code) in the block where db->error is checked,
change the static_cast<int32_t>(decode_error::DELTA_PARAMS_UNSUPPORTED) to
static_cast<kernel_error::value_type>(decode_error::DELTA_PARAMS_UNSUPPORTED) so
it matches other delta decode kernels (see other uses of
kernel_error::value_type) and keeps type consistency for set_error, using the
same symbols db->error, block.thread_rank(), set_error,
decode_error::DELTA_PARAMS_UNSUPPORTED, and error_code.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8aa79831-7a08-4539-a7ed-f2c3779e2ab2

📥 Commits

Reviewing files that changed from the base of the PR and between d7c7891 and 45e7d15.

📒 Files selected for processing (2)

cpp/src/io/parquet/delta_binary.cuh
cpp/src/io/parquet/page_delta_decode.cu

mhaseeb123 · 2026-05-14T21:39:20Z

-    values_per_mb     = block_size / mini_block_count;
+    error             = false;
+
+    // Validate header against the DELTA_BINARY_PACKED spec invariants


Validate header here and set error field.

mhaseeb123 · 2026-05-14T21:39:42Z

-  if (batch_size > max_delta_mini_block_size) {
-    set_error(static_cast<kernel_error::value_type>(decode_error::DELTA_PARAMS_UNSUPPORTED),
-              error_code);
+  if (db->error or batch_size > max_delta_mini_block_size) {


Check for db->error field, propagate it and exit early.

mhaseeb123 · 2026-05-14T21:46:57Z

  block.sync();

+  // Propagate malformed-header errors from either underlying DELTA_BINARY_PACKED decoder.
+  if (prefix_db->error or suffix_db->error) {


Same everywhere

mhaseeb123 · 2026-05-14T21:47:27Z

@coderabbitai full review

coderabbitai · 2026-05-14T21:47:31Z

✅ Actions performed

Full review triggered.

Co-authored-by: Muhammad Haseeb <14217455+mhaseeb123@users.noreply.github.com>

copy-pr-bot · 2026-05-15T21:25:29Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

mhaseeb123 · 2026-05-15T21:26:15Z

/ok to test c839f1b

mhaseeb123 · 2026-05-15T21:26:45Z

/merge

ttnghia

I would expect this increase register usage a little bit.

This PR checks for potential malformed headers in Parquet delta decoder and propagates it in decoders. Authors: - Muhammad Haseeb (https://github.com/mhaseeb123) Approvers: - Paul Mattione (https://github.com/pmattione-nvidia) - Bradley Dice (https://github.com/bdice) - Nghia Truong (https://github.com/ttnghia) URL: NVIDIA#22275

mhaseeb123 changed the title ~~Check potential malformed headers in parquet delta decoder~~ 🚧 Check potential malformed headers in parquet delta decoder Apr 23, 2026

github-actions Bot assigned mhaseeb123 Apr 23, 2026

github-actions Bot added the libcudf Affects libcudf (C++/CUDA) code. label Apr 23, 2026

mhaseeb123 changed the title ~~🚧 Check potential malformed headers in parquet delta decoder~~ 🚧 Fix potential malformed headers in parquet delta decoder Apr 23, 2026

mhaseeb123 changed the title ~~🚧 Fix potential malformed headers in parquet delta decoder~~ Fix potential malformed headers in parquet delta decoder Apr 24, 2026

mhaseeb123 commented Apr 24, 2026

View reviewed changes

Comment thread cpp/src/io/parquet/delta_binary.cuh Outdated

mhaseeb123 commented Apr 24, 2026

View reviewed changes

Comment thread cpp/src/io/parquet/page_delta_decode.cu Outdated

mhaseeb123 commented Apr 24, 2026

View reviewed changes

Comment thread cpp/src/io/parquet/delta_binary.cuh Outdated

mhaseeb123 marked this pull request as ready for review April 24, 2026 20:02

mhaseeb123 requested a review from a team as a code owner April 24, 2026 20:02

mhaseeb123 requested review from pmattione-nvidia and wence- April 24, 2026 20:02

mhaseeb123 added bug Something isn't working 3 - Ready for Review Ready for review by team cuIO cuIO issue non-breaking Non-breaking change labels Apr 24, 2026

pmattione-nvidia reviewed Apr 24, 2026

View reviewed changes

Comment thread cpp/src/io/parquet/page_delta_decode.cu Outdated

wence- reviewed Apr 29, 2026

View reviewed changes

mhaseeb123 marked this pull request as draft April 29, 2026 21:21

GregoryKimball added this to libcudf May 4, 2026

GregoryKimball moved this to Burndown in libcudf May 4, 2026

mhaseeb123 commented May 14, 2026

View reviewed changes

mhaseeb123 marked this pull request as ready for review May 14, 2026 21:38

mhaseeb123 requested review from pmattione-nvidia and wence- May 14, 2026 21:38

coderabbitai Bot reviewed May 14, 2026

View reviewed changes

mhaseeb123 commented May 14, 2026

View reviewed changes

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

pmattione-nvidia approved these changes May 14, 2026

View reviewed changes

bdice approved these changes May 15, 2026

View reviewed changes

mhaseeb123 and others added 6 commits May 15, 2026 21:24

Check malformed block headers in parquet delta decoder

6a32d88

Apply suggestion from @mhaseeb123

23aade2

Apply suggestions from code review

1a52a6d

Co-authored-by: Muhammad Haseeb <14217455+mhaseeb123@users.noreply.github.com>

Cleanup

66df3a3

simplify check

95f3d8a

Fix nit

c839f1b

mhaseeb123 force-pushed the fix/potential-errors-part-02 branch from c2a5a33 to c839f1b Compare May 15, 2026 21:25

mhaseeb123 changed the base branch from main to release/26.06 May 15, 2026 21:25

ttnghia approved these changes May 15, 2026

View reviewed changes

rapids-bot Bot merged commit 3ae4319 into NVIDIA:release/26.06 May 15, 2026
119 checks passed

mhaseeb123 deleted the fix/potential-errors-part-02 branch May 15, 2026 22:32

mhaseeb123 moved this from Burndown to Landed in libcudf May 15, 2026

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 May 19, 2026

GregoryKimball removed this from libcudf Jun 2, 2026

Uh oh!

Conversation

mhaseeb123 commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

copy-pr-bot Bot commented Apr 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wence- Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mhaseeb123 May 14, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mhaseeb123 May 14, 2026

Choose a reason for hiding this comment

Uh oh!

mhaseeb123 May 14, 2026

Choose a reason for hiding this comment

Uh oh!

mhaseeb123 May 14, 2026

Choose a reason for hiding this comment

Uh oh!

mhaseeb123 commented May 14, 2026

Uh oh!

coderabbitai Bot commented May 14, 2026

Uh oh!

copy-pr-bot Bot commented May 15, 2026

Uh oh!

mhaseeb123 commented May 15, 2026

Uh oh!

mhaseeb123 commented May 15, 2026

Uh oh!

ttnghia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mhaseeb123 commented Apr 23, 2026 •

edited

Loading

coderabbitai Bot commented May 14, 2026 •

edited

Loading