Skip to content

Avoid leaving multi-rank execution in a bad state in cudf-polars tests - #23340

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:release/26.08from
wence-:wence/fix/test-flaky
Jul 20, 2026
Merged

Avoid leaving multi-rank execution in a bad state in cudf-polars tests#23340
rapids-bot[bot] merged 2 commits into
NVIDIA:release/26.08from
wence-:wence/fix/test-flaky

Conversation

@wence-

@wence- wence- commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Description

If a cudf-polars query raises an exception during execution, it is possible that it can leave a dangling collective that then tears down one process.

This is racy because it depends on tasks being cancelled in Python and then dropping C++ objects in a particular order.

The one test in the cudf-polars tests suite that could do this is fixed on main, so backport the relevant changes (#23235), and un-xfail the test.

Checklist

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

madsbk and others added 2 commits July 20, 2026 17:58
Previously, `cross_join` threw `"Left table is empty"` or `"Right table is empty"` when either operand had zero columns. Zero-column operands with non-zero row counts are now valid. The result has `left.num_rows() * right.num_rows()` rows and `left.num_columns() + right.num_columns()` columns.

Authors:
  - Mads R. B. Kristensen (https://github.com/madsbk)

Approvers:
  - Matthew Roeschke (https://github.com/mroeschke)
  - Yunsong Wang (https://github.com/PointKernel)
  - Muhammad Haseeb (https://github.com/mhaseeb123)

URL: NVIDIA#23235
@wence-
wence- requested review from a team as code owners July 20, 2026 18:38
@wence- wence- added the bug Something isn't working label Jul 20, 2026
@wence-
wence- requested a review from vyasr July 20, 2026 18:38
@wence- wence- added the non-breaking Non-breaking change label Jul 20, 2026
@wence-
wence- requested review from abigalekim and rjzamora July 20, 2026 18:38
@github-actions github-actions Bot added libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels Jul 20, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python Jul 20, 2026
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

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: cd4c09db-8228-404f-a271-42b9d11f1309

📥 Commits

Reviewing files that changed from the base of the PR and between 95390db and 5509992.

📒 Files selected for processing (6)
  • cpp/include/cudf/join/join.hpp
  • cpp/src/join/cross_join.cu
  • cpp/tests/join/cross_join_tests.cpp
  • python/cudf/cudf/core/join/join.py
  • python/cudf/cudf/tests/reshape/test_merge.py
  • python/cudf_polars/tests/streaming/test_select.py
💤 Files with no reviewable changes (1)
  • python/cudf_polars/tests/streaming/test_select.py

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Cross joins now support inputs with zero columns while preserving the correct output row count.
    • Added protection against cross-join row-count overflow with a clear error.
    • Cross-merge results now correctly retain their row count when the output has no columns.
  • Tests
    • Expanded coverage for zero-column cross joins and overflow handling.
    • Updated streaming selection tests to reflect corrected behavior.

Walkthrough

Changes

The cross-join implementation now supports zero-column operands, preserves their row counts, and throws std::overflow_error when the output row count exceeds cudf::size_type. cuDF cross merges preserve equivalent row counts, and a Polars streaming test removes a version-specific xfail.

Cross-join behavior

Layer / File(s) Summary
libcudf cross-join contract and implementation
cpp/include/cudf/join/join.hpp, cpp/src/join/cross_join.cu, cpp/tests/join/cross_join_tests.cpp
Documentation, implementation, and tests now cover zero-column operands, explicit output row counts, and overflow exceptions.
cuDF cross-merge row preservation
python/cudf/cudf/core/join/join.py, python/cudf/cudf/tests/reshape/test_merge.py
Cross merges pass explicit row counts through libcudf and preserve zero-column result indexes, with pandas comparison coverage.

Polars streaming test expectation

Layer / File(s) Summary
Unconditional streaming selection test
python/cudf_polars/tests/streaming/test_select.py
test_select_with_len no longer depends on a Polars version check or conditional xfail marker.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: improvement

Suggested reviewers: vyasr, galipremsagar

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main goal of preventing bad multi-rank execution states in cudf-polars tests.
Description check ✅ Passed The description is clearly about the same cudf-polars backport and test fix as the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

@rjzamora rjzamora left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Python changes look reasonable to me. Thanks!

@TomAugspurger

Copy link
Copy Markdown
Contributor

For reference, this is part of a series of changes supporting length 0 tables in libcudf/cudf-polars (#22765, #23234). I believe this is the only bit that wasn't merged before release/26.08 was created.

@vyasr vyasr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks.

@TomAugspurger

Copy link
Copy Markdown
Contributor

/merge

@rapids-bot
rapids-bot Bot merged commit f68d00b into NVIDIA:release/26.08 Jul 20, 2026
138 of 140 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in cuDF Python Jul 20, 2026
@wence-
wence- deleted the wence/fix/test-flaky branch July 21, 2026 09:13
davidwendt pushed a commit to wjxiz1992/cudf that referenced this pull request Jul 21, 2026
NVIDIA#23340)

If a cudf-polars query raises an exception during execution, it is possible that it can leave a dangling collective that then tears down one process.

This is racy because it depends on tasks being cancelled in Python and then dropping C++ objects in a particular order.

The one test in the cudf-polars tests suite that could do this is fixed on main, so backport the relevant changes (NVIDIA#23235), and un-xfail the test.

Authors:
  - Lawrence Mitchell (https://github.com/wence-)
  - Mads R. B. Kristensen (https://github.com/madsbk)

Approvers:
  - Richard (Rick) Zamora (https://github.com/rjzamora)
  - Matthew Murray (https://github.com/Matt711)
  - Bradley Dice (https://github.com/bdice)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: NVIDIA#23340
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cudf-polars Issues specific to cudf-polars 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.

9 participants