Skip to content

Forward-merge release/26.08 into main - #23401

Merged
gforsyth merged 17 commits into
NVIDIA:mainfrom
Matt711:main-merge-release/26.08
Jul 27, 2026
Merged

Forward-merge release/26.08 into main#23401
gforsyth merged 17 commits into
NVIDIA:mainfrom
Matt711:main-merge-release/26.08

Conversation

@Matt711

@Matt711 Matt711 commented Jul 22, 2026

Copy link
Copy Markdown
Member

Description

Handle forward merge conflict.

Supercedes #23390

Checklist

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

abigalekim and others added 4 commits July 22, 2026 12:09
Closes NVIDIA#23348

Fixes build/CI by linking RMM to cuDF testing libraries.

Authors:
  - Abigale Kim (https://github.com/abigalekim)

Approvers:
  - Kyle Edwards (https://github.com/KyleFromNVIDIA)
  - Muhammad Haseeb (https://github.com/mhaseeb123)
  - Bradley Dice (https://github.com/bdice)

URL: NVIDIA#23341
Closes NVIDIA#23178.

Fixes a lifetime hazard in Parquet statistics filtering where `stats_caster_base::host_column::to_device` could return while asynchronous copies were still reading buffers owned by the temporary host column. This keeps the host-owned value and null-mask buffers alive until the stream has consumed them, covering both fixed-width and string statistics.

Authors:
  - Aryan Srivastava (https://github.com/aryansri05)
  - https://github.com/nursexams-lgtm
  - Muhammad Haseeb (https://github.com/mhaseeb123)

Approvers:
  - Vukasin Milovanovic (https://github.com/vuule)
  - Muhammad Haseeb (https://github.com/mhaseeb123)
  - Basit Ayantunde (https://github.com/lamarrr)

URL: NVIDIA#23219
We've observed some cudf-polars runs taking >1 hour to complete, while runs on different, but similar, hardware take <15 minutes.

We've resolved most of the outright hangs now. Under the assumption that it's preferable to wait for a job to complete rather than restarting, we'll bump the timeout here.

Authors:
  - Tom Augspurger (https://github.com/TomAugspurger)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: NVIDIA#23399
The `ci-imgs` change in rapidsai/ci-imgs#408 switched `citestwheel` images from CUDA `devel` to `base`, which no longer bundles `libcufile.so` with the container. Now we must install cuFile wheels in Python test environments that use CUDA wheels.

CUDA 12 uses `nvidia-cufile-cu12` with some caveats in the pinnings, because cuFile wheels weren't available until 12.6.3 so the `[cufile]` extra isn't reliable.

CUDA 13 uses the `cuda-toolkit[cufile]` extra.

Authors:
  - Bradley Dice (https://github.com/bdice)
  - Gil Forsyth (https://github.com/gforsyth)

Approvers:
  - Gil Forsyth (https://github.com/gforsyth)

URL: NVIDIA#23392
@Matt711
Matt711 requested review from a team as code owners July 22, 2026 17:16
@Matt711 Matt711 added the improvement Improvement / enhancement to an existing function label Jul 22, 2026
@Matt711
Matt711 requested review from a team as code owners July 22, 2026 17:16
@Matt711
Matt711 requested a review from bdice July 22, 2026 17:16
@Matt711 Matt711 added the non-breaking Non-breaking change label Jul 22, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jul 22, 2026

Copy link
Copy Markdown

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.

@Matt711
Matt711 requested a review from mattgara July 22, 2026 17:16
@github-actions github-actions Bot added libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue labels Jul 22, 2026
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved Parquet deletion-vector handling to support both deletion and retention modes.
    • Enhanced pandas-compat indexing/reshape behavior, including more reliable handling of NaN keys and missing-driven dtype promotion in pivot/unstack.
  • New Features
    • Expanded experimental VARIANT field path parsing to better support bracketed array indexing.
    • Improved streaming join filter pushdown decisions with cost-aware domain scoring.
  • Tests
    • Increased test time limits and adjusted test verbosity to reduce flaky runs.
  • Build and Reliability
    • Updated GPU test/runtime wiring for broader CUDA-wheel coverage.

Walkthrough

The PR updates CI execution, CUDA dependencies, bloom-filter sizing, Parquet and ORC behavior, Java artifact packaging, pandas-compatible cuDF reshaping, and cudf-polars join-filter pushdown.

Changes

Runtime and CI compatibility

Layer / File(s) Summary
CI execution updates
ci/run_cudf_polars_*.sh, ci/test_*cudf_polars.sh
Python-enabled test timeouts increase to 5400 seconds and pytest verbosity/fail-fast arguments change.
CUDA dependencies and Parquet support
dependencies.yaml, cpp/CMakeLists.txt, cpp/src/io/parquet/stats_filter_helpers.hpp
CUDA cuFile requirements, RMM test linkage, and Parquet null-mask construction are updated.

Bloom-filter sizing

Layer / File(s) Summary
Byte-based filter sizing
cpp/libcudf_streaming/..., python/cudf_streaming/..., cpp/libcudf_streaming/benchmarks/...
Bloom filters now use aligned byte storage sizes with validation and updated bindings, callers, and tests.

Parquet, ORC, and Java

Layer / File(s) Summary
Retention vectors and VARIANT arrays
cpp/include/cudf/io/experimental/..., cpp/src/io/parquet/experimental/..., java/src/..., cpp/tests/io/...
Retention-vector metadata propagates through native and Java APIs; VARIANT extraction supports numeric array paths.
ORC statistics and Maven artifacts
cpp/src/io/orc/..., java/ci/...
ORC row statistics accumulate using uint64_t, while Java builds and repository assembly validate sources and Javadoc artifacts.

cuDF pandas compatibility

Layer / File(s) Summary
Index metadata and reshaping
python/cudf/cudf/core/{column_accessor,dataframe,indexed_frame,multiindex,reshape}.py
NaN labels, MultiIndex metadata, stack/unstack ordering, and missing-value integer promotion are updated and tested.

cudf-polars planning

Layer / File(s) Summary
Cost-aware join-filter pushdown
python/cudf_polars/cudf_polars/{dsl,streaming,utils}/..., python/cudf_polars/tests/...
Pushdown candidate selection uses source costs, node reference counts, profitability gates, and updated defaults.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Suggested labels: 5 - Ready to Merge, ci

Suggested reviewers: vyasr, bdice, mattgara, tomaugspurger, abellina

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 34.55% 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 states this is a forward merge from release/26.08 into main, matching the change set.
Description check ✅ Passed The description is related to the merge and conflict-resolution work in this pull request.
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.

paul-aiyedun and others added 11 commits July 22, 2026 17:33
…IDIA#23389)

Align the `java-build` and `java-gather` artifact set with what Maven Central requires for a release (main classifier JAR, sources.jar, javadoc.jar, POM).

Authors:
  - https://github.com/paul-aiyedun

Approvers:
  - Tim Liu (https://github.com/NvTimLiu)

URL: NVIDIA#23389
NVIDIA#23365)

Split out of NVIDIA#23255 (2/6).

Rebuilding a frame's pandas columns `MultiIndex` from tuples re-sorts the levels and re-infers their dtypes, losing the exact source layout: unsorted explicit level orders (which change the behavior of pandas operations that work on level codes, e.g. legacy `stack(sort=True)` after a fast-to-slow conversion under `cudf.pandas`), categorical/object/int64 level dtypes (int64 levels with missing entries upcast to float64), and NaN column labels (fresh `float('nan')` objects hash unequal, so lookups miss).

- Prime the cached `to_pandas_index` with the exact source `pd.MultiIndex` at `DataFrame` construction and propagate it through accessor copies.
- Restore recorded per-level dtypes in `to_pandas_index` when the cast round-trips losslessly.
- Match NaN-containing column labels under pandas' all-NaNs-equal semantics.
- Read level dtypes off `MultiIndex.levels` (`get_level_values` materializes missing entries as NaN and upcasts), also for `cudf.MultiIndex` columns.
- Keep hierarchical columns through DataFrame binops when only level dtypes differ (restored `Int8` vs `int64` fails `Index.equals`).

Fixes 13 pandas-tests (constructor dict-NaN-key, concat keys with specific levels, groupby ordered multi-func aggregate, MultiIndex loc, and several `test_stack_unstack.py` cases); their xfail entries are removed. Attribution verified by running each node id against an isolated build containing only this change (pass) and a clean build (fail).

Independent of the other NVIDIA#23255 split PRs, but the stack (3/6) and unstack (4/6) PRs depend on this one.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: NVIDIA#23365
This PR enables the experimental parquet reader to now also support retention vectors in addition to deletion vectors

Authors:
  - Muhammad Haseeb (https://github.com/mhaseeb123)

Approvers:
  - Vukasin Milovanovic (https://github.com/vuule)
  - Jihoon Son (https://github.com/jihoonson)

URL: NVIDIA#23402
…A#22895)

## Summary

Contributes to NVIDIA#22312

Extends the JSONPath-like path used by `get_variant_field`/`extract_variant_field` to descend into array values via zero-based `[N]` index steps, in addition to the existing object-key descent. This enables extraction of array elements from Parquet VARIANT columns (e.g. `$.a[0]`, `$[0].field`).

Leading zeroes are allowed, whitespace characters are not.

Authors:
  - Vukasin Milovanovic (https://github.com/vuule)

Approvers:
  - Yunsong Wang (https://github.com/PointKernel)
  - Simon Eves (https://github.com/simoneves)
  - Abigale Kim (https://github.com/abigalekim)
  - Nghia Truong (https://github.com/ttnghia)

URL: NVIDIA#22895
Fix ORC root column statistics when an `orc_chunked_writer` receives multiple non-empty tables.

`persisted_statistics::persist` replaced `num_rows` on every write, so the root `numberOfValues` stored in the footer reflected only the final table. Accumulate the row count across writes instead, while retaining the existing early return for an empty current table.

This caused Spark ORC aggregate pushdown to return the number of output files instead of the row count for `COUNT` on a top-level struct written by the RAPIDS Accelerator. Related issue: NVIDIA/cudf-spark#15186.

Validation:

- Before the fix, `OrcChunkedWriterTest.RootStatisticsAccumulateRows` failed with root `number_of_values = 1`, expected `6`.
- After the fix, the targeted test passed.
- Full `ORC_TEST`: `197 tests from 36 test suites`, `197 passed`, `4 disabled`.

Authors:
  - Allen Xu (https://github.com/wjxiz1992)

Approvers:
  - Chong Gao (https://github.com/res-life)
  - Vukasin Milovanovic (https://github.com/vuule)
  - Muhammad Haseeb (https://github.com/mhaseeb123)

URL: NVIDIA#23118
These two nightly jobs were scheduled on the same (slow) runner and made it to ~95% before being timed out:

- https://github.com/rapidsai/cudf/actions/runs/29991014913/job/89153660965
- https://github.com/rapidsai/cudf/actions/runs/29991014913/job/89153660991

I've bumped the timeout slightly more.

Authors:
  - Tom Augspurger (https://github.com/TomAugspurger)

Approvers:
  - Lawrence Mitchell (https://github.com/wence-)
  - James Lamb (https://github.com/jameslamb)

URL: NVIDIA#23410
NVIDIA#23370)

Split out of NVIDIA#23255 (3/6), superseding it. **Depends on NVIDIA#23365 (MultiIndex column fidelity)** — 14 of the 28 un-xfailed pandas-tests need both fixes, so this PR's pandas-tests job goes green once NVIDIA#23365 merges.

- Resolve `level` positionally: integer column-level *names* no longer collide with level *positions* (pandas' `Index.get_level_values` resolves integers by name first, so frames with integer level names returned data from the wrong level).
- Validate out-of-bounds integer levels (`IndexError`) and duplicated level names (`ValueError`) with pandas' messages; negative out-of-bounds levels previously wrapped around silently.
- Build the stacked level keys from the column MultiIndex's own levels/codes so per-level dtypes survive: int64 levels with missing entries no longer upcast to float64, and categorical levels stay categorical through the pylibcudf `tile` step (which only sees codes).
- Emit stacked keys in appearance order, matching pandas. This replaces the argsort-based reordering, which misaligned column data for non-involution column permutations (e.g. a 3-cycle) and NaN keys; pandas legacy stack sorts multi-level keys by level *codes*, not values.
- Attach pandas-faithful levels/codes to the result index eagerly (the original index contributes its own levels/codes; flat indexes and the tiled level get appearance-order factorization) so a later `unstack` restores the original row/column order; the legacy `dropna` path preserves them by masking codes instead of gathering the index.

Fixes 28 pandas-tests; their xfail entries are removed. Three classic categorical unstack params are un-xfailed (fixed by this change together with NVIDIA#23365). Attribution verified per node id against isolated builds: 14 pass with only this change, 14 need this plus NVIDIA#23365.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Matthew Murray (https://github.com/Matt711)

URL: NVIDIA#23370
Rather than asking for a fitting number of blocks for an size, just require
the user to provide the size of the filter directly. Since it must be a
multiple of the storage type, also offer an aligned_size utility that takes
an input and makes that happen.

Additionally, share the new cuco parametric filter policy across cudf and
cudf-streaming.

Authors:
  - Lawrence Mitchell (https://github.com/wence-)
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Tom Augspurger (https://github.com/TomAugspurger)

Approvers:
  - Tom Augspurger (https://github.com/TomAugspurger)
  - Muhammad Haseeb (https://github.com/mhaseeb123)
  - Yunsong Wang (https://github.com/PointKernel)

URL: NVIDIA#23067
This changes the cudf-polars default configuration to disable the join filter pushdown rewrite added in
NVIDIA#22996. It can be re-enabled through an env var with CUDF_POLARS__EXECUTOR__JOIN_FILTER_PUSHDOWN=1, or through python by passing
`join_filter_pushdown=JoinFilterPushdownOptions()`

I'm proposing to disable this for the 26.08 release based on some slowdowns in our pds-h benchmarks at smaller scales (SF-1K on a single H100 GPU).

<img width="1694" height="391" alt="image" src="https://github.com/user-attachments/assets/67766d4c-6d35-4afc-9751-29a98344943b" />

Eventually, I think that the join filter pushdown optimization should be on by default. But I'd like to understand a bit more about the slowdowns observed in that benchmark before turning it on by default.

Authors:
  - Tom Augspurger (https://github.com/TomAugspurger)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: NVIDIA#23407
…IA#23368)

Split out of NVIDIA#23255 (4/6). **Depends on NVIDIA#23365 (fidelity), the stack PR (NVIDIA#23370), and NVIDIA#23366 (groupby agg)** — 8 of the 29 un-xfailed pandas-tests need those fixes too, so this PR's pandas-tests job goes green once they merge.

- Order result rows/columns by the removed level's codes (level order preserved, missing keys first) instead of sorted values with nulls last, by encoding the integer code columns instead of the level values.
- Propagate the source frame's column-axis level names into the result instead of hardcoding `None`; also fixes the `ValueError: Length of names must match number of levels` crash when unstacking MultiIndex-column frames.
- Promote integer source columns to float64 when the reshape introduces missing cells (pandas' block semantics), gated on `mode.pandas_compatible`; `pivot_table`/`crosstab` opt out via a module-private `_unstack` parameter when `fill_value` fills the cells afterwards.
- Preserve unused categories of the removed level in the result's column levels (pandas GH 17845); also fixes a libcudf `Column sizes don't match` crash for indexes with unused categorical categories.
- Validate the level on flat-index frames (`KeyError`) and duplicated index names (`ValueError`) like pandas; `pivot` with `values=` drops the original columns-axis names.

Fixes 29 pandas-tests; their xfail entries are removed, three remaining `test_stack_unstack.py` entries get real failure reasons, and two classic categorical unstack params are un-xfailed. Attribution verified per node id against isolated builds: 21 pass with only this change, 4 need the stack PR, 2 need stack+fidelity, 2 need the groupby-agg PR.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Matthew Roeschke (https://github.com/mroeschke)

URL: NVIDIA#23368
Teach the join filter pushdown planner to estimate the source-scan
cost needed to build candidate domains. Candidate selection now prefers
cheaper domain producers before smaller estimated output rows, and rejects
pushdown when the domain-build cost is too large relative to the target
being reduced.

Also prevent a single-source domain from being stacked onto a target source
that is already below the filtered side of a semi join. This heuristic does
a better job of keeping profitable domain choices while avoiding cases
where pushing down a filter would add an additional high-cardinality source
onto a join target that is already significantly filtered.

Additionally, extends trace metadata with estimated target, domain, and
constraint costs.

Material results of this change running NDSH SF30K on 8xNVL4 nodes are (previous results come from the change in NVIDIA#22996):
* **Q9 improved runtime: 11.77s lukewarm, 7.45s hot** (previously 47.12s lukewarm, 32.68s hot)
* Q5 unchaned: 9.55s lukewarm, 5.31s hot (previously 9.35s lukewarm, 5.14s hot)
* **NDSH SF30K for all 22 queries: 118.87s lukewarm, 111.63s hot**

Authors:
  - Peter Andreas Entschev (https://github.com/pentschev)
  - Lawrence Mitchell (https://github.com/wence-)

Approvers:
  - Gil Forsyth (https://github.com/gforsyth)
  - Tom Augspurger (https://github.com/TomAugspurger)

URL: NVIDIA#22997
@Matt711

Matt711 commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

/ok to test 03a2919

@Matt711

Matt711 commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

/merge nosquash

@rapids-bot

rapids-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Commit history integrity check failed: not all commits from original PR #23390 appear to be present individually in this PR's history. This usually happens if commits were squashed during the manual resolution process. Please ensure all original commits are preserved individually. You can fix this and try the /merge nosquash command again.

jihoonson and others added 2 commits July 27, 2026 09:31
…DIA#23423)

First of all, I apologize for making a PR targeting the 26.08 branch. I
understand the code freeze has taken effect since yesterday, but wish
this change can still be shipped in the 26.08 release.

## Description

This PR adds the JNI bridge for the recent change in
NVIDIA#23402 to support the retention
filter for the deletion vector. Previously, it was assumed that deletion
vectors represent only the rows deleted. With
NVIDIA#23402 and this change, now Java
applications can pass the correct filter type for deletion vectors.

## Checklist
- [x] I am familiar with the [Contributing
Guidelines](https://github.com/rapidsai/cudf/blob/HEAD/CONTRIBUTING.md).
- [x] New or existing tests cover these changes.
- [x] The documentation is up to date with these changes.
@bdice
bdice force-pushed the main-merge-release/26.08 branch from 03a2919 to a596c3e Compare July 27, 2026 14:00
@bdice
bdice requested review from a team as code owners July 27, 2026 14:00
@bdice
bdice requested a review from vyasr July 27, 2026 14:00
@github-actions github-actions Bot added Python Affects Python cuDF API. Java Affects Java cuDF API. cudf.pandas Issues specific to cudf.pandas cudf-polars Issues specific to cudf-polars labels Jul 27, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python Jul 27, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (7)
cpp/tests/io/parquet_deletion_vectors_test.cpp (1)

605-638: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extend retention-mode coverage to CustomRowIndex and MultipleDeletionVectors count tests.

NoRowIndex (lines 616-638) now loops over both are_retention_vectors values, but CustomRowIndex (lines 686-705) and MultipleDeletionVectors (line 739 onward) still only exercise deletion mode. Consider mirroring the NoRowIndex loop pattern in these two tests for consistent coverage of compute_num_deleted_rows under retention mode with custom row indices and multiple bitmaps.

Also applies to: 641-705, 708-768

🤖 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/parquet_deletion_vectors_test.cpp` around lines 605 - 638,
Extend the retention-mode loop used in DeletionVectorsCountTests.NoRowIndex to
the CustomRowIndex and MultipleDeletionVectors tests. For each
are_retention_vectors value, build the corresponding expected mask and
deletion-vector metadata, then assert compute_num_deleted_rows for the existing
chunk sizes against the matching expected count while preserving their
custom-index and multiple-bitmap scenarios.
cpp/include/cudf/io/experimental/deletion_vectors.hpp (1)

33-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Doc comments not updated for the new retention-vector mode.

The struct-level comment (lines 34-36), the new field's one-line doc, the chunked_parquet_reader class doc (lines 56-68, 131-143), and compute_num_deleted_rows's doc (lines 187-195) all still describe only "deletion" semantics. Since are_retention_vectors inverts the meaning of the roaring bitmap (retain vs. delete) and inverts the returned count in compute_num_deleted_rows, it would help API consumers to spell out the retention-mode behavior in these Doxygen comments.

🤖 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/include/cudf/io/experimental/deletion_vectors.hpp` around lines 33 - 54,
Update the Doxygen comments for deletion_vector_info, are_retention_vectors,
chunked_parquet_reader, and compute_num_deleted_rows to document both
deletion-vector and retention-vector modes. Explain that are_retention_vectors
makes bitmap entries identify rows to retain rather than delete, and that
compute_num_deleted_rows returns the corresponding inverted deleted-row count in
retention mode.
python/cudf_polars/cudf_polars/streaming/actor_graph/join.py (1)

809-811: 🚀 Performance & Scalability | 🔵 Trivial

TODO: hardcoded 32 MiB filter size.

filter_size is now a fixed constant rather than being derived from expected row counts (fitting_num_blocks), which could over/under-size the filter for skewed build-side cardinalities. The TODO acknowledges this.

Want me to help wire this up to BloomFilter.aligned_size(...) and an actual build-side row estimate once one is available in this actor graph?

🤖 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 `@python/cudf_polars/cudf_polars/streaming/actor_graph/join.py` around lines
809 - 811, Update the BloomFilter construction in the join actor graph to derive
filter_size from the expected build-side row count, such as fitting_num_blocks,
using BloomFilter.aligned_size(...), instead of the hardcoded 32 MiB constant.
Preserve the existing context, communicator, and hash-seed arguments while
sizing the filter for the estimated cardinality.
cpp/libcudf_streaming/tests/streaming/test_bloom_filter.cu (1)

30-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use CUDF_KERNEL instead of raw __global__.

As per coding guidelines, cpp/**/*.{cpp,cu} code should "Use CUDF_KERNEL rather than raw __global__, preferably with __launch_bounds__."

🔧 Proposed fix
-__global__ void block_index_kernel(std::uint32_t upper_hash,
-                                   std::size_t num_blocks,
-                                   std::uint32_t* result)
+CUDF_KERNEL void block_index_kernel(std::uint32_t upper_hash,
+                                    std::size_t num_blocks,
+                                    std::uint32_t* result)
🤖 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/libcudf_streaming/tests/streaming/test_bloom_filter.cu` around lines 30 -
35, Replace the raw __global__ qualifier on block_index_kernel with the
project-standard CUDF_KERNEL macro, preserving the kernel signature and body.

Source: Coding guidelines

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

43-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Doc comment still describes the old cuco::parametric_filter_policy.

The comment says "Uses cuco's parametric_filter_policy..." but line 56 now aliases cudf::arrow_filter_policy. Worth updating the comment to reflect the actual policy in use (and why, given the new test validating it supports more blocks than the former Arrow limit).

📝 Proposed doc update
- * Uses cuco's `parametric_filter_policy` with the Apache Arrow layout: 256-bit blocks (8 x
- * `uint32_t`), 8 fingerprint bits per key, fully horizontal add (Theta=8) and fully vertical
- * contains (Phi=8). This layout is bit-compatible with Apache Arrow, as verified by cuCollections
- * `tests/bloom_filter/arrow_compat_test.cu`.
+ * Uses `cudf::arrow_filter_policy` with the Apache Arrow layout: 256-bit blocks (8 x
+ * `uint32_t`), 8 fingerprint bits per key, fully horizontal add (Theta=8) and fully vertical
+ * contains (Phi=8). This layout is bit-compatible with Apache Arrow, as verified by cuCollections
+ * `tests/bloom_filter/arrow_compat_test.cu`.
🤖 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/bloom_filter_reader.cu` around lines 43 - 56, Update the
documentation for the arrow_filter_policy alias to describe
cudf::arrow_filter_policy rather than cuco::parametric_filter_policy, and
mention that the policy preserves the Apache Arrow layout while supporting more
blocks than the former Arrow limit, as covered by the compatibility test.
java/src/test/java/ai/rapids/cudf/VariantUtilsTest.java (1)

410-418: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding a Java-level success-path test for array indexing.

The malformed-path fix is correct, but only the error path for "[N]" syntax is exercised here. The underlying C++ layer gained several new success-path tests for array indexing (ApacheArrayPrimitiveIndexing, MixedObjectArrayTraversal, etc. in variant_extract_test.cpp), while the Java binding — which is now able to accept the same "[N]" paths — has no corresponding test confirming the JNI round-trip works (e.g. VariantUtils.extractVariantField(variant, "$[0]", DType.INT32)). This would need extending VariantEncoder with an array(...) builder analogous to object(...).

As per path instructions, "Check that Java API matches the underlying C++ API behavior" for java/**/*.

🤖 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 `@java/src/test/java/ai/rapids/cudf/VariantUtilsTest.java` around lines 410 -
418, Extend VariantEncoder with an array(...) builder analogous to object(...),
then add a success-path test near malformedPathThrows in VariantUtilsTest that
constructs an array variant and verifies
VariantUtils.extractVariantField(variant, "$[0]", DType.INT32) returns the
expected value, confirming Java/JNI array-indexing behavior matches C++.

Source: Path instructions

cpp/src/io/parquet/experimental/variant_extract.cu (1)

373-430: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add Doxygen documentation to the new device helpers for consistency.

locate_array_element and parse_index_step use plain // comments, while sibling helpers in the same anonymous namespace with comparable bit-layout complexity (locate_object_field, variant_value_length, find_key_in_metadata, decode_object_array_header) all use full /** @brief ... */ Doxygen blocks documenting layout, parameters, and return semantics. Matching that style would keep the file's documentation generation/linting consistent.

As per coding guidelines, "Use doxygen as a documentation generator and linter for C++ and CUDA code" for **/*.{cpp,cu,h,hpp,cuh}.

Also applies to: 463-487

🤖 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/experimental/variant_extract.cu` around lines 373 - 430,
Replace the plain comments documenting locate_array_element and parse_index_step
with full Doxygen blocks using the surrounding helper style. Include `@brief` text
plus parameter and return documentation, preserving the existing descriptions of
layout, bounds, truncation, and empty-span semantics without changing
implementation behavior.

Source: Coding guidelines

🤖 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 `@java/src/main/java/ai/rapids/cudf/DeletionVector.java`:
- Around line 64-66: Restore the removed public DeletionVectorInfo constructor
overload, mark it deprecated, and delegate to the current constructor with
isRetention set to false. Keep the existing serializedBitmap, rowGroupOffsets,
and rowGroupNumRows parameters and preserve the current constructor’s behavior.

---

Nitpick comments:
In `@cpp/include/cudf/io/experimental/deletion_vectors.hpp`:
- Around line 33-54: Update the Doxygen comments for deletion_vector_info,
are_retention_vectors, chunked_parquet_reader, and compute_num_deleted_rows to
document both deletion-vector and retention-vector modes. Explain that
are_retention_vectors makes bitmap entries identify rows to retain rather than
delete, and that compute_num_deleted_rows returns the corresponding inverted
deleted-row count in retention mode.

In `@cpp/libcudf_streaming/tests/streaming/test_bloom_filter.cu`:
- Around line 30-35: Replace the raw __global__ qualifier on block_index_kernel
with the project-standard CUDF_KERNEL macro, preserving the kernel signature and
body.

In `@cpp/src/io/parquet/bloom_filter_reader.cu`:
- Around line 43-56: Update the documentation for the arrow_filter_policy alias
to describe cudf::arrow_filter_policy rather than
cuco::parametric_filter_policy, and mention that the policy preserves the Apache
Arrow layout while supporting more blocks than the former Arrow limit, as
covered by the compatibility test.

In `@cpp/src/io/parquet/experimental/variant_extract.cu`:
- Around line 373-430: Replace the plain comments documenting
locate_array_element and parse_index_step with full Doxygen blocks using the
surrounding helper style. Include `@brief` text plus parameter and return
documentation, preserving the existing descriptions of layout, bounds,
truncation, and empty-span semantics without changing implementation behavior.

In `@cpp/tests/io/parquet_deletion_vectors_test.cpp`:
- Around line 605-638: Extend the retention-mode loop used in
DeletionVectorsCountTests.NoRowIndex to the CustomRowIndex and
MultipleDeletionVectors tests. For each are_retention_vectors value, build the
corresponding expected mask and deletion-vector metadata, then assert
compute_num_deleted_rows for the existing chunk sizes against the matching
expected count while preserving their custom-index and multiple-bitmap
scenarios.

In `@java/src/test/java/ai/rapids/cudf/VariantUtilsTest.java`:
- Around line 410-418: Extend VariantEncoder with an array(...) builder
analogous to object(...), then add a success-path test near malformedPathThrows
in VariantUtilsTest that constructs an array variant and verifies
VariantUtils.extractVariantField(variant, "$[0]", DType.INT32) returns the
expected value, confirming Java/JNI array-indexing behavior matches C++.

In `@python/cudf_polars/cudf_polars/streaming/actor_graph/join.py`:
- Around line 809-811: Update the BloomFilter construction in the join actor
graph to derive filter_size from the expected build-side row count, such as
fitting_num_blocks, using BloomFilter.aligned_size(...), instead of the
hardcoded 32 MiB constant. Preserve the existing context, communicator, and
hash-seed arguments while sizing the filter for the estimated cardinality.
🪄 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: d1b02066-6bbf-4da3-aa92-fcc31bc5d516

📥 Commits

Reviewing files that changed from the base of the PR and between dadbc30 and a596c3e.

📒 Files selected for processing (58)
  • ci/run_cudf_polars_polars_tests.sh
  • ci/run_cudf_polars_pytests.sh
  • ci/test_python_other.sh
  • ci/test_wheel_cudf_polars.sh
  • cpp/CMakeLists.txt
  • cpp/include/cudf/io/experimental/deletion_vectors.hpp
  • cpp/include/cudf/io/experimental/variant.hpp
  • cpp/include/cudf/reduction/bloom_filter.cuh
  • cpp/libcudf_streaming/benchmarks/streaming/ndsh/q03.cpp
  • cpp/libcudf_streaming/benchmarks/streaming/ndsh/q04.cpp
  • cpp/libcudf_streaming/benchmarks/streaming/ndsh/q21.cpp
  • cpp/libcudf_streaming/include/cudf_streaming/bloom_filter.hpp
  • cpp/libcudf_streaming/include/cudf_streaming/detail/device_bloom_filter.hpp
  • cpp/libcudf_streaming/src/bloom_filter.cpp
  • cpp/libcudf_streaming/src/detail/device_bloom_filter.cu
  • cpp/libcudf_streaming/tests/CMakeLists.txt
  • cpp/libcudf_streaming/tests/streaming/test_bloom_filter.cu
  • cpp/libcudf_streaming/tests/streaming/test_bloom_filter_config.cpp
  • cpp/src/io/orc/writer_impl.cu
  • cpp/src/io/orc/writer_impl.hpp
  • cpp/src/io/parquet/bloom_filter_reader.cu
  • cpp/src/io/parquet/experimental/deletion_vectors.cu
  • cpp/src/io/parquet/experimental/variant_extract.cu
  • cpp/src/io/parquet/experimental/variant_path.cpp
  • cpp/src/io/parquet/experimental/variant_path.hpp
  • cpp/src/io/parquet/stats_filter_helpers.hpp
  • cpp/tests/io/experimental/variant_extract_test.cpp
  • cpp/tests/io/orc_test.cpp
  • cpp/tests/io/parquet_bloom_filter_test.cu
  • cpp/tests/io/parquet_deletion_vectors_test.cpp
  • dependencies.yaml
  • java/ci/assemble_maven_repo.sh
  • java/ci/build_cudf_java_jar_in_container.sh
  • java/src/main/java/ai/rapids/cudf/DeletionVector.java
  • java/src/main/native/src/DeletionVectorJni.cpp
  • java/src/test/java/ai/rapids/cudf/DeletionVectorTableTest.java
  • java/src/test/java/ai/rapids/cudf/VariantUtilsTest.java
  • python/cudf/cudf/core/column_accessor.py
  • python/cudf/cudf/core/dataframe.py
  • python/cudf/cudf/core/indexed_frame.py
  • python/cudf/cudf/core/multiindex.py
  • python/cudf/cudf/core/reshape.py
  • python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
  • python/cudf/cudf/tests/dataframe/methods/test_swaplevel.py
  • python/cudf/cudf/tests/private_objects/test_column_accessor.py
  • python/cudf/cudf/tests/reshape/test_pivot_table.py
  • python/cudf/cudf/tests/reshape/test_stack.py
  • python/cudf/cudf/tests/reshape/test_unstack.py
  • python/cudf_polars/cudf_polars/dsl/traversal.py
  • python/cudf_polars/cudf_polars/streaming/actor_graph/join.py
  • python/cudf_polars/cudf_polars/streaming/join_filter_pushdown.py
  • python/cudf_polars/cudf_polars/utils/config.py
  • python/cudf_polars/tests/streaming/test_join_filter_pushdown.py
  • python/cudf_polars/tests/test_config.py
  • python/cudf_streaming/cudf_streaming/bloom_filter.pxd
  • python/cudf_streaming/cudf_streaming/bloom_filter.pyi
  • python/cudf_streaming/cudf_streaming/bloom_filter.pyx
  • python/cudf_streaming/cudf_streaming/tests/test_bloom_filter.py
💤 Files with no reviewable changes (1)
  • ci/test_wheel_cudf_polars.sh
🚧 Files skipped from review as they are similar to previous changes (4)
  • ci/run_cudf_polars_pytests.sh
  • cpp/CMakeLists.txt
  • cpp/src/io/parquet/stats_filter_helpers.hpp
  • dependencies.yaml

Comment thread java/src/main/java/ai/rapids/cudf/DeletionVector.java
@Matt711

Matt711 commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

/ok to test a596c3e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CMake CMake build issue cudf.pandas Issues specific to cudf.pandas cudf-polars Issues specific to cudf-polars improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.