From 320de1c20aefbf204f6888e2ad3663863afeba9f Mon Sep 17 00:00:00 2001
From: Andrew Lamb <andrew@nerdnetworks.org>
Date: Fri, 12 Nov 2021 06:18:09 -0500
Subject: [PATCH] add checker for appending i128 to decimal builder (#928)
 (#943)

* add check for appending i128 to decimal builder

* remove the ArrowError(DecimalError)

Co-authored-by: Kun Liu <liukun@apache.org>
---
 arrow/src/array/array_binary.rs | 35 ++++++++++++-
 arrow/src/array/builder.rs      | 89 +++++++++++++++++++++++++++++++++
 2 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/arrow/src/array/array_binary.rs b/arrow/src/array/array_binary.rs
index 0592c048c03..ecf1850ad45 100644
--- a/arrow/src/array/array_binary.rs
+++ b/arrow/src/array/array_binary.rs
@@ -1286,9 +1286,42 @@ mod tests {
         assert_eq!(16, decimal_array.value_length());
     }
 
+    #[test]
+    fn test_decimal_append_error_value() {
+        let mut decimal_builder = DecimalBuilder::new(10, 5, 3);
+        let mut result = decimal_builder.append_value(123456);
+        let mut error = result.unwrap_err();
+        assert_eq!(
+            "Invalid argument error: The value of 123456 i128 is not compatible with Decimal(5,3)",
+            error.to_string()
+        );
+        decimal_builder.append_value(12345).unwrap();
+        let arr = decimal_builder.finish();
+        assert_eq!("12.345", arr.value_as_string(0));
+
+        decimal_builder = DecimalBuilder::new(10, 2, 1);
+        result = decimal_builder.append_value(100);
+        error = result.unwrap_err();
+        assert_eq!(
+            "Invalid argument error: The value of 100 i128 is not compatible with Decimal(2,1)",
+            error.to_string()
+        );
+        decimal_builder.append_value(99).unwrap();
+        result = decimal_builder.append_value(-100);
+        error = result.unwrap_err();
+        assert_eq!(
+            "Invalid argument error: The value of -100 i128 is not compatible with Decimal(2,1)",
+            error.to_string()
+        );
+        decimal_builder.append_value(-99).unwrap();
+        let arr = decimal_builder.finish();
+        assert_eq!("9.9", arr.value_as_string(0));
+        assert_eq!("-9.9", arr.value_as_string(1));
+    }
+
     #[test]
     fn test_decimal_array_value_as_string() {
-        let mut decimal_builder = DecimalBuilder::new(7, 5, 3);
+        let mut decimal_builder = DecimalBuilder::new(7, 6, 3);
         for value in [123450, -123450, 100, -100, 10, -10, 0] {
             decimal_builder.append_value(value).unwrap();
         }
diff --git a/arrow/src/array/builder.rs b/arrow/src/array/builder.rs
index 60f76d95485..d08816c6276 100644
--- a/arrow/src/array/builder.rs
+++ b/arrow/src/array/builder.rs
@@ -1118,6 +1118,87 @@ pub struct FixedSizeBinaryBuilder {
     builder: FixedSizeListBuilder<UInt8Builder>,
 }
 
+const MAX_DECIMAL_FOR_EACH_PRECISION: [i128; 38] = [
+    9,
+    99,
+    999,
+    9999,
+    99999,
+    999999,
+    9999999,
+    99999999,
+    999999999,
+    9999999999,
+    99999999999,
+    999999999999,
+    9999999999999,
+    99999999999999,
+    999999999999999,
+    9999999999999999,
+    99999999999999999,
+    999999999999999999,
+    9999999999999999999,
+    99999999999999999999,
+    999999999999999999999,
+    9999999999999999999999,
+    99999999999999999999999,
+    999999999999999999999999,
+    9999999999999999999999999,
+    99999999999999999999999999,
+    999999999999999999999999999,
+    9999999999999999999999999999,
+    99999999999999999999999999999,
+    999999999999999999999999999999,
+    9999999999999999999999999999999,
+    99999999999999999999999999999999,
+    999999999999999999999999999999999,
+    9999999999999999999999999999999999,
+    99999999999999999999999999999999999,
+    999999999999999999999999999999999999,
+    9999999999999999999999999999999999999,
+    170141183460469231731687303715884105727,
+];
+const MIN_DECIMAL_FOR_EACH_PRECISION: [i128; 38] = [
+    -9,
+    -99,
+    -999,
+    -9999,
+    -99999,
+    -999999,
+    -9999999,
+    -99999999,
+    -999999999,
+    -9999999999,
+    -99999999999,
+    -999999999999,
+    -9999999999999,
+    -99999999999999,
+    -999999999999999,
+    -9999999999999999,
+    -99999999999999999,
+    -999999999999999999,
+    -9999999999999999999,
+    -99999999999999999999,
+    -999999999999999999999,
+    -9999999999999999999999,
+    -99999999999999999999999,
+    -999999999999999999999999,
+    -9999999999999999999999999,
+    -99999999999999999999999999,
+    -999999999999999999999999999,
+    -9999999999999999999999999999,
+    -99999999999999999999999999999,
+    -999999999999999999999999999999,
+    -9999999999999999999999999999999,
+    -99999999999999999999999999999999,
+    -999999999999999999999999999999999,
+    -9999999999999999999999999999999999,
+    -99999999999999999999999999999999999,
+    -999999999999999999999999999999999999,
+    -9999999999999999999999999999999999999,
+    -170141183460469231731687303715884105728,
+];
+
 ///
 /// Array Builder for [`DecimalArray`]
 ///
@@ -1431,6 +1512,14 @@ impl DecimalBuilder {
     /// distinct array element.
     #[inline]
     pub fn append_value(&mut self, value: i128) -> Result<()> {
+        if value > MAX_DECIMAL_FOR_EACH_PRECISION[self.precision - 1]
+            || value < MIN_DECIMAL_FOR_EACH_PRECISION[self.precision - 1]
+        {
+            return Err(ArrowError::InvalidArgumentError(format!(
+                "The value of {} i128 is not compatible with Decimal({},{})",
+                value, self.precision, self.scale
+            )));
+        }
         let value_as_bytes = Self::from_i128_to_fixed_size_bytes(
             value,
             self.builder.value_length() as usize,