From 86a23ebb709873565bd89681434fea1bd77fe011 Mon Sep 17 00:00:00 2001 From: Orson Peters Date: Thu, 5 Sep 2024 22:10:20 +0200 Subject: [PATCH 1/4] refactor(rust): Correctly support more types in new-streaming sum --- crates/polars-core/src/datatypes/any_value.rs | 28 +++++++++++++++---- crates/polars-expr/src/reduce/sum.rs | 3 +- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/crates/polars-core/src/datatypes/any_value.rs b/crates/polars-core/src/datatypes/any_value.rs index d59cc1317901..9009902c1277 100644 --- a/crates/polars-core/src/datatypes/any_value.rs +++ b/crates/polars-core/src/datatypes/any_value.rs @@ -341,17 +341,21 @@ impl<'a> Deserialize<'a> for AnyValue<'static> { } impl AnyValue<'static> { - pub fn zero(dtype: &DataType) -> Self { + pub fn zero_sum(dtype: &DataType) -> Self { match dtype { DataType::String => AnyValue::StringOwned(PlSmallStr::EMPTY), - DataType::Boolean => AnyValue::Boolean(false), - // SAFETY: - // Numeric values are static, inform the compiler of this. + DataType::Binary => AnyValue::BinaryOwned(Vec::new()), + DataType::Boolean => (0 as IdxSize).into(), + // SAFETY: numeric values are static, inform the compiler of this. d if d.is_numeric() => unsafe { std::mem::transmute::, AnyValue<'static>>( AnyValue::UInt8(0).cast(dtype), ) }, + DataType::Duration(unit) => AnyValue::Duration(0, *unit), + DataType::Decimal(_p, s) => { + AnyValue::Decimal(0, s.expect("unknown scale during execution")) + }, _ => AnyValue::Null, } } @@ -836,7 +840,21 @@ impl<'a> AnyValue<'a> { (UInt64(l), UInt64(r)) => UInt64(l + r), (Float32(l), Float32(r)) => Float32(l + r), (Float64(l), Float64(r)) => Float64(l + r), - _ => todo!(), + (Duration(l, lu), Duration(r, ru)) => { + if lu != ru { + unimplemented!("adding durations with different units is not supported here"); + } + + Duration(l + r, *lu) + }, + (Decimal(l, ls), Decimal(r, rs)) => { + if ls != rs { + unimplemented!("adding decimals with different scales is not supported here"); + } + + Decimal(l + r, *ls) + }, + _ => unimplemented!(), } } diff --git a/crates/polars-expr/src/reduce/sum.rs b/crates/polars-expr/src/reduce/sum.rs index fa19f6ebd860..0f1d094ded3f 100644 --- a/crates/polars-expr/src/reduce/sum.rs +++ b/crates/polars-expr/src/reduce/sum.rs @@ -13,6 +13,7 @@ impl SumReduce { // returning the empty sum to be consistent. use DataType::*; let dtype = match dtype { + Boolean => IDX_DTYPE, Int8 | UInt8 | Int16 | UInt16 => Int64, dt => dt, }; @@ -22,7 +23,7 @@ impl SumReduce { impl Reduction for SumReduce { fn new_reducer(&self) -> Box { - let value = Scalar::new(self.dtype.clone(), AnyValue::zero(&self.dtype)); + let value = Scalar::new(self.dtype.clone(), AnyValue::zero_sum(&self.dtype)); Box::new(SumReduceState { value }) } } From ee54ff5c34ea429908f6b06b14c5d343a9f70667 Mon Sep 17 00:00:00 2001 From: Orson Peters Date: Fri, 6 Sep 2024 10:58:29 +0200 Subject: [PATCH 2/4] add missing feature flags --- crates/polars-core/src/datatypes/any_value.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/polars-core/src/datatypes/any_value.rs b/crates/polars-core/src/datatypes/any_value.rs index 9009902c1277..58cd5a704894 100644 --- a/crates/polars-core/src/datatypes/any_value.rs +++ b/crates/polars-core/src/datatypes/any_value.rs @@ -353,6 +353,7 @@ impl AnyValue<'static> { ) }, DataType::Duration(unit) => AnyValue::Duration(0, *unit), + #[cfg(feature="dtype-decimal")] DataType::Decimal(_p, s) => { AnyValue::Decimal(0, s.expect("unknown scale during execution")) }, @@ -847,6 +848,7 @@ impl<'a> AnyValue<'a> { Duration(l + r, *lu) }, + #[cfg(feature="dtype-decimal")] (Decimal(l, ls), Decimal(r, rs)) => { if ls != rs { unimplemented!("adding decimals with different scales is not supported here"); From 65abaf97de58bfdda6f3703eace1e1045f1792d8 Mon Sep 17 00:00:00 2001 From: Orson Peters Date: Fri, 6 Sep 2024 12:09:55 +0200 Subject: [PATCH 3/4] more missing feature flags --- crates/polars-core/src/datatypes/any_value.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/polars-core/src/datatypes/any_value.rs b/crates/polars-core/src/datatypes/any_value.rs index 58cd5a704894..c36c1b628a07 100644 --- a/crates/polars-core/src/datatypes/any_value.rs +++ b/crates/polars-core/src/datatypes/any_value.rs @@ -352,6 +352,7 @@ impl AnyValue<'static> { AnyValue::UInt8(0).cast(dtype), ) }, + #[cfg(feature = "dtype-duration")] DataType::Duration(unit) => AnyValue::Duration(0, *unit), #[cfg(feature="dtype-decimal")] DataType::Decimal(_p, s) => { @@ -841,6 +842,7 @@ impl<'a> AnyValue<'a> { (UInt64(l), UInt64(r)) => UInt64(l + r), (Float32(l), Float32(r)) => Float32(l + r), (Float64(l), Float64(r)) => Float64(l + r), + #[cfg(feature = "dtype-duration")] (Duration(l, lu), Duration(r, ru)) => { if lu != ru { unimplemented!("adding durations with different units is not supported here"); From 8c93e4959940bc730594d1190be8971f0357cfd4 Mon Sep 17 00:00:00 2001 From: Orson Peters Date: Fri, 6 Sep 2024 16:07:43 +0200 Subject: [PATCH 4/4] fmt --- crates/polars-core/src/datatypes/any_value.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/polars-core/src/datatypes/any_value.rs b/crates/polars-core/src/datatypes/any_value.rs index c36c1b628a07..5475b8fb5203 100644 --- a/crates/polars-core/src/datatypes/any_value.rs +++ b/crates/polars-core/src/datatypes/any_value.rs @@ -354,7 +354,7 @@ impl AnyValue<'static> { }, #[cfg(feature = "dtype-duration")] DataType::Duration(unit) => AnyValue::Duration(0, *unit), - #[cfg(feature="dtype-decimal")] + #[cfg(feature = "dtype-decimal")] DataType::Decimal(_p, s) => { AnyValue::Decimal(0, s.expect("unknown scale during execution")) }, @@ -850,7 +850,7 @@ impl<'a> AnyValue<'a> { Duration(l + r, *lu) }, - #[cfg(feature="dtype-decimal")] + #[cfg(feature = "dtype-decimal")] (Decimal(l, ls), Decimal(r, rs)) => { if ls != rs { unimplemented!("adding decimals with different scales is not supported here");