fix: Decimal append_scalar_value (#3412)

Author: a10y

encodings/decimal-byte-parts/src/decimal_byte_parts/compute/compare.rs

@@ -56,8 +56,8 @@ where
     P: NativePType + NumCast,
     ScalarValue: From<P>,
 {
-    match_each_decimal_value!(decimal_value, |$decimal_v| {
-        Some(ScalarValue::from(<P as NumCast>::from($decimal_v)?))
+    match_each_decimal_value!(decimal_value, |decimal_v| {
+        Some(ScalarValue::from(<P as NumCast>::from(decimal_v)?))
     })
 }
 

vortex-array/src/arrays/constant/canonical.rs

@@ -66,12 +66,8 @@ impl CanonicalVTable<ConstantVTable> for ConstantVTable {
                     return Ok(Canonical::Decimal(all_null));
                 };
 
-                let decimal_array = match_each_decimal_value!(value, |$V| {
-                   DecimalArray::new(
-                        Buffer::full(*$V, array.len()),
-                        *decimal_type,
-                        validity,
-                    )
+                let decimal_array = match_each_decimal_value!(value, |value| {
+                    DecimalArray::new(Buffer::full(*value, array.len()), *decimal_type, validity)
                 });
                 Canonical::Decimal(decimal_array)
             }

vortex-array/src/builders/mod.rs

@@ -46,11 +46,11 @@ pub use primitive::*;
 pub use struct_::*;
 pub use varbinview::*;
 use vortex_dtype::{DType, match_each_native_ptype};
-use vortex_error::{VortexExpect, VortexResult, vortex_bail, vortex_err};
+use vortex_error::{VortexResult, vortex_bail, vortex_err};
 use vortex_mask::Mask;
 use vortex_scalar::{
-    BinaryScalar, BoolScalar, ExtScalar, ListScalar, PrimitiveScalar, Scalar, ScalarValue,
-    StructScalar, Utf8Scalar, match_each_decimal_value_type,
+    BinaryScalar, BoolScalar, DecimalValue, ExtScalar, ListScalar, PrimitiveScalar, Scalar,
+    ScalarValue, StructScalar, Utf8Scalar, match_each_decimal_value, match_each_decimal_value_type,
 };
 
 use crate::arrays::smallest_storage_type;
@@ -210,18 +210,19 @@ pub trait ArrayBuilderExt: ArrayBuilder {
                         .append_option(PrimitiveScalar::try_from(scalar)?.typed_value::<P>())
                 })
             }
-            DType::Decimal(decimal_type, _) => {
-                match_each_decimal_value_type!(smallest_storage_type(decimal_type), |D| {
-                    self.as_any_mut()
-                        .downcast_mut::<DecimalBuilder>()
-                        .ok_or_else(|| {
-                            vortex_err!("Cannot append decimal scalar to non-decimal builder")
-                        })?
-                        .append_option(
-                            Option::<D>::try_from(scalar.as_decimal())
-                                .vortex_expect("decimal conversion failure"),
-                        )
-                })
+            DType::Decimal(..) => {
+                let builder = self
+                    .as_any_mut()
+                    .downcast_mut::<DecimalBuilder>()
+                    .ok_or_else(|| {
+                        vortex_err!("Cannot append decimal scalar to non-decimal builder")
+                    })?;
+                match scalar.as_decimal().decimal_value() {
+                    None => builder.append_null(),
+                    Some(v) => match_each_decimal_value!(v, |dec_val| {
+                        builder.append_value(*dec_val);
+                    }),
+                }
             }
             DType::Utf8(_) => self
                 .as_any_mut()

vortex-python/src/scalar/into_py.rs

@@ -169,12 +169,10 @@ fn decimal_value_to_py(
     let m = py.import("decimal")?;
     let decimal_class = m.getattr("Decimal")?;
 
-    match_each_decimal_value!(decimal_value, |$V| {
-       {
-            let (whole, decimal) = $V.decimal_parts(scale);
-            let repr = format!("{}.{:0>width$}", whole, decimal, width = scale as usize)
-                .into_pyobject(py)?;
-            decimal_class.call1((repr,))
-        }
+    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,))
     })
 }

vortex-scalar/src/decimal.rs

@@ -316,17 +316,34 @@ decimal_scalar_pack!(i256, i256, I256);
 
 #[macro_export]
 macro_rules! match_each_decimal_value {
-    ($self:expr, | $_:tt $value:ident | $($body:tt)*) => ({
-        macro_rules! __with__ {( $_ $value:ident ) => ( $($body)* )}
+    ($self:expr, | $value:ident | $body:block) => {{
         match $self {
-            DecimalValue::I8(v) => __with__! { v },
-            DecimalValue::I16(v) => __with__! { v },
-            DecimalValue::I32(v) => __with__! { v },
-            DecimalValue::I64(v) => __with__! { v },
-            DecimalValue::I128(v) => __with__! { v },
-            DecimalValue::I256(v) => __with__! { v },
+            DecimalValue::I8(v) => {
+                let $value = v;
+                $body
+            }
+            DecimalValue::I16(v) => {
+                let $value = v;
+                $body
+            }
+            DecimalValue::I32(v) => {
+                let $value = v;
+                $body
+            }
+            DecimalValue::I64(v) => {
+                let $value = v;
+                $body
+            }
+            DecimalValue::I128(v) => {
+                let $value = v;
+                $body
+            }
+            DecimalValue::I256(v) => {
+                let $value = v;
+                $body
+            }
         }
-    });
+    }};
 }
 
 /// Macro to match over each decimal value type, binding the corresponding native type (from `DecimalValueType`)