Panic when rounding long fractions

[dpbriggs]

The following code:

use bigdecimal::BigDecimal;

fn main() {
    let n = BigDecimal::from(1);
    let d = BigDecimal::from(3);
    println!("{}", (n / d).round(0));
}

Panics when ran:

➜  bigdecimal-unwrap git:(master) cargo run
   Compiling bigdecimal-unwrap v0.1.0 (/home/david/programming/bigdecimal-unwrap)
    Finished dev [unoptimized + debuginfo] target(s) in 0.19s
     Running `target/debug/bigdecimal-unwrap`
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /home/david/.cargo/registry/src/github.com-1ecc6299db9ec823/bigdecimal-0.2.0/src/lib.rs:596:43
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Expected result: It rounds the number to 0.

Repo with example is here.

It looks like this line is the culprit.

[dpbriggs]

This patch fixes the issue - should I open a pull request?

@@ -593,8 +593,8 @@ impl BigDecimal {
             return self.clone();
         }
 
-        let mut number = bigint.to_i128().unwrap();
-        if number < 0 {
+        let mut number = bigint.clone();
+        if number < BigInt::zero() {
             number = -number;
         }
         for _ in 0..(need_to_round_digits - 1) {
@@ -602,7 +602,7 @@ impl BigDecimal {
         }
         let digit = number % 10;
 
-        if digit <= 4 {
+        if digit <= BigInt::from(4) {
             self.with_scale(round_digits)
         } else if bigint.is_negative() {
             self.with_scale(round_digits) - BigDecimal::new(BigInt::from(1), round_digits)
@@ -2598,6 +2598,7 @@ mod bigdecimal_tests {
             ("1.449999999", 1, "1.4"),
             ("-9999.444455556666", 10, "-9999.4444555567"),
             ("-12345678987654321.123456789", 8, "-12345678987654321.12345679"),
+            ("0.33333333333333333333333333333333333333333333333333333333333333333333333333333333333333", 0, "0"),
         ];
         for &(x, digits, y) in test_cases.iter() {
             let a = BigDecimal::from_str(x).unwrap();

[jubos]

I ran into this one today as well. @akubera, any thoughts on merging this?

[dpbriggs]

@jubos if you're comfortable pointing to a git repo - I've forked & fixed the issue here: https://github.com/dpbriggs/bigdecimal-rs

Make sure you get the right rev, so here's the line I use in my Cargo.toml:

bigdecimal = { git = "https://github.com/dpbriggs/bigdecimal-rs", rev="02ba26b", features = ["serde"] }

[jubos]

Thanks @dpbriggs, I pulled this down and worked for me.

[kyle-silver]

Any thoughts on opening a PR to get this merged?

[dpbriggs]

@kyleAsilver I made a PR here: https://github.com/akubera/bigdecimal-rs/pull/72

[mreschke]

Too bad this library is not maintained. I may have to switch to @dpbriggs fork at some point. As a temp fix I had to create a total hacked function to help divide two BigNums

/// Divide (dividend / divisor) two BigInts as BigDecimals with Rounding up to 20 decimal places
pub fn divide_bigint(dividend: BigInt, divisor: BigInt, mut round: usize) -> BigDecimal {
    // This function is a hack to fix  BigDecimals .round() method.  Because BigDecimal crashes if the decimal is larger than a u128.
    // When I divide 2 huge BigNum like:
    // 26959535291011309493156476344723991336010898738574164086137773096960 / 1280876111474813957445809627925710317539675495922139136
    // I get a BigDecimal 21047730572451.55635883164405741141998445934612869989080465671927896842919589096896319192707531055635
    // and when that massive decimal is rounded, BigDecimal crashes.  So this hack function trims that decimal by the round
    // parameter + 1 then calls BigDecimal.round() to avoid the crash.

    // Max round without BigDecimal erroring is ~20 places
    if round > 20 { round = 20; }

    // Convert BigInt into BigDecimal
    let dividend = BigDecimal::new(dividend, 0);  // ex: 26959535291011309493156476344723991336010898738574164086137773096960
    let divisor = BigDecimal::new(divisor, 0);    // ex: 1280876111474813957445809627925710317539675495922139136

    // Divide the two BigDecimals
    // ex: 21047730572451.55635883164405741141998445934612869989080465671927896842919589096896319192707531055635
    let mut result: String = (dividend / divisor).to_string();

    // Split result by decimal place
    // ex: ["21047730572451", "55635883164405741141998445934612869989080465671927896842919589096896319192707531055635"]
    let split : Vec<&str> = result.split(".").collect();

    // If length of decimal places is > round then truncate it to (round + 1)
    // ex: 21047730572451.55635883164405741142
    if split.len() == 2 && split[1].len() > round {
        result = BigDecimal::from_str(
            // Whole number + truncated decimal places
            &(split[0].to_string() + "." + &split[1][0..(round + 1)].to_string())
        )
        .unwrap()
        .round(round as i64)
        .to_string();
    }
    
    // Convert and retrun string as BigDecimal
    BigDecimal::from_str(&result).unwrap()
}

[notdanilo]

I just published @dpbriggs fix as https://crates.io/crates/bigdecimal-rs while the PR isn't accepted and merged. I will not maintain this fork and it's just a workaround to allow us to publish crates that depend on it. Here is a tip if you want to replace bigdecimal with bigdecimal-rs:

bigdecimal = { package = "bigdecimal-rs", version = "0.2.1", features = ["serde"] }

Cheers!

[akubera]

This has been fixed. My apologies about the hiatus.