Chore: Validate Scalar::new for null and clean up DecimalScalar (#4435)

Progress towards #4378 

~~I don't think that it makes a lot of sense to add `new_unchecked`
given that the only check we really need is `!dtype.is_nullable()`
(which is cheap), and by making the fields private we can guarantee this
invariant.~~

Edit: In the future the `Scalar::new` should validate that the dtype
matches the value that is passed in.

Also cleans up the `decimal` module in `vortex-scalar`.

---------

Signed-off-by: Connor Tsui <[email protected]>

Author: connortsui20

vortex-array/src/builders/list.rs

@@ -123,6 +123,7 @@ impl<O: OffsetPType> ArrayBuilder for ListBuilder<O> {
 
     fn append_zeros(&mut self, n: usize) {
         let count = self.value_builder.len();
+        // TODO(connor): this is incorrect, as it creates lists of size 1 instead of 0.
         self.value_builder.append_zeros(n);
         for i in 0..n {
             self.append_index(

vortex-scalar/src/binary.rs

@@ -168,10 +168,10 @@ impl<'a> BinaryScalar<'a> {
 impl Scalar {
     /// Creates a new binary scalar from a byte buffer.
     pub fn binary(buffer: impl Into<ByteBuffer>, nullability: Nullability) -> Self {
-        Self {
-            dtype: DType::Binary(nullability),
-            value: ScalarValue(InnerScalarValue::Buffer(Arc::new(buffer.into()))),
-        }
+        Self::new(
+            DType::Binary(nullability),
+            ScalarValue(InnerScalarValue::Buffer(Arc::new(buffer.into()))),
+        )
     }
 }
 
@@ -184,7 +184,7 @@ impl<'a> TryFrom<&'a Scalar> for BinaryScalar<'a> {
         }
         Ok(Self {
             dtype: value.dtype(),
-            value: value.value.as_buffer()?,
+            value: value.value().as_buffer()?,
         })
     }
 }
@@ -238,19 +238,16 @@ impl From<&[u8]> for Scalar {
 
 impl From<ByteBuffer> for Scalar {
     fn from(value: ByteBuffer) -> Self {
-        Self {
-            dtype: DType::Binary(Nullability::NonNullable),
-            value: value.into(),
-        }
+        Self::new(DType::Binary(Nullability::NonNullable), value.into())
     }
 }
 
 impl From<Arc<ByteBuffer>> for Scalar {
     fn from(value: Arc<ByteBuffer>) -> Self {
-        Self {
-            dtype: DType::Binary(Nullability::NonNullable),
-            value: ScalarValue(InnerScalarValue::Buffer(value)),
-        }
+        Self::new(
+            DType::Binary(Nullability::NonNullable),
+            ScalarValue(InnerScalarValue::Buffer(value)),
+        )
     }
 }
 

vortex-scalar/src/bool.rs

@@ -111,7 +111,7 @@ impl<'a> TryFrom<&'a Scalar> for BoolScalar<'a> {
         }
         Ok(Self {
             dtype: value.dtype(),
-            value: value.value.as_bool()?,
+            value: value.value().as_bool()?,
         })
     }
 }

vortex-scalar/src/decimal/macros.rs

@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: Copyright the Vortex contributors
+
+/// Implements `NativeDecimalType` for the given type.
+macro_rules! impl_native_decimal_type {
+    ($T:ty, $variant:ident) => {
+        impl NativeDecimalType for $T {
+            const VALUES_TYPE: DecimalValueType = DecimalValueType::$variant;
+
+            fn maybe_from(decimal_type: DecimalValue) -> Option<Self> {
+                if let DecimalValue::$variant(v) = decimal_type {
+                    Some(v)
+                } else {
+                    None
+                }
+            }
+        }
+    };
+}
+pub(crate) use impl_native_decimal_type;
+
+/// Implements `TryFrom<DecimalScalar>` for the given type.
+macro_rules! decimal_scalar_unpack {
+    ($T:ident, $arm:ident) => {
+        impl TryFrom<DecimalScalar<'_>> for Option<$T> {
+            type Error = VortexError;
+
+            fn try_from(value: DecimalScalar) -> Result<Self, Self::Error> {
+                Ok(match value.value {
+                    None => None,
+                    Some(DecimalValue::$arm(v)) => Some(v),
+                    v => vortex_error::vortex_bail!(
+                        "Cannot extract decimal {:?} as {}",
+                        v,
+                        stringify!($T)
+                    ),
+                })
+            }
+        }
+
+        impl TryFrom<DecimalScalar<'_>> for $T {
+            type Error = VortexError;
+
+            fn try_from(value: DecimalScalar) -> Result<Self, Self::Error> {
+                match value.value {
+                    None => vortex_error::vortex_bail!("Cannot extract value from null decimal"),
+                    Some(DecimalValue::$arm(v)) => Ok(v),
+                    v => vortex_error::vortex_bail!(
+                        "Cannot extract decimal {:?} as {}",
+                        v,
+                        stringify!($T)
+                    ),
+                }
+            }
+        }
+    };
+}
+pub(crate) use decimal_scalar_unpack;
+
+/// Implements `Into<DecimalValue>` for the given type.
+macro_rules! decimal_scalar_pack {
+    ($from:ident, $to:ident, $arm:ident) => {
+        impl From<$from> for DecimalValue {
+            fn from(value: $from) -> Self {
+                DecimalValue::$arm(value as $to)
+            }
+        }
+    };
+}
+pub(crate) use decimal_scalar_pack;
+
+/// Matches over each decimal value variant, binding the inner value to a variable.
+///
+/// # Example
+///
+/// ```ignore
+/// match_each_decimal_value!(value, |v| {
+///     println!("Value: {}", v);
+/// });
+/// ```
+#[macro_export] // Used in `vortex-array`.
+macro_rules! match_each_decimal_value {
+    ($self:expr, | $value:ident | $body:block) => {{
+        match $self {
+            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`)
+#[macro_export] // Used in `vortex-array`.
+macro_rules! match_each_decimal_value_type {
+    ($self:expr, | $enc:ident | $body:block) => {{
+        use $crate::{DecimalValueType, i256};
+        match $self {
+            DecimalValueType::I8 => {
+                type $enc = i8;
+                $body
+            }
+            DecimalValueType::I16 => {
+                type $enc = i16;
+                $body
+            }
+            DecimalValueType::I32 => {
+                type $enc = i32;
+                $body
+            }
+            DecimalValueType::I64 => {
+                type $enc = i64;
+                $body
+            }
+            DecimalValueType::I128 => {
+                type $enc = i128;
+                $body
+            }
+            DecimalValueType::I256 => {
+                type $enc = i256;
+                $body
+            }
+            ty => unreachable!("unknown decimal value type {:?}", ty),
+        }
+    }};
+}