Skip to content

Commit

Permalink
refactor(rust): Fix nan-ignoring max/min in new-streaming (#18593)
Browse files Browse the repository at this point in the history
  • Loading branch information
orlp authored Sep 6, 2024
1 parent dcd4195 commit 106e239
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 2 deletions.
8 changes: 8 additions & 0 deletions crates/polars-core/src/datatypes/any_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,14 @@ impl<'a> AnyValue<'a> {
)
}

pub fn is_nan(&self) -> bool {
match self {
AnyValue::Float32(f) => f.is_nan(),
AnyValue::Float64(f) => f.is_nan(),
_ => false,
}
}

pub fn is_null(&self) -> bool {
matches!(self, AnyValue::Null)
}
Expand Down
5 changes: 5 additions & 0 deletions crates/polars-core/src/scalar/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ impl Scalar {
self.value.is_null()
}

#[inline(always)]
pub fn is_nan(&self) -> bool {
self.value.is_nan()
}

#[inline(always)]
pub fn value(&self) -> &AnyValue<'static> {
&self.value
Expand Down
8 changes: 6 additions & 2 deletions crates/polars-expr/src/reduce/min_max.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ struct MinReduceState {

impl MinReduceState {
fn update_with_value(&mut self, other: &AnyValue<'static>) {
if self.value.is_null() || !other.is_null() && other < self.value.value() {
if self.value.is_null()
|| !other.is_null() && (other < self.value.value() || self.value.is_nan())
{
self.value.update(other.clone());
}
}
Expand Down Expand Up @@ -78,7 +80,9 @@ struct MaxReduceState {

impl MaxReduceState {
fn update_with_value(&mut self, other: &AnyValue<'static>) {
if self.value.is_null() || !other.is_null() && other > self.value.value() {
if self.value.is_null()
|| !other.is_null() && (other > self.value.value() || self.value.is_nan())
{
self.value.update(other.clone());
}
}
Expand Down

0 comments on commit 106e239

Please sign in to comment.