diff --git a/lib/explorer/series.ex b/lib/explorer/series.ex index 177a7d64e..9382bc9ec 100644 --- a/lib/explorer/series.ex +++ b/lib/explorer/series.ex @@ -32,6 +32,11 @@ defmodule Explorer.Series do > > This functionality is considered unstable. It is a work-in-progress feature > and may not always work as expected. It may be changed at any point. + > + > Note: The underlying Polars decimal type is a fixed point decimal with optional + > precision and non-negative scale, backed by a signed 128-bit integer which allows + > for up to 38 significant digits (max precision is 38). Elixir's `Decimal` can handle + > higher precision, which may cause errors when converting very large decimal values. * `:null` - `nil`s exclusively * `:string` - UTF-8 encoded binary * `:time` - Time type that unwraps to `Elixir.Time` diff --git a/native/explorer/src/datatypes.rs b/native/explorer/src/datatypes.rs index c78aae0aa..de595d281 100644 --- a/native/explorer/src/datatypes.rs +++ b/native/explorer/src/datatypes.rs @@ -629,31 +629,42 @@ impl ExDecimal { } } - pub fn signed_coef(self) -> i128 { - self.sign as i128 * self.coef + pub fn signed_coef(self) -> Result { + let base = self.sign as i128 * self.coef; + if self.exp > 0 { + base.checked_mul(10_i128.pow(self.exp as u32)) + .ok_or_else(|| { + ExplorerError::Other( + "decimal coefficient overflow: value exceeds i128 limits".to_string(), + ) + }) + } else { + Ok(base) + } } pub fn scale(self) -> usize { - self.exp - .abs() - .try_into() - .expect("cannot convert exponent (Elixir) to scale (Rust)") + // A positive `exp` represents an integer. Polars requires `scale == 0` + // for integers. The case where `exp > 0` is handled by `.signed_coef`. + if self.exp > 0 { + 0 + } else { + self.exp + .abs() + .try_into() + .expect("cannot convert exponent (Elixir) to scale (Rust)") + } } } impl Literal for ExDecimal { fn lit(self) -> Expr { - let size = usize::try_from(-(self.exp)).expect("exponent should fit an usize"); + let coef = self.signed_coef().expect("decimal coefficient overflow"); + let scale = self.scale(); + Expr::Literal(LiteralValue::Scalar(Scalar::new( - DataType::Decimal(Some(size), Some(size)), - AnyValue::Decimal( - if self.sign.is_positive() { - self.coef - } else { - -self.coef - }, - size, - ), + DataType::Decimal(Some(scale), Some(scale)), + AnyValue::Decimal(coef, scale), ))) } } diff --git a/native/explorer/src/series/from_list.rs b/native/explorer/src/series/from_list.rs index 9e6c9e866..6d0492180 100644 --- a/native/explorer/src/series/from_list.rs +++ b/native/explorer/src/series/from_list.rs @@ -242,12 +242,13 @@ pub fn s_from_list_decimal( TermType::Map => item .decode::() - .map(|ex_decimal| AnyValue::Decimal(ex_decimal.signed_coef(), ex_decimal.scale())) - .map_err(|error| { + .map_err(|err| { ExplorerError::Other(format!( - "cannot decode a valid decimal from term; check that `coef` fits into an `i128`. error: {error:?}" + "cannot decode a valid decimal from term; check that `coef` fits into an `i128`. error: {err:?}" )) - }), + }) + .and_then(|ex_decimal| Ok(AnyValue::Decimal(ex_decimal.signed_coef()?, ex_decimal.scale()))), + TermType::Atom => Ok(AnyValue::Null), TermType::Float => item diff --git a/test/explorer/series_test.exs b/test/explorer/series_test.exs index e89fcc3ab..97d4b3605 100644 --- a/test/explorer/series_test.exs +++ b/test/explorer/series_test.exs @@ -1997,6 +1997,38 @@ defmodule Explorer.SeriesTest do assert s3.dtype == {:decimal, 38, 1} assert Series.to_list(s3) === [Decimal.new("2.2"), Decimal.new("4.0"), Decimal.new("6.1")] end + + test "adding decimal series with positive and negative exponents" do + s1 = + Series.from_list([ + Decimal.new("2.1e20"), + Decimal.new("-3.5e10"), + Decimal.new("1.5e-15"), + Decimal.new("1.0e2") + ]) + + s2 = + Series.from_list([ + Decimal.new("1.0e20"), + Decimal.new("2.0e10"), + Decimal.new("2.5e-15"), + Decimal.new("5.0e-2") + ]) + + s3 = Series.add(s1, s2) + [v1, v2, v3, v4] = Series.to_list(s3) + + assert Decimal.eq?(v1, Decimal.new("3.1e20")) + assert Decimal.eq?(v2, Decimal.new("-1.5e10")) + assert Decimal.eq?(v3, Decimal.new("4.0e-15")) + assert Decimal.eq?(v4, Decimal.new("100.05")) + end + + test "overflow with values exceeding i128 limits" do + assert_raise RuntimeError, + "Generic Error: decimal coefficient overflow: value exceeds i128 limits", + fn -> Series.from_list([Decimal.new("3.4e38")]) end + end end describe "subtract/2" do