Skip to content

Commit

Permalink
Fixes issue whereby scale 29 is required however is optimized away (#619
Browse files Browse the repository at this point in the history
)

* Fixes issue whereby scale 29 is required however is optimized away
* Disable test for legacy ops
  • Loading branch information
paupino authored Nov 10, 2023
1 parent aacdefd commit 1686b69
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/ops/add.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::constants::{MAX_I32_SCALE, POWERS_10, SCALE_MASK, SCALE_SHIFT, SIGN_MASK, U32_MASK, U32_MAX};
use crate::constants::{
MAX_I32_SCALE, MAX_PRECISION_U32, POWERS_10, SCALE_MASK, SCALE_SHIFT, SIGN_MASK, U32_MASK, U32_MAX,
};
use crate::decimal::{CalculationResult, Decimal};
use crate::ops::common::{Buf24, Dec64};

Expand Down Expand Up @@ -261,7 +263,7 @@ fn unaligned_add(

rescale_factor -= MAX_I32_SCALE;

if tmp64 > U32_MAX {
if tmp64 > U32_MAX || scale > MAX_PRECISION_U32 {
break;
} else {
high = tmp64 as u32;
Expand Down
34 changes: 34 additions & 0 deletions tests/decimal_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4746,4 +4746,38 @@ mod issues {
assert!(c.is_some());
assert_eq!("-429391.87200000000002327170816", c.unwrap().to_string())
}

#[test]
#[cfg(not(feature = "legacy-ops"))] // I will deprecate this feature/behavior in an upcoming release
fn issue_618_rescaling_overflow() {
fn assert_result(scale: u32, v1: Decimal, v2: Decimal) {
assert_eq!(scale, v1.scale(), "initial scale: {scale}");
let result1 = v1 + -v2;
assert_eq!(
result1.to_string(),
"-0.0999999999999999999999999999",
"a + -b : {scale}"
);
assert_eq!(28, result1.scale(), "a + -b : {scale}");
let result2 = v1 - v2;
assert_eq!(
result2.to_string(),
"-0.0999999999999999999999999999",
"a - b : {scale}"
);
assert_eq!(28, result2.scale(), "a - b : {scale}");
}

let mut a = Decimal::from_str("0.0000000000000000000000000001").unwrap();
let b = Decimal::from_str("0.1").unwrap();
assert_result(28, a, b);

// Try at a new scale (this works)
a.rescale(30);
assert_result(30, a, b);

// And finally the scale causing an issue
a.rescale(29);
assert_result(29, a, b);
}
}

0 comments on commit 1686b69

Please sign in to comment.