Add shared conversion to canonical decimal arrays or Widen

vortex-array/src/arrays/decimal/array.rs

@@ -59,20 +59,20 @@ pub struct DecimalSlots {
 ///
 /// Decimals are stored as scaled integers in a supported scalar value type.
 ///
-/// The precisions supported for each scalar type are:
+/// The canonical widths for each precision are:
 /// - **i8**: precision 1-2 digits
 /// - **i16**: precision 3-4 digits
 /// - **i32**: precision 5-9 digits
 /// - **i64**: precision 10-18 digits
 /// - **i128**: precision 19-38 digits
 /// - **i256**: precision 39-76 digits
 ///
-/// These are just the maximal ranges for each scalar type, but it is perfectly legal to store
-/// values with precision that does not match this exactly. For example, a valid DecimalArray with
-/// precision=39 may store its values in an `i8` if all of the actual values fit into it.
-///
-/// Similarly, a `DecimalArray` can be built that stores a set of precision=2 values in a
-/// `Buffer<i256>`.
+/// Storage width is still independent of precision during the canonical decimal migration.
+/// Existing `vortex.decimal` nodes and DecimalByteParts assembly can produce noncanonical storage.
+/// For example, an array with precision 39 can currently use an `i8` buffer when all values fit,
+/// while an array with precision 2 can use a `Buffer<i256>`. [`DecimalDataParts`] preserves the
+/// actual width for conversion into canonical decimal leaves, optionally under
+/// [`Widen`](crate::arrays::Widen).
 ///
 /// ## Precision and Scale
 ///
@@ -86,7 +86,7 @@ pub struct DecimalSlots {
 ///
 /// ## Valid Scalar Types
 ///
-/// The underlying storage uses these native types based on precision:
+/// Canonical decimal leaves select one of these native types from precision:
 /// - `DecimalType::I8`, `I16`, `I32`, `I64`, `I128`, `I256`
 /// - Type selection is automatic based on the required precision
 ///
@@ -111,6 +111,7 @@ pub struct DecimalSlots {
 pub struct DecimalData {
     pub(super) decimal_dtype: DecimalDType,
     pub(super) values: BufferHandle,
+    /// Actual storage type, including noncanonical widths still accepted during the migration.
     pub(super) values_type: DecimalType,
 }
 
@@ -124,10 +125,21 @@ impl Display for DecimalData {
     }
 }
 
+/// Decimal storage and validity, as returned by [`DecimalArray::into_data_parts`].
 pub struct DecimalDataParts {
+    /// The precision and scale of the values.
     pub decimal_dtype: DecimalDType,
+    /// Unscaled integers of type `values_type` in host or device memory.
     pub values: BufferHandle,
+    /// Actual storage type, retained to interpret input before canonical conversion.
+    ///
+    /// Legacy `vortex.decimal` nodes and DecimalByteParts assembly can use a different width from
+    /// [`DecimalDType::canonical_values_type`]. Comparing the widths identifies noncanonical
+    /// storage, not the age of the data: current [`WidenPlugin`] writes use that wire form too.
+    ///
+    /// [`WidenPlugin`]: crate::arrays::widen::WidenPlugin
     pub values_type: DecimalType,
+    /// The validity of the values, which also gives the nullability of the array.
     pub validity: Validity,
 }
 
@@ -235,15 +247,17 @@ impl DecimalData {
     ///
     /// # Errors
     ///
-    /// See [`DecimalArray::new_unchecked`] for invariants that are checked.
+    /// Returns an error if the buffer length is not a multiple of the width of `values_type`, or
+    /// if the buffer is not aligned to that width.
     pub fn try_new_handle(
         values: BufferHandle,
         values_type: DecimalType,
         decimal_dtype: DecimalDType,
     ) -> VortexResult<Self> {
         Self::validate(&values, values_type)?;
 
-        // SAFETY: validate ensures all invariants are met.
+        // SAFETY: `validate` checks the buffer layout. Values outside the declared precision can
+        // produce incorrect results, but do not affect memory safety.
         Ok(unsafe { Self::new_unchecked_handle(values, values_type, decimal_dtype) })
     }
 
@@ -295,10 +309,10 @@ impl DecimalData {
         }
     }
 
-    /// Validates the components that would be used to create a [`DecimalArray`] from a byte buffer.
+    /// Validates the layout of a decimal values buffer.
     ///
-    /// This function checks all the invariants required by [`DecimalArray::new_unchecked`].
-    fn validate(buffer: &BufferHandle, values_type: DecimalType) -> VortexResult<()> {
+    /// Checks byte length and alignment for `values_type`. Does not read values or check validity.
+    pub(crate) fn validate(buffer: &BufferHandle, values_type: DecimalType) -> VortexResult<()> {
         let byte_width = values_type.byte_width();
         vortex_ensure!(
             buffer.len().is_multiple_of(byte_width),

vortex-array/src/arrays/decimal/vtable/mod.rs

@@ -50,9 +50,13 @@ pub(crate) fn initialize(session: &VortexSession) {
     kernel::initialize(session);
 }
 
-// The type of the values can be determined by looking at the type info...right?
+/// Metadata for the existing `vortex.decimal` wire representation.
 #[derive(prost::Message)]
 pub struct DecimalMetadata {
+    /// Stored [`DecimalType`] discriminant, retained for wire compatibility.
+    ///
+    /// The legacy format records the actual width separately from the logical dtype. Readers
+    /// need this field even when the in-memory representation uses canonical decimal leaves.
     #[prost(int32, tag = "1")]
     pub(super) values_type: i32,
 }

vortex-array/src/arrays/widen/compute/aggregate.rs

@@ -86,6 +86,9 @@ fn aggregate_decimal(
         return Ok(Some(partial));
     }
 
+    // Kernel registrations apply to the whole encoding. Keep this decimal allowlist so adding an
+    // integer aggregate cannot enable it here without checking its partials against canonical decimal
+    // execution. Every other decimal aggregate uses the default path.
     if !(aggregate_fn.is::<MinMax>()
         || aggregate_fn.is::<Min>()
         || aggregate_fn.is::<Max>()

vortex-array/src/arrays/widen/compute/fill_null.rs

@@ -27,7 +27,7 @@ impl FillNullReduce for Widen {
         let values = array.values();
         let dtype = match values.dtype() {
             DType::Primitive(..) => {
-                let Some(ptype) = ptype_holding_values_and_scalar(array, fill_value)? else {
+                let Some(ptype) = ptype_holding_values_and_scalar(array, fill_value) else {
                     return Ok(None);
                 };
 

vortex-array/src/arrays/widen/compute/mod.rs

@@ -31,6 +31,7 @@ mod slice;
 mod take;
 mod zip;
 
+use vortex_error::VortexExpect;
 use vortex_error::VortexResult;
 
 use crate::ArrayRef;
@@ -40,7 +41,6 @@ use crate::arrays::ConstantArray;
 use crate::arrays::Widen;
 use crate::arrays::WidenArray;
 use crate::arrays::widen::WidenArraySlotsExt;
-use crate::arrays::widen::encode::integer_value;
 use crate::arrays::widen::encode::smallest_integer_ptype;
 use crate::builtins::ArrayBuiltins;
 use crate::dtype::DType;
@@ -69,29 +69,30 @@ pub(super) fn wrap_values(values: ArrayRef, dtype: DType) -> VortexResult<ArrayR
 pub(super) fn ptype_holding_values_and_scalar(
     array: ArrayView<'_, Widen>,
     scalar: &Scalar,
-) -> VortexResult<Option<PType>> {
+) -> Option<PType> {
     let (DType::Primitive(values_ptype, _), DType::Primitive(logical_ptype, _)) =
         (array.values().dtype(), array.dtype())
     else {
-        return Ok(None);
+        return None;
     };
     if !scalar.dtype().eq_ignore_nullability(array.dtype()) {
-        return Ok(None);
+        return None;
     }
     if scalar.is_null() {
-        return Ok(Some(*values_ptype));
+        return Some(*values_ptype);
     }
 
-    let value = integer_value(scalar)?;
-    let Some(scalar_ptype) = smallest_integer_ptype(value..=value, logical_ptype.is_signed_int())
-    else {
-        return Ok(None);
-    };
+    let value = scalar
+        .as_primitive()
+        .pvalue()
+        .and_then(PValue::as_i128)
+        .vortex_expect("the non-null scalar has the Widen's integer dtype");
+    let scalar_ptype = smallest_integer_ptype(value..=value, logical_ptype.is_signed_int())?;
 
     if scalar_ptype.byte_width() > values_ptype.byte_width() {
-        Ok(Some(scalar_ptype))
+        Some(scalar_ptype)
     } else {
-        Ok(Some(*values_ptype))
+        Some(*values_ptype)
     }
 }
 

vortex-array/src/arrays/widen/compute/numeric.rs

@@ -25,7 +25,7 @@ use crate::arrays::scalar_fn::ScalarFnArrayView;
 use crate::arrays::widen::WidenArraySlotsExt;
 use crate::arrays::widen::compute::cast_keeping_nullability;
 use crate::arrays::widen::compute::wrap_values;
-use crate::arrays::widen::encode::integer_value;
+use crate::arrays::widen::encode::integer_extrema;
 use crate::arrays::widen::encode::smallest_integer_ptype;
 use crate::dtype::DType;
 use crate::dtype::PType;
@@ -162,9 +162,7 @@ fn valid_value_interval(
         return Ok(None);
     };
 
-    Ok(Some(
-        integer_value(&bounds.min)?..=integer_value(&bounds.max)?,
-    ))
+    Ok(Some(integer_extrema(bounds)))
 }
 
 /// Returns the smallest interval that holds `lhs operator rhs` for every value of `lhs` and every

vortex-array/src/arrays/widen/compute/zip.rs

@@ -30,7 +30,7 @@ impl ZipKernel for Widen {
             };
             (other.values().clone(), ptype)
         } else if let Some(constant) = if_false.as_constant() {
-            let Some(ptype) = ptype_holding_values_and_scalar(array, constant)? else {
+            let Some(ptype) = ptype_holding_values_and_scalar(array, constant) else {
                 return Ok(None);
             };
             (if_false.clone(), ptype)