chore(query): fix

databendlabs · Sep 25, 2023 · a6abcb3 · a6abcb3
1 parent 4be9ef9
commit a6abcb3
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 34 deletions.
diff --git a/src/query/expression/src/types/decimal.rs b/src/query/expression/src/types/decimal.rs
@@ -690,34 +690,35 @@ impl DecimalDataType {
         other.max_precision()
     }
 
-    // For div ops, we unify types to a super type
-    pub fn div_common_type(a: &Self, b: &Self) -> Result<(Self, Self)> {
-        let l: u8 = (a.leading_digits() + b.scale()).max(b.leading_digits());
-        let scale = a.scale().max((a.scale() + 6).min(12));
-
-        let mut precision = l + scale;
+    pub fn belong_diff_precision(precision_a: u8, precision_b: u8) -> bool {
+        (precision_a > MAX_DECIMAL128_PRECISION && precision_b <= MAX_DECIMAL128_PRECISION)
+            || (precision_a <= MAX_DECIMAL128_PRECISION && precision_b > MAX_DECIMAL128_PRECISION)
+    }
 
-        // if the args both are Decimal128, we need to clamp the precision to 38
-        if a.precision() <= MAX_DECIMAL128_PRECISION && b.precision() <= MAX_DECIMAL128_PRECISION {
-            precision = precision.min(MAX_DECIMAL128_PRECISION);
-        }
-        precision = precision.min(MAX_DECIMAL256_PRECISION);
+    // For div ops, a,b and return must belong to same width types
+    pub fn div_common_type(a: &Self, b: &Self, return_size: DecimalSize) -> Result<(Self, Self)> {
+        let precision_a = if Self::belong_diff_precision(a.precision(), return_size.precision) {
+            return_size.precision
+        } else {
+            a.precision()
+        };
 
-        let a_type = Self::from_size(DecimalSize { precision, scale })?;
+        let precision_b = if Self::belong_diff_precision(b.precision(), return_size.precision) {
+            return_size.precision
+        } else {
+            b.precision()
+        };
 
-        let mut b_precision = b.precision();
-        if b_precision <= MAX_DECIMAL128_PRECISION && a_type.precision() > MAX_DECIMAL128_PRECISION
-        {
-            b_precision = a_type.precision();
-        }
+        let a_type = Self::from_size(DecimalSize {
+            precision: precision_a,
+            scale: a.scale(),
+        })?;
+        let b_type = Self::from_size(DecimalSize {
+            precision: precision_b,
+            scale: b.scale(),
+        })?;
 
-        Ok((
-            a_type,
-            Self::from_size(DecimalSize {
-                precision: b_precision,
-                scale: b.scale(),
-            })?,
-        ))
+        Ok((a_type, b_type))
     }
 
     // Returns binded types and result type
@@ -752,9 +753,14 @@ impl DecimalDataType {
         // if the args both are Decimal128, we need to clamp the precision to 38
         if a.precision() <= MAX_DECIMAL128_PRECISION && b.precision() <= MAX_DECIMAL128_PRECISION {
             precision = precision.min(MAX_DECIMAL128_PRECISION);
+        } else if precision <= MAX_DECIMAL128_PRECISION
+            && Self::belong_diff_precision(a.precision(), b.precision())
+        {
+            // lift up to decimal256
+            precision = MAX_DECIMAL128_PRECISION + 1;
         }
-
         precision = precision.min(MAX_DECIMAL256_PRECISION);
+
         let result_type = Self::from_size(DecimalSize { precision, scale })?;
 
         if is_multiply {
@@ -770,7 +776,7 @@ impl DecimalDataType {
                 result_type,
             ))
         } else if is_divide {
-            let (a, b) = Self::div_common_type(a, b)?;
+            let (a, b) = Self::div_common_type(a, b, result_type.size())?;
             Ok((a, b, result_type))
         } else {
             Ok((result_type, result_type, result_type))

diff --git a/src/query/functions/tests/it/scalars/testdata/arithmetic.txt b/src/query/functions/tests/it/scalars/testdata/arithmetic.txt
@@ -1035,7 +1035,7 @@ evaluation (internal):
 
 ast            : c / 0.5
 raw expr       : divide(c::UInt32, 0.5)
-checked expr   : divide<Decimal(17, 6), Decimal(1, 1)>(to_decimal<UInt32>(17, 6)(c), 0.5_d128(1,1))
+checked expr   : divide<Decimal(10, 0), Decimal(1, 1)>(to_decimal<UInt32>(10, 0)(c), 0.5_d128(1,1))
 evaluation:
 +--------+-----------+----------------+
 |        | c         | Output         |
@@ -1128,13 +1128,13 @@ error:
   --> SQL:1:5
   |
 1 | 2.0 / 0
-  |     ^ divided by zero while evaluating function `divide(2.0000000, 0)`
+  |     ^ divided by zero while evaluating function `divide(2.0, 0)`
 
 
 
 ast            : c / e
 raw expr       : divide(c::UInt32, e::Decimal(10, 1))
-checked expr   : divide<Decimal(17, 6), Decimal(10, 1)>(to_decimal<UInt32>(17, 6)(c), e)
+checked expr   : divide<Decimal(10, 0), Decimal(10, 1)>(to_decimal<UInt32>(10, 0)(c), e)
 evaluation:
 +--------+-----------+----------------+----------------+
 |        | c         | e              | Output         |
@@ -1180,7 +1180,7 @@ evaluation (internal):
 
 ast            : d2 / e
 raw expr       : divide(d2::UInt8 NULL, e::Decimal(10, 1))
-checked expr   : divide<Decimal(15, 6) NULL, Decimal(10, 1) NULL>(CAST(d2 AS Decimal(15, 6) NULL), CAST(e AS Decimal(10, 1) NULL))
+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))
 evaluation:
 +--------+------------------+----------------+---------------------+
 |        | d2               | e              | Output              |
