Skip to content

Fix NULL handling in ScalarValue::partial_cmp (closes #19579) #19587

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Brijesh-Thakkar wants to merge 10 commits into
base: main
Choose a base branch
from
+89 −21
Open

Fix NULL handling in ScalarValue::partial_cmp (closes #19579) #19587

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4fb4147
Fix ScalarValue partial ordering behavior with NULLs
Brijesh-Thakkar Dec 31, 2025
390c7c7
Remove unreachable NULL match arm in ScalarValue::partial_cmp
Brijesh-Thakkar Dec 31, 2025
500697e
Add tests for explicit ScalarValue::Null partial ordering
Brijesh-Thakkar Dec 31, 2025
d868e5b
docs: clarify try_cmp behavior for NULL inputs
Brijesh-Thakkar Jan 1, 2026
1e8b532
Align vector ordering with PostgreSQL NULL semantics
Brijesh-Thakkar Jan 1, 2026
b79699a
Merge branch 'main' into fix-scalarvalue-null-partial-cmp
Brijesh-Thakkar Jan 1, 2026
552a1e9
Align vector ordering tests with ScalarValue NULL partial_cmp semantics
Brijesh-Thakkar Jan 1, 2026
6ef8e57
Merge branch 'main' into fix-scalarvalue-null-partial-cmp
Brijesh-Thakkar Jan 2, 2026
8cdb969
Fix ScalarValue partial_cmp and clippy warnings in datafusion-common
Brijesh-Thakkar Jan 2, 2026
3f8c1db
Fix ORDER BY rewrite for aggregate aliases and preserve casts
Brijesh-Thakkar Jan 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions datafusion/common/src/scalar/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,10 +568,15 @@ impl PartialEq for ScalarValue {
impl PartialOrd for ScalarValue {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
use ScalarValue::*;

if self.is_null() || other.is_null() {
return None;
}
// This purposely doesn't have a catch-all "(_, _)" so that
// any newly added enum variant will require editing this list
// or else face a compile error
match (self, other) {
(Null, _) | (_, Null) => None,
(Decimal32(v1, p1, s1), Decimal32(v2, p2, s2)) => {
if p1.eq(p2) && s1.eq(s2) {
v1.partial_cmp(v2)
Expand Down Expand Up @@ -723,8 +728,6 @@ impl PartialOrd for ScalarValue {
if k1 == k2 { v1.partial_cmp(v2) } else { None }
}
(Dictionary(_, _), _) => None,
(Null, Null) => Some(Ordering::Equal),
(Null, _) => None,
}
}
}
Expand Down Expand Up @@ -5760,10 +5763,9 @@ mod tests {
.unwrap(),
Ordering::Less
);
assert_eq!(
assert!(
ScalarValue::try_cmp(&ScalarValue::Int32(None), &ScalarValue::Int32(Some(2)))
.unwrap(),
Ordering::Less
.is_err()
);
assert_starts_with(
ScalarValue::try_cmp(
Expand Down Expand Up @@ -9348,4 +9350,23 @@ mod tests {
]
);
}
#[test]
fn scalar_partial_ordering_nulls() {
use ScalarValue::*;

assert_eq!(
Int32(Some(3)).partial_cmp(&Int32(None)),
None
);

assert_eq!(
Int32(None).partial_cmp(&Int32(Some(3))),
None
);

assert_eq!(
Int32(None).partial_cmp(&Int32(None)),
None
);
}
}
12 changes: 8 additions & 4 deletions datafusion/common/src/utils/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1026,22 +1026,26 @@ mod tests {
ScalarValue::Int32(Some(2)),
Null,
ScalarValue::Int32(Some(0)),
] < vec![
]
.partial_cmp(&vec![
ScalarValue::Int32(Some(2)),
Null,
ScalarValue::Int32(Some(1)),
]
])
.is_none()
);
assert!(
vec![
ScalarValue::Int32(Some(2)),
ScalarValue::Int32(None),
ScalarValue::Int32(Some(0)),
] < vec![
]
.partial_cmp(&vec![
ScalarValue::Int32(Some(2)),
ScalarValue::Int32(None),
ScalarValue::Int32(Some(1)),
]
])
.is_none()
);
}

Expand Down