Make builder append methods infallible where possible (#2103)

* Make builder append methods infallible where possible

* Revert result changes in MapBuilder

* Fix take_kernels benchmark

* Clippy fixes

* More clippy fixes

arrow/benches/builder.rs

@@ -43,7 +43,7 @@ fn bench_primitive(c: &mut Criterion) {
         b.iter(|| {
             let mut builder = Int64Builder::new(64);
             for _ in 0..NUM_BATCHES {
-                let _ = black_box(builder.append_slice(&data[..]));
+                builder.append_slice(&data[..]);
             }
             black_box(builder.finish());
         })
@@ -57,7 +57,7 @@ fn bench_primitive_nulls(c: &mut Criterion) {
         b.iter(|| {
             let mut builder = UInt8Builder::new(64);
             for _ in 0..NUM_BATCHES * BATCH_SIZE {
-                let _ = black_box(builder.append_null());
+                builder.append_null();
             }
             black_box(builder.finish());
         })
@@ -80,7 +80,7 @@ fn bench_bool(c: &mut Criterion) {
         b.iter(|| {
             let mut builder = BooleanBuilder::new(64);
             for _ in 0..NUM_BATCHES {
-                let _ = black_box(builder.append_slice(&data[..]));
+                builder.append_slice(&data[..]);
             }
             black_box(builder.finish());
         })
@@ -98,7 +98,7 @@ fn bench_string(c: &mut Criterion) {
         b.iter(|| {
             let mut builder = StringBuilder::new(64);
             for _ in 0..NUM_BATCHES * BATCH_SIZE {
-                let _ = black_box(builder.append_value(SAMPLE_STRING));
+                builder.append_value(SAMPLE_STRING);
             }
             black_box(builder.finish());
         })

arrow/benches/cast_kernels.rs

@@ -49,12 +49,12 @@ fn build_utf8_date_array(size: usize, with_nulls: bool) -> ArrayRef {
 
     for _ in 0..size {
         if with_nulls && rng.gen::<f32>() > 0.8 {
-            builder.append_null().unwrap();
+            builder.append_null();
         } else {
             let string = NaiveDate::from_num_days_from_ce(rng.sample(range))
                 .format("%Y-%m-%d")
                 .to_string();
-            builder.append_value(&string).unwrap();
+            builder.append_value(&string);
         }
     }
     Arc::new(builder.finish())
@@ -70,12 +70,12 @@ fn build_utf8_date_time_array(size: usize, with_nulls: bool) -> ArrayRef {
 
     for _ in 0..size {
         if with_nulls && rng.gen::<f32>() > 0.8 {
-            builder.append_null().unwrap();
+            builder.append_null();
         } else {
             let string = NaiveDateTime::from_timestamp(rng.sample(range), 0)
                 .format("%Y-%m-%dT%H:%M:%S")
                 .to_string();
-            builder.append_value(&string).unwrap();
+            builder.append_value(&string);
         }
     }
     Arc::new(builder.finish())

arrow/benches/take_kernels.rs

@@ -33,10 +33,10 @@ fn create_random_index(size: usize, null_density: f32) -> UInt32Array {
     let mut builder = UInt32Builder::new(size);
     for _ in 0..size {
         if rng.gen::<f32>() < null_density {
-            builder.append_null().unwrap()
+            builder.append_null();
         } else {
             let value = rng.gen_range::<u32, _>(0u32..size as u32);
-            builder.append_value(value).unwrap();
+            builder.append_value(value);
         }
     }
     builder.finish()

arrow/examples/builders.rs

@@ -37,19 +37,17 @@ fn main() {
     let mut primitive_array_builder = Int32Builder::new(100);
 
     // Append an individual primitive value
-    primitive_array_builder.append_value(55).unwrap();
+    primitive_array_builder.append_value(55);
 
     // Append a null value
-    primitive_array_builder.append_null().unwrap();
+    primitive_array_builder.append_null();
 
     // Append a slice of primitive values
-    primitive_array_builder.append_slice(&[39, 89, 12]).unwrap();
+    primitive_array_builder.append_slice(&[39, 89, 12]);
 
     // Append lots of values
-    primitive_array_builder.append_null().unwrap();
-    primitive_array_builder
-        .append_slice(&(25..50).collect::<Vec<i32>>())
-        .unwrap();
+    primitive_array_builder.append_null();
+    primitive_array_builder.append_slice(&(25..50).collect::<Vec<i32>>());
 
     // Build the `PrimitiveArray`
     let primitive_array = primitive_array_builder.finish();

arrow/src/array/array_boolean.rs

@@ -276,9 +276,9 @@ mod tests {
     #[test]
     fn test_boolean_with_null_fmt_debug() {
         let mut builder = BooleanArray::builder(3);
-        builder.append_value(true).unwrap();
-        builder.append_null().unwrap();
-        builder.append_value(false).unwrap();
+        builder.append_value(true);
+        builder.append_null();
+        builder.append_value(false);
         let arr = builder.finish();
         assert_eq!(
             "BooleanArray\n[\n  true,\n  null,\n  false,\n]",

arrow/src/array/array_dictionary.rs

@@ -319,9 +319,7 @@ impl<'a, T: ArrowPrimitiveType + ArrowDictionaryKeyType> FromIterator<Option<&'a
                     .append(i)
                     .expect("Unable to append a value to a dictionary array.");
             } else {
-                builder
-                    .append_null()
-                    .expect("Unable to append a null value to a dictionary array.");
+                builder.append_null();
             }
         });
 
@@ -463,7 +461,7 @@ mod tests {
         let value_builder = PrimitiveBuilder::<UInt32Type>::new(2);
         let mut builder = PrimitiveDictionaryBuilder::new(key_builder, value_builder);
         builder.append(12345678).unwrap();
-        builder.append_null().unwrap();
+        builder.append_null();
         builder.append(22345678).unwrap();
         let array = builder.finish();
         assert_eq!(

arrow/src/array/array_primitive.rs

@@ -933,9 +933,9 @@ mod tests {
     #[test]
     fn test_int32_with_null_fmt_debug() {
         let mut builder = Int32Array::builder(3);
-        builder.append_slice(&[0, 1]).unwrap();
-        builder.append_null().unwrap();
-        builder.append_slice(&[3, 4]).unwrap();
+        builder.append_slice(&[0, 1]);
+        builder.append_null();
+        builder.append_slice(&[3, 4]);
         let arr = builder.finish();
         assert_eq!(
             "PrimitiveArray<Int32>\n[\n  0,\n  1,\n  null,\n  3,\n  4,\n]",

arrow/src/array/array_string.rs

@@ -446,15 +446,12 @@ mod tests {
         let string_builder = StringBuilder::new(3);
         let mut list_of_string_builder = ListBuilder::new(string_builder);
 
-        list_of_string_builder.values().append_value("foo").unwrap();
-        list_of_string_builder.values().append_value("bar").unwrap();
-        list_of_string_builder.append(true).unwrap();
+        list_of_string_builder.values().append_value("foo");
+        list_of_string_builder.values().append_value("bar");
+        list_of_string_builder.append(true);
 
-        list_of_string_builder
-            .values()
-            .append_value("foobar")
-            .unwrap();
-        list_of_string_builder.append(true).unwrap();
+        list_of_string_builder.values().append_value("foobar");
+        list_of_string_builder.append(true);
         let list_of_strings = list_of_string_builder.finish();
 
         assert_eq!(list_of_strings.len(), 2);

arrow/src/array/builder/boolean_builder.rs

@@ -23,7 +23,9 @@ use crate::array::ArrayData;
 use crate::array::ArrayRef;
 use crate::array::BooleanArray;
 use crate::datatypes::DataType;
-use crate::error::{ArrowError, Result};
+
+use crate::error::ArrowError;
+use crate::error::Result;
 
 use super::BooleanBufferBuilder;
 
@@ -81,44 +83,42 @@ impl BooleanBuilder {
 
     /// Appends a value of type `T` into the builder
     #[inline]
-    pub fn append_value(&mut self, v: bool) -> Result<()> {
+    pub fn append_value(&mut self, v: bool) {
         self.values_builder.append(v);
         if let Some(b) = self.bitmap_builder.as_mut() {
             b.append(true)
         }
-        Ok(())
     }
 
     /// Appends a null slot into the builder
     #[inline]
-    pub fn append_null(&mut self) -> Result<()> {
+    pub fn append_null(&mut self) {
         self.materialize_bitmap_builder();
         self.bitmap_builder.as_mut().unwrap().append(false);
         self.values_builder.advance(1);
-        Ok(())
     }
 
     /// Appends an `Option<T>` into the builder
     #[inline]
-    pub fn append_option(&mut self, v: Option<bool>) -> Result<()> {
+    pub fn append_option(&mut self, v: Option<bool>) {
         match v {
-            None => self.append_null()?,
-            Some(v) => self.append_value(v)?,
+            None => self.append_null(),
+            Some(v) => self.append_value(v),
         };
-        Ok(())
     }
 
     /// Appends a slice of type `T` into the builder
     #[inline]
-    pub fn append_slice(&mut self, v: &[bool]) -> Result<()> {
+    pub fn append_slice(&mut self, v: &[bool]) {
         if let Some(b) = self.bitmap_builder.as_mut() {
             b.append_n(v.len(), true)
         }
         self.values_builder.append_slice(v);
-        Ok(())
     }
 
-    /// Appends values from a slice of type `T` and a validity boolean slice
+    /// Appends values from a slice of type `T` and a validity boolean slice.
+    ///
+    /// Returns an error if the slices are of different lengths
     #[inline]
     pub fn append_values(&mut self, values: &[bool], is_valid: &[bool]) -> Result<()> {
         if values.len() != is_valid.len() {
@@ -205,9 +205,9 @@ mod tests {
         let mut builder = BooleanArray::builder(10);
         for i in 0..10 {
             if i == 3 || i == 6 || i == 9 {
-                builder.append_value(true).unwrap();
+                builder.append_value(true);
             } else {
-                builder.append_value(false).unwrap();
+                builder.append_value(false);
             }
         }
 
@@ -229,10 +229,10 @@ mod tests {
             BooleanArray::from(vec![Some(true), Some(false), None, None, Some(false)]);
 
         let mut builder = BooleanArray::builder(0);
-        builder.append_slice(&[true, false]).unwrap();
-        builder.append_null().unwrap();
-        builder.append_null().unwrap();
-        builder.append_value(false).unwrap();
+        builder.append_slice(&[true, false]);
+        builder.append_null();
+        builder.append_null();
+        builder.append_value(false);
         let arr2 = builder.finish();
 
         assert_eq!(arr1, arr2);
@@ -243,7 +243,7 @@ mod tests {
         let arr1 = BooleanArray::from(vec![true; 513]);
 
         let mut builder = BooleanArray::builder(512);
-        builder.append_slice(&[true; 513]).unwrap();
+        builder.append_slice(&[true; 513]);
         let arr2 = builder.finish();
 
         assert_eq!(arr1, arr2);
@@ -252,9 +252,9 @@ mod tests {
     #[test]
     fn test_boolean_array_builder_no_null() {
         let mut builder = BooleanArray::builder(0);
-        builder.append_option(Some(true)).unwrap();
-        builder.append_value(false).unwrap();
-        builder.append_slice(&[true, false, true]).unwrap();
+        builder.append_option(Some(true));
+        builder.append_value(false);
+        builder.append_slice(&[true, false, true]);
         builder
             .append_values(&[false, false, true], &[true, true, true])
             .unwrap();

arrow/src/array/builder/buffer_builder.rs

@@ -362,7 +362,6 @@ mod tests {
     use crate::array::Int32BufferBuilder;
     use crate::array::Int8Builder;
     use crate::array::UInt8BufferBuilder;
-    use crate::error::Result;
 
     #[test]
     fn test_builder_i32_empty() {
@@ -457,17 +456,17 @@ mod tests {
     }
 
     #[test]
-    fn test_append_values() -> Result<()> {
+    fn test_append_values() {
         let mut a = Int8Builder::new(0);
-        a.append_value(1)?;
-        a.append_null()?;
-        a.append_value(-2)?;
+        a.append_value(1);
+        a.append_null();
+        a.append_value(-2);
         assert_eq!(a.len(), 3);
 
         // append values
         let values = &[1, 2, 3, 4];
         let is_valid = &[true, true, false, true];
-        a.append_values(values, is_valid)?;
+        a.append_values(values, is_valid);
 
         assert_eq!(a.len(), 7);
         let array = a.finish();
@@ -478,7 +477,5 @@ mod tests {
         assert_eq!(array.value(4), 2);
         assert!(array.is_null(5));
         assert_eq!(array.value(6), 4);
-
-        Ok(())
     }
 }

arrow/src/array/builder/decimal_builder.rs

@@ -108,7 +108,7 @@ impl Decimal128Builder {
 
     /// Append a null value to the array.
     #[inline]
-    pub fn append_null(&mut self) -> Result<()> {
+    pub fn append_null(&mut self) {
         self.builder.append_null()
     }
 
@@ -167,6 +167,8 @@ impl Decimal256Builder {
     }
 
     /// Appends a [`Decimal256`] number into the builder.
+    ///
+    /// Returns an error if `value` has different precision, scale or length in bytes than this builder
     #[inline]
     pub fn append_value(&mut self, value: &Decimal256) -> Result<()> {
         if self.precision != value.precision() || self.scale != value.scale() {
@@ -187,7 +189,7 @@ impl Decimal256Builder {
 
     /// Append a null value to the array.
     #[inline]
-    pub fn append_null(&mut self) -> Result<()> {
+    pub fn append_null(&mut self) {
         self.builder.append_null()
     }
 
@@ -215,7 +217,7 @@ mod tests {
         let mut builder = Decimal128Builder::new(30, 38, 6);
 
         builder.append_value(8_887_000_000_i128).unwrap();
-        builder.append_null().unwrap();
+        builder.append_null();
         builder.append_value(-8_887_000_000_i128).unwrap();
         let decimal_array: Decimal128Array = builder.finish();
 
@@ -233,7 +235,7 @@ mod tests {
         builder
             .append_value(Decimal128::new_from_i128(30, 38, 8_887_000_000_i128))
             .unwrap();
-        builder.append_null().unwrap();
+        builder.append_null();
         builder
             .append_value(Decimal128::new_from_i128(30, 38, -8_887_000_000_i128))
             .unwrap();
@@ -255,7 +257,7 @@ mod tests {
         let value = Decimal256::try_new_from_bytes(40, 6, bytes.as_slice()).unwrap();
         builder.append_value(&value).unwrap();
 
-        builder.append_null().unwrap();
+        builder.append_null();
 
         bytes = vec![255; 32];
         let value = Decimal256::try_new_from_bytes(40, 6, bytes.as_slice()).unwrap();