Document null binary operator semantics explicitly - #23604
Open
findepi wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
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
📒 Files selected for processing (4)
cpp/include/cudf/ast/ast_operator.hppcpp/include/cudf/binaryop.hppjava/src/main/java/ai/rapids/cudf/BinaryOp.javajava/src/main/java/ai/rapids/cudf/ast/BinaryOperator.java
findepi
force-pushed
the
findepi/document-null-binary-operator-semantics-explicitly-a1b3c9
branch
from
August 10, 2026 14:12
87621eb to
a612eae
Compare
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe 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. ChangesOperator semantics documentation
Estimated code review effort: 1 (Trivial) | ~4 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
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
force-pushed
the
findepi/document-null-binary-operator-semantics-explicitly-a1b3c9
branch
from
August 11, 2026 09:27
a612eae to
55f7341
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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