Skip to content

Fix Java Aggregation.sumOverflow doc: sort-based groupby is supported - #24949

Open
findepi wants to merge 2 commits into
NVIDIA:mainfrom
findepi:findepi/java-sum-overflow-doc
Open

findepi wants to merge 2 commits into
NVIDIA:mainfrom
findepi:findepi/java-sum-overflow-doc

Conversation

@findepi

@findepi findepi commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Description

Sort-based groupby support for SUM_OVERFLOW was added in #22832, but the Aggregation.sumOverflow Javadoc still said it was unsupported, contradicting GroupByAggregation.sumOverflow.

Checklist

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

Sort-based groupby support for SUM_OVERFLOW was added in NVIDIA#22832, but the
Aggregation.sumOverflow Javadoc still said it was unsupported, contradicting
GroupByAggregation.sumOverflow.
@findepi
findepi requested a review from a team as a code owner October 30, 2026 11:13
@findepi
findepi requested a review from liurenjie1024 October 30, 2026 11:13
@copy-pr-bot

copy-pr-bot Bot commented Oct 30, 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.

@github-actions github-actions Bot added the Java Affects Java cuDF API. label Oct 30, 2026
@findepi

findepi commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

cc @PointKernel

@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: NVIDIA/cudf
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: a52cc57
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: e22bcb93-b8ca-4c11-a2fd-f03981a85bf8
Commits

Reviewing files that changed from the base of the PR and between 13de9bf and faa56d5.

Files selected for processing: 1
  • java/src/main/java/ai/rapids/cudf/Aggregation.java

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

📝 Summary

Summary by CodeRabbit

  • Documentation
    • Clarified that sum overflow behavior supports fixed-point decimal input for column reductions, hash-based groupby, and sort-based groupby. Scan, segmented reduce, and rolling operations remain unsupported. This makes the supported and unsupported operations clearer when working with fixed-point decimal inputs.

The sumOverflow documentation now lists sort-based groupby as supported. Scan, segmented reduce, and rolling remain unsupported.

Changes: sumOverflow documentation

Layer / File(s) Summary
Update supported operations
java/src/main/java/ai/rapids/cudf/Aggregation.java
The documentation adds sort-based groupby to supported operations and removes it from the unsupported list.

Priority: Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Change: Other

Suggested reviewers: pointkernel

Merge Risk: Minimal · up to faa56

This documentation-only change is ready to merge.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly identifies the documentation fix and the supported sort-based groupby behavior for Aggregation.sumOverflow.
Description check Passed The description directly explains that the Javadoc incorrectly marked sort-based groupby as unsupported and states the related support change.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
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.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@PointKernel PointKernel added non-breaking Non-breaking change bug Something isn't working labels Oct 30, 2026
@PointKernel PointKernel added doc Documentation and removed bug Something isn't working labels Oct 30, 2026
@PointKernel

Copy link
Copy Markdown
Member

/ok to test faa56d5

@findepi

findepi commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

@PointKernel thanks for running the build. It turned back green.

@PointKernel

Copy link
Copy Markdown
Member

@PointKernel thanks for running the build. It turned back green.

Thanks for the reminder! This is a Java change, and I’m a C++ code owner, so I don’t have permission to approve Java PRs. I’ll ping the team on Monday to get it approved and merged.

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

Labels

doc Documentation Java Affects Java cuDF API. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants