fix(query): fix decimal op loss precision (#15313)

databendlabs · Apr 23, 2024 · 541b8e9 · 541b8e9
1 parent da63bb6
commit 541b8e9
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 37 deletions.
diff --git a/src/query/functions/src/scalars/decimal/arithmetic.rs b/src/query/functions/src/scalars/decimal/arithmetic.rs
@@ -28,6 +28,9 @@ use databend_common_expression::FunctionRegistry;
 use databend_common_expression::FunctionSignature;
 use ethnum::i256;
 
+use super::convert_to_decimal;
+use super::convert_to_decimal_domain;
+
 #[derive(Copy, Clone, Debug)]
 enum ArithmeticOp {
     Plus,
@@ -267,16 +270,13 @@ macro_rules! register_decimal_binary_op {
             let function = Function {
                 signature: FunctionSignature {
                     name: format!("{:?}", $arithmetic_op).to_lowercase(),
-                    args_type: vec![
-                        DataType::Decimal(left.clone()),
-                        DataType::Decimal(right.clone()),
-                    ],
+                    args_type: args_type.clone(),
                     return_type: DataType::Decimal(return_decimal_type),
                 },
                 eval: FunctionEval::Scalar {
-                    calc_domain: Box::new(move |_ctx, d| {
-                        let lhs = d[0].as_decimal();
-                        let rhs = d[1].as_decimal();
+                    calc_domain: Box::new(move |ctx, d| {
+                        let lhs = convert_to_decimal_domain(ctx, d[0].clone(), left.clone());
+                        let rhs = convert_to_decimal_domain(ctx, d[1].clone(), right.clone());
 
                         if lhs.is_none() || rhs.is_none() {
                             return FunctionDomain::Full;
@@ -308,9 +308,15 @@ macro_rules! register_decimal_binary_op {
                         .unwrap_or($default_domain)
                     }),
                     eval: Box::new(move |args, ctx| {
+                        let a = convert_to_decimal(&args[0], ctx, &args_type[0], left);
+                        let b = convert_to_decimal(&args[1], ctx, &args_type[1], right);
+
+                        let a = a.as_ref();
+                        let b = b.as_ref();
+
                         let res = op_decimal!(
-                            &args[0],
-                            &args[1],
+                            &a,
+                            &b,
                             ctx,
                             left,
                             right,

diff --git a/src/query/functions/src/scalars/decimal/cast.rs b/src/query/functions/src/scalars/decimal/cast.rs
@@ -328,7 +328,7 @@ fn decimal_to_string(
     })
 }
 
-fn convert_to_decimal(
+pub fn convert_to_decimal(
     arg: &ValueRef<AnyType>,
     ctx: &mut EvalContext,
     from_type: &DataType,
@@ -387,7 +387,7 @@ fn convert_to_decimal(
     })
 }
 
-fn convert_to_decimal_domain(
+pub fn convert_to_decimal_domain(
     func_ctx: &FunctionContext,
     domain: Domain,
     dest_type: DecimalDataType,

diff --git a/src/query/functions/src/scalars/decimal/mod.rs b/src/query/functions/src/scalars/decimal/mod.rs
@@ -18,6 +18,8 @@ mod comparison;
 mod math;
 
 pub(crate) use arithmetic::register_decimal_arithmetic;
+pub(crate) use cast::convert_to_decimal;
+pub(crate) use cast::convert_to_decimal_domain;
 pub(crate) use cast::register_decimal_to_float;
 pub(crate) use cast::register_decimal_to_int;
 pub(crate) use cast::register_decimal_to_string;

diff --git a/src/query/functions/tests/it/scalars/testdata/arithmetic.txt b/src/query/functions/tests/it/scalars/testdata/arithmetic.txt
@@ -69,8 +69,7 @@ evaluation (internal):
 
 ast            : c + 0.5
 raw expr       : plus(c::UInt32, 0.5)
-checked expr   : plus<Decimal(12, 1), Decimal(12, 1)>(to_decimal<UInt32>(12, 1)(c), to_decimal<Decimal(1, 1)>(12, 1)(0.5_d128(1,1)))
-optimized expr : plus<Decimal(12, 1), Decimal(12, 1)>(to_decimal<UInt32>(12, 1)(c), 0.5_d128(12,1))
+checked expr   : plus<UInt32, Decimal(1, 1)>(c, 0.5_d128(1,1))
 evaluation:
 +--------+-----------+----------------+
 |        | c         | Output         |
@@ -138,7 +137,7 @@ evaluation (internal):
 
 ast            : c + e
 raw expr       : plus(c::UInt32, e::Decimal(10, 1))
-checked expr   : plus<Decimal(12, 1), Decimal(12, 1)>(to_decimal<UInt32>(12, 1)(c), to_decimal<Decimal(10, 1)>(12, 1)(e))
+checked expr   : plus<UInt32, Decimal(10, 1)>(c, e)
 evaluation:
 +--------+-----------+----------------+----------------+
 |        | c         | e              | Output         |
@@ -184,7 +183,7 @@ evaluation (internal):
 
 ast            : d2 + e
 raw expr       : plus(d2::UInt8 NULL, e::Decimal(10, 1))
-checked expr   : plus<Decimal(11, 1) NULL, Decimal(11, 1) NULL>(CAST(d2 AS Decimal(11, 1) NULL), CAST(e AS Decimal(11, 1) NULL))
+checked expr   : plus<UInt8 NULL, Decimal(10, 1) NULL>(d2, CAST(e AS Decimal(10, 1) NULL))
 evaluation:
 +--------+------------------+----------------+------------------------+
 |        | d2               | e              | Output                 |
@@ -207,7 +206,7 @@ evaluation (internal):
 
 ast            : d2 + f
 raw expr       : plus(d2::UInt8 NULL, f::Decimal(76, 2))
-checked expr   : plus<Decimal(76, 2) NULL, Decimal(76, 2) NULL>(CAST(d2 AS Decimal(76, 2) NULL), CAST(f AS Decimal(76, 2) NULL))
+checked expr   : plus<UInt8 NULL, Decimal(76, 2) NULL>(d2, CAST(f AS Decimal(76, 2) NULL))
 evaluation:
 +--------+------------------+----------------+-------------------------+
 |        | d2               | f              | Output                  |
@@ -230,7 +229,7 @@ evaluation (internal):
 
 ast            : e + f
 raw expr       : plus(e::Decimal(10, 1), f::Decimal(76, 2))
-checked expr   : plus<Decimal(76, 2), Decimal(76, 2)>(to_decimal<Decimal(10, 1)>(76, 2)(e), f)
+checked expr   : plus<Decimal(10, 1), Decimal(76, 2)>(e, f)
 evaluation:
 +--------+----------------+----------------+-----------------+
 |        | e              | f              | Output          |
@@ -322,8 +321,7 @@ evaluation (internal):
 
 ast            : c - 0.5
 raw expr       : minus(c::UInt32, 0.5)
-checked expr   : minus<Decimal(12, 1), Decimal(12, 1)>(to_decimal<UInt32>(12, 1)(c), to_decimal<Decimal(1, 1)>(12, 1)(0.5_d128(1,1)))
-optimized expr : minus<Decimal(12, 1), Decimal(12, 1)>(to_decimal<UInt32>(12, 1)(c), 0.5_d128(12,1))
+checked expr   : minus<UInt32, Decimal(1, 1)>(c, 0.5_d128(1,1))
 evaluation:
 +--------+-----------+----------------+
 |        | c         | Output         |
@@ -391,7 +389,7 @@ evaluation (internal):
 
 ast            : c - e
 raw expr       : minus(c::UInt32, e::Decimal(10, 1))
-checked expr   : minus<Decimal(12, 1), Decimal(12, 1)>(to_decimal<UInt32>(12, 1)(c), to_decimal<Decimal(10, 1)>(12, 1)(e))
+checked expr   : minus<UInt32, Decimal(10, 1)>(c, e)
 evaluation:
 +--------+-----------+----------------+-----------------+
 |        | c         | e              | Output          |
@@ -437,7 +435,7 @@ evaluation (internal):
 
 ast            : d2 - e
 raw expr       : minus(d2::UInt8 NULL, e::Decimal(10, 1))
-checked expr   : minus<Decimal(11, 1) NULL, Decimal(11, 1) NULL>(CAST(d2 AS Decimal(11, 1) NULL), CAST(e AS Decimal(11, 1) NULL))
+checked expr   : minus<UInt8 NULL, Decimal(10, 1) NULL>(d2, CAST(e AS Decimal(10, 1) NULL))
 evaluation:
 +--------+------------------+----------------+--------------------------+
 |        | d2               | e              | Output                   |
@@ -460,7 +458,7 @@ evaluation (internal):
 
 ast            : d2 - f
 raw expr       : minus(d2::UInt8 NULL, f::Decimal(76, 2))
-checked expr   : minus<Decimal(76, 2) NULL, Decimal(76, 2) NULL>(CAST(d2 AS Decimal(76, 2) NULL), CAST(f AS Decimal(76, 2) NULL))
+checked expr   : minus<UInt8 NULL, Decimal(76, 2) NULL>(d2, CAST(f AS Decimal(76, 2) NULL))
 evaluation:
 +--------+------------------+----------------+--------------------------+
 |        | d2               | f              | Output                   |
@@ -483,7 +481,7 @@ evaluation (internal):
 
 ast            : e - f
 raw expr       : minus(e::Decimal(10, 1), f::Decimal(76, 2))
-checked expr   : minus<Decimal(76, 2), Decimal(76, 2)>(to_decimal<Decimal(10, 1)>(76, 2)(e), f)
+checked expr   : minus<Decimal(10, 1), Decimal(76, 2)>(e, f)
 evaluation:
 +--------+----------------+----------------+------------------+
 |        | e              | f              | Output           |
@@ -759,8 +757,7 @@ evaluation (internal):
 
 ast            : c * 0.5
 raw expr       : multiply(c::UInt32, 0.5)
-checked expr   : multiply<Decimal(11, 0), Decimal(11, 1)>(to_decimal<UInt32>(11, 0)(c), to_decimal<Decimal(1, 1)>(11, 1)(0.5_d128(1,1)))
-optimized expr : multiply<Decimal(11, 0), Decimal(11, 1)>(to_decimal<UInt32>(11, 0)(c), 0.5_d128(11,1))
+checked expr   : multiply<UInt32, Decimal(1, 1)>(c, 0.5_d128(1,1))
 evaluation:
 +--------+-----------+----------------+
 |        | c         | Output         |
@@ -828,7 +825,7 @@ evaluation (internal):
 
 ast            : c * e
 raw expr       : multiply(c::UInt32, e::Decimal(10, 1))
-checked expr   : multiply<Decimal(20, 0), Decimal(20, 1)>(to_decimal<UInt32>(20, 0)(c), to_decimal<Decimal(10, 1)>(20, 1)(e))
+checked expr   : multiply<UInt32, Decimal(10, 1)>(c, e)
 evaluation:
 +--------+-----------+----------------+-----------------+
 |        | c         | e              | Output          |
@@ -874,7 +871,7 @@ evaluation (internal):
 
 ast            : d2 * e
 raw expr       : multiply(d2::UInt8 NULL, e::Decimal(10, 1))
-checked expr   : multiply<Decimal(13, 0) NULL, Decimal(13, 1) NULL>(CAST(d2 AS Decimal(13, 0) NULL), CAST(e AS Decimal(13, 1) NULL))
+checked expr   : multiply<UInt8 NULL, Decimal(10, 1) NULL>(d2, CAST(e AS Decimal(10, 1) NULL))
 evaluation:
 +--------+------------------+----------------+------------------------+
 |        | d2               | e              | Output                 |
@@ -897,7 +894,7 @@ evaluation (internal):
 
 ast            : d2 * f
 raw expr       : multiply(d2::UInt8 NULL, f::Decimal(76, 2))
-checked expr   : multiply<Decimal(76, 0) NULL, Decimal(76, 2) NULL>(CAST(d2 AS Decimal(76, 0) NULL), CAST(f AS Decimal(76, 2) NULL))
+checked expr   : multiply<UInt8 NULL, Decimal(76, 2) NULL>(d2, CAST(f AS Decimal(76, 2) NULL))
 evaluation:
 +--------+------------------+----------------+-------------------------+
 |        | d2               | f              | Output                  |
@@ -920,7 +917,7 @@ evaluation (internal):
 
 ast            : e * f
 raw expr       : multiply(e::Decimal(10, 1), f::Decimal(76, 2))
-checked expr   : multiply<Decimal(76, 1), Decimal(76, 2)>(to_decimal<Decimal(10, 1)>(76, 1)(e), f)
+checked expr   : multiply<Decimal(10, 1), Decimal(76, 2)>(e, f)
 evaluation:
 +--------+----------------+----------------+--------------------+
 |        | e              | f              | Output             |
@@ -943,8 +940,7 @@ evaluation (internal):
 
 ast            : e * 0.5
 raw expr       : multiply(e::Decimal(10, 1), 0.5)
-checked expr   : multiply<Decimal(11, 1), Decimal(11, 1)>(to_decimal<Decimal(10, 1)>(11, 1)(e), to_decimal<Decimal(1, 1)>(11, 1)(0.5_d128(1,1)))
-optimized expr : multiply<Decimal(11, 1), Decimal(11, 1)>(to_decimal<Decimal(10, 1)>(11, 1)(e), 0.5_d128(11,1))
+checked expr   : multiply<Decimal(10, 1), Decimal(1, 1)>(e, 0.5_d128(1,1))
 evaluation:
 +--------+----------------+----------------+
 |        | e              | Output         |
@@ -1035,7 +1031,7 @@ evaluation (internal):
 
 ast            : c / 0.5
 raw expr       : divide(c::UInt32, 0.5)
-checked expr   : divide<Decimal(10, 0), Decimal(1, 1)>(to_decimal<UInt32>(10, 0)(c), 0.5_d128(1,1))
+checked expr   : divide<UInt32, Decimal(1, 1)>(c, 0.5_d128(1,1))
 evaluation:
 +--------+-----------+----------------+
 |        | c         | Output         |
@@ -1134,7 +1130,7 @@ error:
 
 ast            : c / e
 raw expr       : divide(c::UInt32, e::Decimal(10, 1))
-checked expr   : divide<Decimal(10, 0), Decimal(10, 1)>(to_decimal<UInt32>(10, 0)(c), e)
+checked expr   : divide<UInt32, Decimal(10, 1)>(c, e)
 evaluation:
 +--------+-----------+----------------+----------------+
 |        | c         | e              | Output         |
@@ -1180,7 +1176,7 @@ evaluation (internal):
 
 ast            : d2 / e
 raw expr       : divide(d2::UInt8 NULL, e::Decimal(10, 1))
-checked expr   : divide<Decimal(3, 0) NULL, Decimal(10, 1) NULL>(CAST(d2 AS Decimal(3, 0) NULL), CAST(e AS Decimal(10, 1) NULL))
+checked expr   : divide<UInt8 NULL, Decimal(10, 1) NULL>(d2, CAST(e AS Decimal(10, 1) NULL))
 evaluation:
 +--------+------------------+----------------+---------------------+
 |        | d2               | e              | Output              |
@@ -1203,7 +1199,7 @@ evaluation (internal):
 
 ast            : d2 / f
 raw expr       : divide(d2::UInt8 NULL, f::Decimal(76, 2))
-checked expr   : divide<Decimal(39, 0) NULL, Decimal(76, 2) NULL>(CAST(d2 AS Decimal(39, 0) NULL), CAST(f AS Decimal(76, 2) NULL))
+checked expr   : divide<UInt8 NULL, Decimal(76, 2) NULL>(d2, CAST(f AS Decimal(76, 2) NULL))
 evaluation:
 +--------+------------------+----------------+---------------------+
 |        | d2               | f              | Output              |
@@ -1226,7 +1222,7 @@ evaluation (internal):
 
 ast            : e / f
 raw expr       : divide(e::Decimal(10, 1), f::Decimal(76, 2))
-checked expr   : divide<Decimal(39, 1), Decimal(76, 2)>(to_decimal<Decimal(10, 1)>(39, 1)(e), f)
+checked expr   : divide<Decimal(10, 1), Decimal(76, 2)>(e, f)
 evaluation:
 +--------+----------------+----------------+----------------+
 |        | e              | f              | Output         |

diff --git a/src/query/functions/tests/it/scalars/testdata/math.txt b/src/query/functions/tests/it/scalars/testdata/math.txt
@@ -464,7 +464,7 @@ output         : 120
 
 ast            : truncate(0)(10.28*100, 0)
 raw expr       : truncate(0)(multiply(10.28, 100), 0)
-checked expr   : truncate<Decimal(7, 2), UInt8>(0)(multiply<Decimal(7, 2), Decimal(7, 0)>(to_decimal<Decimal(4, 2)>(7, 2)(10.28_d128(4,2)), to_decimal<UInt8>(7, 0)(100_u8)), 0_u8)
+checked expr   : truncate<Decimal(7, 2), UInt8>(0)(multiply<Decimal(4, 2), UInt8>(10.28_d128(4,2), 100_u8), 0_u8)
 optimized expr : 1028_d128(7,0)
 output type    : Decimal(7, 0)
 output domain  : {1028..=1028}

diff --git a/tests/sqllogictests/suites/query/functions/02_0000_function_arithmetic.test b/tests/sqllogictests/suites/query/functions/02_0000_function_arithmetic.test
@@ -146,3 +146,25 @@ query I
 select * from numbers(4) where -number > -1;
 ----
 0
+
+
+## decimal
+
+statement ok
+create table test3 (
+	amount1 DECIMAL(38, 18) NULL,
+	amount2 DECIMAL(28, 8) NULL
+);
+
+statement ok
+insert into test3 values('30.606168460000000000','30.60616846');
+
+
+query TTTTTT
+select sum(amount1)a ,  sum(amount2) b , a  +  b, a - b, a * b, a / b from test3;
+----
+30.606168460000000000 30.60616846 61.212336920000000000 0.000000000000000000 936.73754780189877160000000000 1.000000000000000000
+
+statement ok
+drop table test3
+