Remove reference indirection for Copy types in substring (#1576)

apache · Apr 17, 2022 · 791bfc4 · 791bfc4
1 parent 786792c
commit 791bfc4
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 18 deletions.
diff --git a/arrow/benches/string_kernels.rs b/arrow/benches/string_kernels.rs
@@ -26,12 +26,7 @@ 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).as_ref(),
-    )
-    .unwrap();
+    substring(criterion::black_box(arr), start, Some(length as u64)).unwrap();
 }
 
 fn add_benchmark(c: &mut Criterion) {

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();
@@ -47,9 +47,9 @@ fn generic_substring<OffsetSize: StringOffsetSizeTrait>(
 
     let cal_new_length: Box<dyn Fn(OffsetSize, OffsetSize) -> OffsetSize> =
         if let Some(length) = length {
-            Box::new(|start: OffsetSize, end: OffsetSize| (*length).min(end - start))
+            Box::new(move |start: OffsetSize, end: OffsetSize| length.min(end - start))
         } else {
-            Box::new(|start: OffsetSize, end: OffsetSize| end - start)
+            Box::new(move |start: OffsetSize, end: OffsetSize| end - start)
         };
 
     // start and end offsets for each substring
@@ -130,27 +130,23 @@ fn generic_substring<OffsetSize: StringOffsetSizeTrait>(
 ///
 /// # Error
 /// this function errors when the passed array is not a \[Large\]String array.
-pub fn substring(
-    array: &dyn Array,
-    start: i64,
-    length: Option<&u64>,
-) -> Result<ArrayRef> {
+pub fn substring(array: &dyn Array, start: i64, length: Option<u64>) -> Result<ArrayRef> {
     match array.data_type() {
         DataType::LargeUtf8 => generic_substring(
             array
                 .as_any()
                 .downcast_ref::<LargeStringArray>()
                 .expect("A large string is expected"),
             start,
-            length.map(|e| *e as i64).as_ref(),
+            length.map(|e| e as i64),
         ),
         DataType::Utf8 => generic_substring(
             array
                 .as_any()
                 .downcast_ref::<StringArray>()
                 .expect("A string is expected"),
             start as i32,
-            length.map(|e| *e as i32).as_ref(),
+            length.map(|e| e as i32),
         ),
         _ => Err(ArrowError::ComputeError(format!(
             "substring does not support type {:?}",
@@ -206,7 +202,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.as_ref())?;
+                let result: ArrayRef = substring(&array, start, length)?;
                 assert_eq!(array.len(), result.len());
 
                 let result = result.as_any().downcast_ref::<T>().unwrap();
@@ -287,7 +283,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.as_ref())?;
+                let result = substring(&array, start, length)?;
                 assert_eq!(array.len(), result.len());
                 let result = result.as_any().downcast_ref::<StringArray>().unwrap();
                 let expected = StringArray::from(expected);