Skip to content

Reuse null comparison helper in single-column sorting - #24142

Open
PointKernel wants to merge 1 commit into
NVIDIA:mainfrom
PointKernel:simple-comparator-utils
Open

Reuse null comparison helper in single-column sorting#24142
PointKernel wants to merge 1 commit into
NVIDIA:mainfrom
PointKernel:simple-comparator-utils

Conversation

@PointKernel

Copy link
Copy Markdown
Member

Description

This PR reuses null_compare in simple_comparator to remove duplicate null-ordering logic in single-column sorting.

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

copy-pr-bot Bot commented Sep 12, 2026

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added the libcudf Affects libcudf (C++/CUDA) code. label Sep 12, 2026
@PointKernel PointKernel added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 12, 2026
@PointKernel
PointKernel marked this pull request as ready for review September 12, 2026 01:26
@PointKernel
PointKernel requested a review from a team as a code owner September 12, 2026 01:26
@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown

Review Change StackReview 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: 80bccaf8-7e00-404d-9baf-06230db8fc3e

📥 Commits

Reviewing files that changed from the base of the PR and between a07a985 and a03921f.

📒 Files selected for processing (1)
  • cpp/src/sort/sort_column_impl.cuh

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Corrected null-value ordering during sorting, including descending sorts.
    • Preserved existing ordering behavior for non-null values.

Walkthrough

Changes

Sort null ordering

Layer / File(s) Summary
Comparator null ordering
cpp/src/sort/sort_column_impl.cuh
simple_comparator now uses null_compare and weak_ordering for null ordering. Non-null comparison behavior remains unchanged.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to a0392

The null-ordering refactor is mergeable with no material residual risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description accurately explains that simple_comparator reuses null_compare to remove duplicate null-ordering logic.
Title check ✅ Passed The title clearly summarizes the main change: reusing the null comparison helper in single-column sorting.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.

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

Labels

improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants