[GitHub] [arrow-rs] tustvold commented on a diff in pull request #2857: Make DecimalArray as PrimitiveArray

[GitBox <gi...@apache.org>]

tustvold commented on code in PR #2857:
URL: https://github.com/apache/arrow-rs/pull/2857#discussion_r992892010


##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -827,9 +831,178 @@ impl<T: ArrowPrimitiveType> From<ArrayData> for PrimitiveArray<T> {
     }
 }
 
+impl<T: DecimalType + ArrowPrimitiveType> PrimitiveArray<T> {
+    /// Returns a Decimal array with the same data as self, with the
+    /// specified precision.
+    ///
+    /// Returns an Error if:
+    /// 1. `precision` is larger than `T:MAX_PRECISION`
+    /// 2. `scale` is larger than `T::MAX_SCALE`
+    /// 3. `scale` is > `precision`
+    pub fn with_precision_and_scale(
+        self,
+        precision: u8,
+        scale: u8,
+    ) -> Result<Self, ArrowError>
+    where
+        Self: Sized,
+    {
+        // validate precision and scale
+        self.validate_precision_scale(precision, scale)?;
+
+        // Ensure that all values are within the requested
+        // precision. For performance, only check if the precision is
+        // decreased
+        let p = self.precision()?;
+        if precision < p {
+            self.validate_decimal_precision(precision)?;
+        }
+
+        // safety: self.data is valid DataType::Decimal as checked above
+        let new_data_type = T::TYPE_CONSTRUCTOR(precision, scale);
+        let data = self.data().clone().into_builder().data_type(new_data_type);
+
+        // SAFETY
+        // Validated data above
+        Ok(unsafe { data.build_unchecked().into() })
+    }
+
+    /// Returns a Decimal array with the same data as self, with the
+    /// specified precision.
+    ///
+    /// # Safety
+    ///
+    /// This doesn't validate decimal values with specified precision.
+    pub unsafe fn unchecked_with_precision_and_scale(

Review Comment:
   The move to PrimitiveArray implicitly means away from validating the contents for overflow, as any value of the underlying primitive is permitted.
   
   I think we should therefore make this safe, and remove any value validation except for when this method is explicitly called by the user, as an opt-in



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org