fix(python): build Decimal scalars from the unscaled integer and an exponent

vortex-python/src/scalar/into_py.rs

@@ -1,8 +1,6 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
-use std::cmp::Ordering;
-
 use pyo3::Bound;
 use pyo3::IntoPyObject;
 use pyo3::PyAny;
@@ -22,7 +20,6 @@ use vortex::buffer::ByteBuffer;
 use vortex::dtype::DType;
 use vortex::dtype::PType;
 use vortex::dtype::half::f16;
-use vortex::dtype::i256;
 use vortex::error::VortexExpect;
 use vortex::error::vortex_err;
 use vortex::scalar::DecimalValue;
@@ -151,70 +148,23 @@ impl<'py> IntoPyObject<'py> for PyVortex<ListScalar<'_>> {
     }
 }
 
-trait DecimalIntoParts: Sized {
-    /// Split an integer encoding a decimal with the given `scale` into a
-    /// (whole number, decimal) parts.
-    ///
-    /// For example, for the number 123i128 and scale 2, this will return returns (1, 23).
-    fn decimal_parts(self, scale: i8) -> (Self, Self);
-}
-
-macro_rules! impl_decimal_into_parts {
-    ($ty:ident, $ten:expr) => {
-        impl DecimalIntoParts for $ty {
-            fn decimal_parts(self, scale: i8) -> (Self, Self) {
-                let scale_factor = $ten.pow(scale.unsigned_abs() as u32);
-                match scale.cmp(&0) {
-                    Ordering::Equal => (self, 0),
-                    Ordering::Less => {
-                        // Negative scale -> apply the given number of trailing zeros
-                        (self * scale_factor, 0)
-                    }
-                    Ordering::Greater => {
-                        // Positive scale -> extract the leading/trailing digits separately.
-                        (self / scale_factor, self % scale_factor)
-                    }
-                }
-            }
-        }
-    };
-}
-
-impl_decimal_into_parts!(i8, 10i8);
-impl_decimal_into_parts!(i16, 10i16);
-impl_decimal_into_parts!(i32, 10i32);
-impl_decimal_into_parts!(i64, 10i64);
-impl_decimal_into_parts!(i128, 10i128);
-
-impl DecimalIntoParts for i256 {
-    fn decimal_parts(self, scale: i8) -> (Self, Self) {
-        match scale.cmp(&0) {
-            Ordering::Equal => (self, i256::ZERO),
-            Ordering::Less => {
-                // Negative scale -> apply the given number of trailing zeros
-                let scale_factor = i256::from_i128(10).wrapping_pow(-scale as u32);
-                (self * scale_factor, i256::ZERO)
-            }
-            Ordering::Greater => {
-                // Positive scale -> extract the leading/trailing digits separately.
-                let scale_factor = i256::from_i128(10).wrapping_pow(scale as u32);
-                (self / scale_factor, self % scale_factor)
-            }
-        }
-    }
-}
-
 fn decimal_value_to_py(
     py: Python,
     scale: i8,
     decimal_value: DecimalValue,
 ) -> PyResult<Bound<PyAny>> {
     let decimal_class = decimal_class(py)?;
 
-    match_each_decimal_value!(decimal_value, |value| {
-        let (whole, decimal) = value.decimal_parts(scale);
-        let repr =
-            format!("{}.{:0>width$}", whole, decimal, width = scale as usize).into_pyobject(py)?;
-        decimal_class.call1((repr,))
-    })
+    // Hand `Decimal` the unscaled integer and an exponent rather than splitting the value into
+    // whole and fractional digits here. Splitting has to reproduce sign handling, zero padding and
+    // the negative-scale case, and `Decimal` already does all three: it parses this form exactly
+    // (the context precision bounds arithmetic, not construction) and the value it returns carries
+    // an exponent of exactly `-scale`, so the dtype's scale survives the round trip.
+    //
+    // The exponent is negated through `i16` because `-i8::MIN` overflows, and `MIN_SCALE` is not
+    // bounded away from `i8::MIN`.
+    let repr = match_each_decimal_value!(decimal_value, |value| {
+        format!("{value}E{}", -i16::from(scale))
+    });
+    decimal_class.call1((repr.into_pyobject(py)?,))
 }

vortex-python/test/test_scalar.py

@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: Apache-2.0
 # SPDX-FileCopyrightText: Copyright the Vortex contributors
 
+import decimal
+
 import pytest
 
 import vortex as vx
@@ -35,3 +37,51 @@ def test_f16() -> None:
     scalar = vx.scalar(1.0, dtype=vx.float_(16))
     assert scalar.dtype == vx.float_(16)
     assert scalar.as_py() == 1.0
+
+
+@pytest.mark.parametrize(
+    "precision,scale,stored,expected",
+    [
+        (10, 2, 12345, "123.45"),
+        # A negative stored value used to render as "-123.-45", which Decimal refuses.
+        (10, 2, -12345, "-123.45"),
+        # Truncating division put the whole part at 0 and left the sign on the fraction.
+        (10, 2, -5, "-0.05"),
+        (10, 2, 5, "0.05"),
+        # The stored value picks the narrowest storage that holds it, so a small value at an
+        # everyday scale reached `10i8.pow(3)`, which overflows an i8.
+        (10, 3, 5, "0.005"),
+        (10, 5, 5, "0.00005"),
+        (10, 3, -5, "-0.005"),
+        # Scale 0 has no fractional digits, so the exponent has to be 0 and not -1.
+        (10, 0, -7, "-7"),
+        # A negative scale means trailing zeros before the point, so the exponent is positive.
+        (5, -5, 1, "1E+5"),
+        (5, -5, -1, "-1E+5"),
+        (1, -128, 1, "1E+128"),
+        # Above scale 38 no storage width holds the factor, i128 included.
+        (76, 39, 1, "1E-39"),
+        (76, 76, -1, "-1E-76"),
+    ],
+)
+def test_decimal_round_trip(precision: int, scale: int, stored: int, expected: str) -> None:
+    scalar = vx.scalar(stored, dtype=vx.decimal(precision=precision, scale=scale))
+
+    value = scalar.as_py()
+    assert isinstance(value, decimal.Decimal)
+    # Compare the string form, not just the numeric value: Decimal("-7") == Decimal("-7.0"), so an
+    # equality check alone would not pin the exponent to the dtype's scale.
+    assert str(value) == expected
+    assert value.as_tuple().exponent == -scale
+
+
+def test_decimal_ignores_context_precision() -> None:
+    """A wide decimal must survive conversion whatever the ambient context precision is."""
+    digits = "9" * 38
+    with decimal.localcontext() as ctx:
+        ctx.prec = 3
+        scalar = vx.scalar(int(digits), dtype=vx.decimal(precision=38, scale=19))
+        value = scalar.as_py()
+    assert isinstance(value, decimal.Decimal)
+    assert len(value.as_tuple().digits) == 38
+    assert str(value) == f"{digits[:19]}.{digits[19:]}"