Replace &Option<T> with Option<&T> (#1571)

* Convert &Option<T> arguments to Option<&T> * fix cargo fmt issues * Update FileMetaData created_by() to return Option<&str>, Fix integration test
apache · Apr 17, 2022 · 786792c · 786792c
1 parent 6bb6ed0
commit 786792c
Show file tree

Hide file tree

Showing 19 changed files with 68 additions and 59 deletions.
diff --git a/arrow-pyarrow-integration-testing/src/lib.rs b/arrow-pyarrow-integration-testing/src/lib.rs
@@ -70,7 +70,7 @@ fn substring(array: ArrayData, start: i64) -> PyResult<ArrayData> {
     let array = ArrayRef::from(array);
 
     // substring
-    let array = kernels::substring::substring(array.as_ref(), start, &None)?;
+    let array = kernels::substring::substring(array.as_ref(), start, None)?;
 
     Ok(array.data().to_owned())
 }

diff --git a/arrow/benches/string_kernels.rs b/arrow/benches/string_kernels.rs
@@ -26,7 +26,12 @@ use arrow::compute::kernels::substring::substring;
 use arrow::util::bench_util::*;
 
 fn bench_substring(arr: &StringArray, start: i64, length: usize) {
-    substring(criterion::black_box(arr), start, &Some(length as u64)).unwrap();
+    substring(
+        criterion::black_box(arr),
+        start,
+        Some(length as u64).as_ref(),
+    )
+    .unwrap();
 }
 
 fn add_benchmark(c: &mut Criterion) {

diff --git a/arrow/src/array/array_struct.rs b/arrow/src/array/array_struct.rs
@@ -360,7 +360,7 @@ mod tests {
         assert_eq!(1, struct_data.null_count());
         assert_eq!(
             // 00001011
-            &Some(Bitmap::from(Buffer::from(&[11_u8]))),
+            Some(&Bitmap::from(Buffer::from(&[11_u8]))),
             struct_data.null_bitmap()
         );
 

diff --git a/arrow/src/array/builder.rs b/arrow/src/array/builder.rs
@@ -3561,7 +3561,7 @@ mod tests {
         assert_eq!(4, struct_data.len());
         assert_eq!(1, struct_data.null_count());
         assert_eq!(
-            &Some(Bitmap::from(Buffer::from(&[11_u8]))),
+            Some(&Bitmap::from(Buffer::from(&[11_u8]))),
             struct_data.null_bitmap()
         );
 
@@ -3675,7 +3675,7 @@ mod tests {
         assert_eq!(3, map_data.len());
         assert_eq!(1, map_data.null_count());
         assert_eq!(
-            &Some(Bitmap::from(Buffer::from(&[5_u8]))),
+            Some(&Bitmap::from(Buffer::from(&[5_u8]))),
             map_data.null_bitmap()
         );
 

diff --git a/arrow/src/array/data.rs b/arrow/src/array/data.rs
@@ -398,8 +398,8 @@ impl ArrayData {
 
     /// Returns a reference to the null bitmap of this array data
     #[inline]
-    pub const fn null_bitmap(&self) -> &Option<Bitmap> {
-        &self.null_bitmap
+    pub const fn null_bitmap(&self) -> Option<&Bitmap> {
+        self.null_bitmap.as_ref()
     }
 
     /// Returns a reference to the null buffer of this array data.
@@ -500,7 +500,7 @@ impl ArrayData {
                     .iter()
                     .map(|data| data.slice(offset, length))
                     .collect(),
-                null_bitmap: self.null_bitmap().clone(),
+                null_bitmap: self.null_bitmap().cloned(),
             };
 
             new_data

diff --git a/arrow/src/array/equal/utils.rs b/arrow/src/array/equal/utils.rs
@@ -132,7 +132,7 @@ pub(super) fn child_logical_null_buffer(
             let ceil = bit_util::ceil(parent_len, 8);
             Bitmap::from(Buffer::from(vec![0b11111111; ceil]))
         });
-    let self_null_bitmap = child_data.null_bitmap().clone().unwrap_or_else(|| {
+    let self_null_bitmap = child_data.null_bitmap().cloned().unwrap_or_else(|| {
         let ceil = bit_util::ceil(child_data.len(), 8);
         Bitmap::from(Buffer::from(vec![0b11111111; ceil]))
     });

diff --git a/arrow/src/compute/kernels/boolean.rs b/arrow/src/compute/kernels/boolean.rs
@@ -1010,7 +1010,7 @@ mod tests {
         let expected = BooleanArray::from(vec![false, false, false, false]);
 
         assert_eq!(expected, res);
-        assert_eq!(&None, res.data_ref().null_bitmap());
+        assert_eq!(None, res.data_ref().null_bitmap());
     }
 
     #[test]
@@ -1023,7 +1023,7 @@ mod tests {
         let expected = BooleanArray::from(vec![false, false, false, false]);
 
         assert_eq!(expected, res);
-        assert_eq!(&None, res.data_ref().null_bitmap());
+        assert_eq!(None, res.data_ref().null_bitmap());
     }
 
     #[test]
@@ -1035,7 +1035,7 @@ mod tests {
         let expected = BooleanArray::from(vec![true, true, true, true]);
 
         assert_eq!(expected, res);
-        assert_eq!(&None, res.data_ref().null_bitmap());
+        assert_eq!(None, res.data_ref().null_bitmap());
     }
 
     #[test]
@@ -1048,7 +1048,7 @@ mod tests {
         let expected = BooleanArray::from(vec![true, true, true, true]);
 
         assert_eq!(expected, res);
-        assert_eq!(&None, res.data_ref().null_bitmap());
+        assert_eq!(None, res.data_ref().null_bitmap());
     }
 
     #[test]
@@ -1060,7 +1060,7 @@ mod tests {
         let expected = BooleanArray::from(vec![false, true, false, true]);
 
         assert_eq!(expected, res);
-        assert_eq!(&None, res.data_ref().null_bitmap());
+        assert_eq!(None, res.data_ref().null_bitmap());
     }
 
     #[test]
@@ -1091,7 +1091,7 @@ mod tests {
         let expected = BooleanArray::from(vec![false, true, false, true]);
 
         assert_eq!(expected, res);
-        assert_eq!(&None, res.data_ref().null_bitmap());
+        assert_eq!(None, res.data_ref().null_bitmap());
     }
 
     #[test]
@@ -1103,7 +1103,7 @@ mod tests {
         let expected = BooleanArray::from(vec![true, false, true, false]);
 
         assert_eq!(expected, res);
-        assert_eq!(&None, res.data_ref().null_bitmap());
+        assert_eq!(None, res.data_ref().null_bitmap());
     }
 
     #[test]
@@ -1134,7 +1134,7 @@ mod tests {
         let expected = BooleanArray::from(vec![true, false, true, false]);
 
         assert_eq!(expected, res);
-        assert_eq!(&None, res.data_ref().null_bitmap());
+        assert_eq!(None, res.data_ref().null_bitmap());
     }
 
     #[test]

diff --git a/arrow/src/compute/kernels/cast.rs b/arrow/src/compute/kernels/cast.rs
@@ -1244,7 +1244,11 @@ where
             to_type,
             array.len(),
             Some(array.null_count()),
-            array.data().null_bitmap().clone().map(|bitmap| bitmap.bits),
+            array
+                .data()
+                .null_bitmap()
+                .cloned()
+                .map(|bitmap| bitmap.bits),
             array.data().offset(),
             array.data().buffers().to_vec(),
             vec![],
@@ -1730,7 +1734,7 @@ fn dictionary_cast<K: ArrowDictionaryKeyType>(
                     cast_keys
                         .data()
                         .null_bitmap()
-                        .clone()
+                        .cloned()
                         .map(|bitmap| bitmap.bits),
                     cast_keys.data().offset(),
                     cast_keys.data().buffers().to_vec(),
@@ -1948,7 +1952,7 @@ fn cast_primitive_to_list<OffsetSize: OffsetSizeTrait + NumCast>(
             cast_array
                 .data()
                 .null_bitmap()
-                .clone()
+                .cloned()
                 .map(|bitmap| bitmap.bits),
             0,
             vec![offsets.into()],
@@ -1976,7 +1980,7 @@ fn cast_list_inner<OffsetSize: OffsetSizeTrait>(
             to_type.clone(),
             array.len(),
             Some(data.null_count()),
-            data.null_bitmap().clone().map(|bitmap| bitmap.bits),
+            data.null_bitmap().cloned().map(|bitmap| bitmap.bits),
             array.offset(),
             // reuse offset buffer
             data.buffers().to_vec(),

diff --git a/arrow/src/compute/kernels/substring.rs b/arrow/src/compute/kernels/substring.rs
@@ -27,7 +27,7 @@ use crate::{
 fn generic_substring<OffsetSize: StringOffsetSizeTrait>(
     array: &GenericStringArray<OffsetSize>,
     start: OffsetSize,
-    length: &Option<OffsetSize>,
+    length: Option<&OffsetSize>,
 ) -> Result<ArrayRef> {
     let offsets = array.value_offsets();
     let null_bit_buffer = array.data_ref().null_buffer().cloned();
@@ -122,7 +122,7 @@ fn generic_substring<OffsetSize: StringOffsetSizeTrait>(
 /// # use arrow::array::StringArray;
 /// # use arrow::compute::kernels::substring::substring;
 /// let array = StringArray::from(vec![Some("E=mc²")]);
-/// let result = substring(&array, -1, &None).unwrap();
+/// let result = substring(&array, -1, None).unwrap();
 /// let result = result.as_any().downcast_ref::<StringArray>().unwrap();
 /// assert_eq!(result.value(0).as_bytes(), &[0x00B2]); // invalid utf-8 format
 /// # }
@@ -133,7 +133,7 @@ fn generic_substring<OffsetSize: StringOffsetSizeTrait>(
 pub fn substring(
     array: &dyn Array,
     start: i64,
-    length: &Option<u64>,
+    length: Option<&u64>,
 ) -> Result<ArrayRef> {
     match array.data_type() {
         DataType::LargeUtf8 => generic_substring(
@@ -142,15 +142,15 @@ pub fn substring(
                 .downcast_ref::<LargeStringArray>()
                 .expect("A large string is expected"),
             start,
-            &length.map(|e| e as i64),
+            length.map(|e| *e as i64).as_ref(),
         ),
         DataType::Utf8 => generic_substring(
             array
                 .as_any()
                 .downcast_ref::<StringArray>()
                 .expect("A string is expected"),
             start as i32,
-            &length.map(|e| e as i32),
+            length.map(|e| *e as i32).as_ref(),
         ),
         _ => Err(ArrowError::ComputeError(format!(
             "substring does not support type {:?}",
@@ -206,7 +206,7 @@ mod tests {
         cases.into_iter().try_for_each::<_, Result<()>>(
             |(array, start, length, expected)| {
                 let array = T::from(array);
-                let result: ArrayRef = substring(&array, start, &length)?;
+                let result: ArrayRef = substring(&array, start, length.as_ref())?;
                 assert_eq!(array.len(), result.len());
 
                 let result = result.as_any().downcast_ref::<T>().unwrap();
@@ -287,7 +287,7 @@ mod tests {
         cases.into_iter().try_for_each::<_, Result<()>>(
             |(array, start, length, expected)| {
                 let array = StringArray::from(array);
-                let result = substring(&array, start, &length)?;
+                let result = substring(&array, start, length.as_ref())?;
                 assert_eq!(array.len(), result.len());
                 let result = result.as_any().downcast_ref::<StringArray>().unwrap();
                 let expected = StringArray::from(expected);

diff --git a/arrow/src/csv/reader.rs b/arrow/src/csv/reader.rs
@@ -509,7 +509,7 @@ impl<R: Read> Iterator for Reader<R> {
             &self.batch_records[..read_records],
             self.schema.fields(),
             Some(self.schema.metadata.clone()),
-            &self.projection,
+            self.projection.as_ref(),
             self.line_number,
             format,
         );
@@ -526,12 +526,12 @@ fn parse(
     rows: &[StringRecord],
     fields: &[Field],
     metadata: Option<std::collections::HashMap<String, String>>,
-    projection: &Option<Vec<usize>>,
+    projection: Option<&Vec<usize>>,
     line_number: usize,
     datetime_format: Option<&str>,
 ) -> Result<RecordBatch> {
     let projection: Vec<usize> = match projection {
-        Some(ref v) => v.clone(),
+        Some(v) => v.clone(),
         None => fields.iter().enumerate().map(|(i, _)| i).collect(),
     };
 

diff --git a/arrow/src/csv/writer.rs b/arrow/src/csv/writer.rs
@@ -221,7 +221,7 @@ impl<W: Write> Writer<W> {
                         .to_string()
                 }
                 DataType::Timestamp(time_unit, time_zone) => {
-                    self.handle_timestamp(time_unit, time_zone, row_index, col)?
+                    self.handle_timestamp(time_unit, time_zone.as_ref(), row_index, col)?
                 }
                 DataType::Decimal(..) => make_string_from_decimal(col, row_index)?,
                 t => {
@@ -242,7 +242,7 @@ impl<W: Write> Writer<W> {
     fn handle_timestamp(
         &self,
         time_unit: &TimeUnit,
-        _time_zone: &Option<String>,
+        _time_zone: Option<&String>,
         row_index: usize,
         col: &ArrayRef,
     ) -> Result<String> {
@@ -280,7 +280,7 @@ impl<W: Write> Writer<W> {
     fn handle_timestamp(
         &self,
         time_unit: &TimeUnit,
-        time_zone: &Option<String>,
+        time_zone: Option<&String>,
         row_index: usize,
         col: &ArrayRef,
     ) -> Result<String> {

diff --git a/arrow/src/datatypes/field.rs b/arrow/src/datatypes/field.rs
@@ -91,8 +91,8 @@ impl Field {
 
     /// Returns the immutable reference to the `Field`'s optional custom metadata.
     #[inline]
-    pub const fn metadata(&self) -> &Option<BTreeMap<String, String>> {
-        &self.metadata
+    pub const fn metadata(&self) -> Option<&BTreeMap<String, String>> {
+        self.metadata.as_ref()
     }
 
     /// Returns an immutable reference to the `Field`'s name.

diff --git a/arrow/src/datatypes/mod.rs b/arrow/src/datatypes/mod.rs
@@ -1276,7 +1276,7 @@ mod tests {
         assert!(f1.try_merge(&f2).is_ok());
         assert!(f1.metadata().is_some());
         assert_eq!(
-            f1.metadata().clone().unwrap(),
+            f1.metadata().cloned().unwrap(),
             [
                 ("foo".to_string(), "bar".to_string()),
                 ("foo2".to_string(), "bar2".to_string())
@@ -1297,7 +1297,7 @@ mod tests {
         assert!(f1.try_merge(&f2).is_ok());
         assert!(f1.metadata().is_some());
         assert_eq!(
-            f1.metadata().clone().unwrap(),
+            f1.metadata().cloned().unwrap(),
             [("foo".to_string(), "bar".to_string())]
                 .iter()
                 .cloned()

diff --git a/parquet/src/column/writer.rs b/parquet/src/column/writer.rs
@@ -270,8 +270,8 @@ impl<T: DataType> ColumnWriterImpl<T> {
         values: &[T::T],
         def_levels: Option<&[i16]>,
         rep_levels: Option<&[i16]>,
-        min: &Option<T::T>,
-        max: &Option<T::T>,
+        min: Option<&T::T>,
+        max: Option<&T::T>,
         null_count: Option<u64>,
         distinct_count: Option<u64>,
     ) -> Result<usize> {
@@ -376,9 +376,7 @@ impl<T: DataType> ColumnWriterImpl<T> {
         def_levels: Option<&[i16]>,
         rep_levels: Option<&[i16]>,
     ) -> Result<usize> {
-        self.write_batch_internal(
-            values, def_levels, rep_levels, &None, &None, None, None,
-        )
+        self.write_batch_internal(values, def_levels, rep_levels, None, None, None, None)
     }
 
     /// Writer may optionally provide pre-calculated statistics for this batch, in which case we do
@@ -389,8 +387,8 @@ impl<T: DataType> ColumnWriterImpl<T> {
         values: &[T::T],
         def_levels: Option<&[i16]>,
         rep_levels: Option<&[i16]>,
-        min: &Option<T::T>,
-        max: &Option<T::T>,
+        min: Option<&T::T>,
+        max: Option<&T::T>,
         nulls_count: Option<u64>,
         distinct_count: Option<u64>,
     ) -> Result<usize> {
@@ -1487,8 +1485,8 @@ mod tests {
                 &[1, 2, 3, 4],
                 None,
                 None,
-                &Some(-17),
-                &Some(9000),
+                Some(&-17),
+                Some(&9000),
                 Some(21),
                 Some(55),
             )

diff --git a/parquet/src/file/metadata.rs b/parquet/src/file/metadata.rs
@@ -139,8 +139,8 @@ impl FileMetaData {
     /// ```shell
     /// parquet-mr version 1.8.0 (build 0fda28af84b9746396014ad6a415b90592a98b3b)
     /// ```
-    pub fn created_by(&self) -> &Option<String> {
-        &self.created_by
+    pub fn created_by(&self) -> Option<&str> {
+        self.created_by.as_deref()
     }
 
     /// Returns key_value_metadata of this file.