@@ -1203,12 +1203,12 @@ evaluation (internal):
 
 ast            : d2 / f
 raw expr       : divide(d2::UInt8 NULL, f::Decimal(76, 2))
-checked expr   : divide<Decimal(76, 6) NULL, Decimal(76, 2) NULL>(CAST(d2 AS Decimal(76, 6) NULL), CAST(f AS Decimal(76, 2) NULL))
+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))
 evaluation:
 +--------+------------------+----------------+---------------------+
 |        | d2               | f              | Output              |
 +--------+------------------+----------------+---------------------+
-| Type   | UInt8 NULL       | Decimal(76, 2) | Decimal(11, 6) NULL |
+| Type   | UInt8 NULL       | Decimal(76, 2) | Decimal(39, 6) NULL |
 | Domain | {0..=3} ∪ {NULL} | {0.50..=12.34} | Unknown             |
 | Row 0  | 1                | 0.50           | 2.000000            |
 | Row 1  | NULL             | 0.92           | NULL                |
@@ -1226,12 +1226,12 @@ evaluation (internal):
 
 ast            : e / f
 raw expr       : divide(e::Decimal(10, 1), f::Decimal(76, 2))
-checked expr   : divide<Decimal(76, 7), Decimal(76, 2)>(to_decimal<Decimal(10, 1)>(76, 7)(e), f)
+checked expr   : divide<Decimal(39, 1), Decimal(76, 2)>(to_decimal<Decimal(10, 1)>(39, 1)(e), f)
 evaluation:
 +--------+----------------+----------------+----------------+
 |        | e              | f              | Output         |
 +--------+----------------+----------------+----------------+
-| Type   | Decimal(10, 1) | Decimal(76, 2) | Decimal(18, 7) |
+| Type   | Decimal(10, 1) | Decimal(76, 2) | Decimal(39, 7) |
 | Domain | {3.1..=188.8}  | {0.50..=12.34} | Unknown        |
 | Row 0  | 3.1            | 0.50           | 6.2000000      |
 | Row 1  | 33.5           | 0.92           | 36.4130434     |