Skip to content

Commit

Permalink
fix(query): decimal div overflow (databendlabs#15856)
Browse files Browse the repository at this point in the history
* fix(query): decimal div overflow

* fix(query): decimal div overflow

* fix(query): decimal div overflow
  • Loading branch information
sundy-li authored Jun 22, 2024
1 parent 85ac9f7 commit b256d3d
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 4 deletions.
20 changes: 20 additions & 0 deletions src/query/expression/src/types/decimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,8 @@ pub trait Decimal:
fn checked_mul(self, rhs: Self) -> Option<Self>;
fn checked_rem(self, rhs: Self) -> Option<Self>;

fn do_round_div(self, rhs: Self, mul: Self) -> Option<Self>;

fn min_for_precision(precision: u8) -> Self;
fn max_for_precision(precision: u8) -> Self;

Expand Down Expand Up @@ -443,6 +445,16 @@ impl Decimal for i128 {
self.checked_rem(rhs)
}

fn do_round_div(self, rhs: Self, mul: Self) -> Option<Self> {
if self.is_negative() == rhs.is_negative() {
let res = (i256::from(self) * i256::from(mul) + i256::from(rhs) / 2) / i256::from(rhs);
Some(*res.low())
} else {
let res = (i256::from(self) * i256::from(mul) - i256::from(rhs) / 2) / i256::from(rhs);
Some(*res.low())
}
}

fn min_for_precision(to_precision: u8) -> Self {
MIN_DECIMAL_FOR_EACH_PRECISION[to_precision as usize - 1]
}
Expand Down Expand Up @@ -647,6 +659,14 @@ impl Decimal for i256 {
self.checked_rem(rhs)
}

fn do_round_div(self, rhs: Self, mul: Self) -> Option<Self> {
if self.is_negative() == rhs.is_negative() {
self.checked_mul(mul).map(|x| (x + rhs / 2) / rhs)
} else {
self.checked_mul(mul).map(|x| (x - rhs / 2) / rhs)
}
}

fn min_for_precision(to_precision: u8) -> Self {
MIN_DECIMAL256_BYTES_FOR_EACH_PRECISION[to_precision as usize - 1]
}
Expand Down
14 changes: 10 additions & 4 deletions src/query/functions/src/scalars/decimal/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ macro_rules! binary_decimal {
let scale_a = $left.scale();
let scale_b = $right.scale();


// Note: the result scale is always larger than the left scale
let scale_mul = scale_b + $size.scale - scale_a;
let multiplier = T::e(scale_mul as u32);
Expand All @@ -102,10 +101,17 @@ macro_rules! binary_decimal {
if std::intrinsics::unlikely(b == zero) {
ctx.set_error(result.len(), "divided by zero");
result.push(one);
} else if a.is_negative() == b.is_negative() {
result.push((a * multiplier + b / 2).div(b));
} else {
result.push((a * multiplier - b / 2).div(b));
match a.do_round_div(b, multiplier) {
Some(t) => result.push(t),
None => {
ctx.set_error(
result.len(),
concat!("Decimal overflow at line : ", line!()),
);
result.push(one);
}
}
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,10 @@ SELECT CAST(987654321.34 AS DECIMAL(76, 2)) / CAST(1.23 AS DECIMAL(76, 2)) AS re
----
802970992.95934959

query I
SELECT 404.754480000000000000000001 / 563.653044520000000000000001, 404.754480000000000000000000 / 563.653044520000000000000000;
----
0.718091535094401799683905 0.718091535094401799683905

## negative

Expand Down Expand Up @@ -1114,5 +1118,16 @@ select cast(b as int), cast(c as int), cast(d as int) from t
-1 -1 -1
-2 -2 -2

statement ok
create table decimal_test2(a decimal(28,8), b decimal(24,16));

statement ok
insert into decimal_test2 values(300.07878791,5325.0000000000000000),(2.00000000,10491.0000000000000000);

query I
select sum(a * b) / sum(a * b), sum(a + b) / sum(a + b), sum(a - b) / sum(a - b) from decimal_test2;
----
1.000000000000000000000000 1.0000000000000000 1.0000000000000000

statement ok
drop database decimal_t;

0 comments on commit b256d3d

Please sign in to comment.