Skip to content
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.

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

Use coef: i128 in decimal #1017

Merged
merged 4 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 10 additions & 8 deletions native/explorer/src/datatypes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,23 +569,25 @@ impl Literal for ExTime {
#[module = "Decimal"]
pub struct ExDecimal {
pub sign: i8,
pub coef: u64,
// `coef` is a positive, arbitrary precision integer on the Elixir side.
// It's convenient to represent it here as a signed `i128` because that's
// what the Decimal dtype expects. While you could technically create an
// `ExDecimal` struct with a negative `coef`, it's not a practical concern.
pub coef: i128,
pub exp: i64,
}

impl ExDecimal {
pub fn new(signed_coef: i128, scale: usize) -> Self {
Self {
coef: signed_coef
.abs()
.try_into()
.expect("signed coef is too large for u64"),
coef: signed_coef.abs(),
sign: if signed_coef >= 0 { 1 } else { -1 },
exp: -(scale as i64),
}
}

pub fn signed_coef(self) -> i128 {
self.sign as i128 * self.coef as i128
self.sign as i128 * self.coef
}

pub fn scale(self) -> usize {
Expand All @@ -600,9 +602,9 @@ impl Literal for ExDecimal {
fn lit(self) -> Expr {
Expr::Literal(LiteralValue::Decimal(
if self.sign.is_positive() {
self.coef.into()
self.coef
} else {
-(self.coef as i128)
-self.coef
},
usize::try_from(-(self.exp)).expect("exponent should fit an usize"),
))
Expand Down
17 changes: 17 additions & 0 deletions test/explorer/series_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,23 @@ defmodule Explorer.SeriesTest do
Series.from_list([[Decimal.new("3.21")], []], dtype: {:list, {:decimal, 3, 2}})
end

test "decimal precision boundary" do
# Biggest number representable by an i128.
max_i128 = 2 ** 127 - 1
biggest = Decimal.new(max_i128)
too_big = Decimal.new(max_i128 + 1)

# Can make a series using the biggest allowed decimal.
s1 = Series.from_list([biggest])
assert [^biggest] = Series.to_list(s1)

# Can't make a series using a number bigger than the biggest allowed decimal.
assert_raise RuntimeError,
"Generic Error: cannot decode a valid decimal from term;" <>
" check that `coef` fits into an `i128`. error: throw(<term>)",
fn -> Series.from_list([too_big]) end
end

test "mixing dates and integers with `:date` dtype" do
s = Series.from_list([1, nil, ~D[2024-06-13]], dtype: :date)

Expand Down
8 changes: 4 additions & 4 deletions test/support/generator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,8 @@ defmodule Explorer.Generator do
date: constant(:date),
datetime: tuple({constant(:datetime), time_unit(), constant("Etc/UTC")}),
decimal:
bind(integer(0..19), fn scale ->
bind(integer((scale + 1)..20), fn precision ->
bind(integer(0..37), fn scale ->
bind(integer((scale + 1)..38), fn precision ->
tuple({constant(:decimal), constant(precision), constant(scale)})
end)
end),
Expand Down Expand Up @@ -397,7 +397,7 @@ defmodule Explorer.Generator do
|> map(&elem(&1, 1))
end

@max_u64 2 ** 64 - 1
@max_i128 2 ** 127 - 1
@spec datetime(pos_integer(), pos_integer()) :: gen(Decimal.t())
defp decimal(precision, scale) do
# Smallest integer with `(scale + 1)` digits.
Expand All @@ -407,7 +407,7 @@ defmodule Explorer.Generator do

tuple({
one_of([constant("-"), constant("+")]),
integer(min_scale..min(max_precision, @max_u64))
integer(min_scale..min(max_precision, @max_i128))
})
|> map(fn {sign, coef} ->
# By construction, we are guaranteed that precision > scale.
Expand Down
Loading