Skip to content

Document null binary operator semantics explicitly - #23604

Open
findepi wants to merge 1 commit into
NVIDIA:mainfrom
findepi:findepi/document-null-binary-operator-semantics-explicitly-a1b3c9
Open

Document null binary operator semantics explicitly#23604
findepi wants to merge 1 commit into
NVIDIA:mainfrom
findepi:findepi/document-null-binary-operator-semantics-explicitly-a1b3c9

Conversation

@findepi

@findepi findepi commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Description

The null-aware binary operators deferred to "Spark rules for null inputs", requiring the reader to already know what those are. Describe the behavior explicitly instead. Spell out the null-propagating behavior of the plain EQUAL, NOT_EQUAL, LOGICAL_AND, and LOGICAL_OR operators. Keep the wording consistent across the codebase (C++ and Java).

Checklist

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

@findepi
findepi requested review from a team as code owners August 10, 2026 13:26
@copy-pr-bot

copy-pr-bot Bot commented Aug 10, 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 libcudf Affects libcudf (C++/CUDA) code. Java Affects Java cuDF API. labels Aug 10, 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: 2

🤖 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 `@cpp/include/cudf/ast/ast_operator.hpp`:
- Around line 39-40: Update every NOT_EQUAL enum entry to explicitly document
that it returns NULL when either operand is NULL, otherwise returns whether the
operands are unequal: cpp/include/cudf/ast/ast_operator.hpp lines 39-40,
java/src/main/java/ai/rapids/cudf/ast/BinaryOperator.java lines 25-28, and
java/src/main/java/ai/rapids/cudf/BinaryOp.java lines 36-37. Keep the existing
NULL_EQUAL documentation unchanged.

In `@cpp/include/cudf/binaryop.hpp`:
- Around line 73-88: Update the shared output-validity documentation for all
three binary_operation overloads to match the null-aware operator semantics:
NULL_EQUALS and NULL_NOT_EQUALS always produce valid results, while
NULL_LOGICAL_AND and NULL_LOGICAL_OR produce valid results for decisive
false/true cases respectively and null otherwise. Preserve the existing
logical-AND validity rule for other operators and the NULL_MIN/NULL_MAX
exceptions.
🪄 Autofix

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: fa155177-3442-4bb5-b326-a327e7f75bd3

📥 Commits

Reviewing files that changed from the base of the PR and between 6a22d1d and 87621eb.

📒 Files selected for processing (4)
  • cpp/include/cudf/ast/ast_operator.hpp
  • cpp/include/cudf/binaryop.hpp
  • java/src/main/java/ai/rapids/cudf/BinaryOp.java
  • java/src/main/java/ai/rapids/cudf/ast/BinaryOperator.java

Comment thread cpp/include/cudf/ast/ast_operator.hpp Outdated
Comment thread cpp/include/cudf/binaryop.hpp
@findepi
findepi force-pushed the findepi/document-null-binary-operator-semantics-explicitly-a1b3c9 branch from 87621eb to a612eae Compare August 10, 2026 14:12
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Documentation
    • Clarified equality, logical, and null-safe operator behavior.
    • Added explicit descriptions of null propagation and Kleene three-valued logic.
    • Documented validity behavior for null-safe binary operations.
    • Aligned operator documentation across C++ and Java APIs.
    • Updated the Java copyright year.

Walkthrough

The change updates C++ and Java documentation for binary operator null handling and three-valued logic. Enum values, identifiers, API declarations, and runtime behavior remain unchanged.

Changes

Operator semantics documentation

Layer / File(s) Summary
Cross-language operator semantics
cpp/include/cudf/ast/ast_operator.hpp, cpp/include/cudf/binaryop.hpp, java/src/main/java/ai/rapids/cudf/BinaryOp.java, java/src/main/java/ai/rapids/cudf/ast/BinaryOperator.java
Documentation now specifies null propagation for standard logical and equality operators, null-safe equality results, and Kleene three-valued behavior for null-prefixed logical operators. The Java copyright year is updated to 2026.

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

Suggested reviewers: mythrocks, davidwendt

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly and concisely describes the main change: documenting null binary operator semantics.
Description check ✅ Passed The description directly explains the documentation changes and their scope across C++ and Java.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

The null-aware binary operators deferred to "Spark rules for null
inputs", requiring the reader to already know what those are. Describe
the behavior explicitly instead. Spell out the null-propagating behavior
of the plain EQUAL, NOT_EQUAL, LOGICAL_AND, and LOGICAL_OR operators.
Keep the wording consistent across the codebase (C++ and Java).
@findepi
findepi force-pushed the findepi/document-null-binary-operator-semantics-explicitly-a1b3c9 branch from a612eae to 55f7341 Compare August 11, 2026 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant