You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by li...@apache.org on 2022/08/17 14:17:00 UTC

[arrow-rs] branch master updated: collation the validate precision code for decimal array (#2446)

This is an automated email from the ASF dual-hosted git repository.

liukun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git


The following commit(s) were added to refs/heads/master by this push:
     new 001317032 collation the validate precision code for decimal array (#2446)
001317032 is described below

commit 00131703225baf77b5e0c2cbd5cdbb0f3d94eec6
Author: Kun Liu <li...@apache.org>
AuthorDate: Wed Aug 17 22:16:55 2022 +0800

    collation the validate precision code for decimal array (#2446)
---
 arrow/src/array/array_decimal.rs | 167 +++++++++++++++++----------------------
 1 file changed, 72 insertions(+), 95 deletions(-)

diff --git a/arrow/src/array/array_decimal.rs b/arrow/src/array/array_decimal.rs
index d051c4b9d..98fc9d0c7 100644
--- a/arrow/src/array/array_decimal.rs
+++ b/arrow/src/array/array_decimal.rs
@@ -249,39 +249,6 @@ impl<const BYTE_WIDTH: usize> BasicDecimalArray<BYTE_WIDTH> {
     fn raw_value_data_ptr(&self) -> *const u8 {
         self.value_data.as_ptr()
     }
-}
-
-impl Decimal128Array {
-    /// Creates a [Decimal128Array] with default precision and scale,
-    /// based on an iterator of `i128` values without nulls
-    pub fn from_iter_values<I: IntoIterator<Item = i128>>(iter: I) -> Self {
-        let val_buf: Buffer = iter.into_iter().collect();
-        let data = unsafe {
-            ArrayData::new_unchecked(
-                Self::default_type(),
-                val_buf.len() / std::mem::size_of::<i128>(),
-                None,
-                None,
-                0,
-                vec![val_buf],
-                vec![],
-            )
-        };
-        Decimal128Array::from(data)
-    }
-
-    // Validates decimal values in this array can be properly interpreted
-    // with the specified precision.
-    fn validate_decimal_precision(&self, precision: usize) -> Result<()> {
-        (0..self.len()).try_for_each(|idx| {
-            if self.is_valid(idx) {
-                let decimal = unsafe { self.value_unchecked(idx) };
-                validate_decimal_precision(decimal.as_i128(), precision)
-            } else {
-                Ok(())
-            }
-        })
-    }
 
     /// Returns a Decimal array with the same data as self, with the
     /// specified precision.
@@ -294,6 +261,23 @@ impl Decimal128Array {
     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
+        if precision < self.precision {
+            self.validate_data(precision)?;
+        }
+
+        // safety: self.data is valid DataType::Decimal as checked above
+        let new_data_type = Self::TYPE_CONSTRUCTOR(precision, scale);
+        Ok(self.data().clone().with_data_type(new_data_type).into())
+    }
+
+    // validate that the new precision and scale are valid or not
+    fn validate_precision_scale(&self, precision: usize, scale: usize) -> Result<()> {
         if precision > Self::MAX_PRECISION {
             return Err(ArrowError::InvalidArgumentError(format!(
                 "precision {} is greater than max {}",
@@ -314,26 +298,67 @@ impl Decimal128Array {
                 scale, precision
             )));
         }
-
-        // Ensure that all values are within the requested
-        // precision. For performance, only check if the precision is
-        // decreased
-        if precision < self.precision {
-            self.validate_decimal_precision(precision)?;
-        }
-
         let data_type = Self::TYPE_CONSTRUCTOR(self.precision, self.scale);
         assert_eq!(self.data().data_type(), &data_type);
 
-        // safety: self.data is valid DataType::Decimal as checked above
-        let new_data_type = Self::TYPE_CONSTRUCTOR(precision, scale);
+        Ok(())
+    }
+
+    // validate all the data in the array are valid within the new precision or not
+    fn validate_data(&self, precision: usize) -> Result<()> {
+        match BYTE_WIDTH {
+            16 => self
+                .as_any()
+                .downcast_ref::<Decimal128Array>()
+                .unwrap()
+                .validate_decimal_precision(precision),
+            32 => self
+                .as_any()
+                .downcast_ref::<Decimal256Array>()
+                .unwrap()
+                .validate_decimal_precision(precision),
+            other_width => {
+                panic!("invalid byte width {}", other_width);
+            }
+        }
+    }
+}
 
-        Ok(self.data().clone().with_data_type(new_data_type).into())
+impl Decimal128Array {
+    /// Creates a [Decimal128Array] with default precision and scale,
+    /// based on an iterator of `i128` values without nulls
+    pub fn from_iter_values<I: IntoIterator<Item = i128>>(iter: I) -> Self {
+        let val_buf: Buffer = iter.into_iter().collect();
+        let data = unsafe {
+            ArrayData::new_unchecked(
+                Self::default_type(),
+                val_buf.len() / std::mem::size_of::<i128>(),
+                None,
+                None,
+                0,
+                vec![val_buf],
+                vec![],
+            )
+        };
+        Decimal128Array::from(data)
+    }
+
+    // Validates decimal128 values in this array can be properly interpreted
+    // with the specified precision.
+    fn validate_decimal_precision(&self, precision: usize) -> Result<()> {
+        (0..self.len()).try_for_each(|idx| {
+            if self.is_valid(idx) {
+                let decimal = unsafe { self.value_unchecked(idx) };
+                validate_decimal_precision(decimal.as_i128(), precision)
+            } else {
+                Ok(())
+            }
+        })
     }
 }
 
 impl Decimal256Array {
-    // Validates decimal values in this array can be properly interpreted
+    // Validates decimal256 values in this array can be properly interpreted
     // with the specified precision.
     fn validate_decimal_precision(&self, precision: usize) -> Result<()> {
         (0..self.len()).try_for_each(|idx| {
@@ -351,54 +376,6 @@ impl Decimal256Array {
             }
         })
     }
-
-    /// Returns a Decimal array with the same data as self, with the
-    /// specified precision.
-    ///
-    /// Returns an Error if:
-    /// 1. `precision` is larger than [`Self::MAX_PRECISION`]
-    /// 2. `scale` is larger than [`Self::MAX_SCALE`];
-    /// 3. `scale` is > `precision`
-    pub fn with_precision_and_scale(self, precision: usize, scale: usize) -> Result<Self>
-    where
-        Self: Sized,
-    {
-        if precision > Self::MAX_PRECISION {
-            return Err(ArrowError::InvalidArgumentError(format!(
-                "precision {} is greater than max {}",
-                precision,
-                Self::MAX_PRECISION
-            )));
-        }
-        if scale > Self::MAX_SCALE {
-            return Err(ArrowError::InvalidArgumentError(format!(
-                "scale {} is greater than max {}",
-                scale,
-                Self::MAX_SCALE
-            )));
-        }
-        if scale > precision {
-            return Err(ArrowError::InvalidArgumentError(format!(
-                "scale {} is greater than precision {}",
-                scale, precision
-            )));
-        }
-
-        // Ensure that all values are within the requested
-        // precision. For performance, only check if the precision is
-        // decreased
-        if precision < self.precision {
-            self.validate_decimal_precision(precision)?;
-        }
-
-        let data_type = Self::TYPE_CONSTRUCTOR(self.precision, self.scale);
-        assert_eq!(self.data().data_type(), &data_type);
-
-        // safety: self.data is valid DataType::Decimal as checked above
-        let new_data_type = Self::TYPE_CONSTRUCTOR(precision, scale);
-
-        Ok(self.data().clone().with_data_type(new_data_type).into())
-    }
 }
 
 impl<const BYTE_WIDTH: usize> From<ArrayData> for BasicDecimalArray<BYTE_WIDTH> {
@@ -942,7 +919,7 @@ mod tests {
             Decimal256::from_big_int(
                 &value1,
                 DECIMAL256_MAX_PRECISION,
-                DECIMAL_DEFAULT_SCALE
+                DECIMAL_DEFAULT_SCALE,
             )
             .unwrap(),
             array.value(0)
@@ -953,7 +930,7 @@ mod tests {
             Decimal256::from_big_int(
                 &value2,
                 DECIMAL256_MAX_PRECISION,
-                DECIMAL_DEFAULT_SCALE
+                DECIMAL_DEFAULT_SCALE,
             )
             .unwrap(),
             array.value(2)