You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by al...@apache.org on 2021/06/23 10:29:38 UTC

[arrow-rs] branch active_release updated: remove unnecessary wraps in sortk (#445) (#483)

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

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


The following commit(s) were added to refs/heads/active_release by this push:
     new 563a506  remove unnecessary wraps in sortk (#445) (#483)
563a506 is described below

commit 563a5067a9e722808a298817c153071fb51a6535
Author: Andrew Lamb <an...@nerdnetworks.org>
AuthorDate: Wed Jun 23 06:29:26 2021 -0400

    remove unnecessary wraps in sortk (#445) (#483)
    
    Co-authored-by: Jiayu Liu <Ji...@users.noreply.github.com>
---
 arrow/src/compute/kernels/sort.rs | 96 +++++++++++++++++++++------------------
 1 file changed, 51 insertions(+), 45 deletions(-)

diff --git a/arrow/src/compute/kernels/sort.rs b/arrow/src/compute/kernels/sort.rs
index 9f55826..a08507e 100644
--- a/arrow/src/compute/kernels/sort.rs
+++ b/arrow/src/compute/kernels/sort.rs
@@ -163,7 +163,7 @@ pub fn sort_to_indices(
 
     let (v, n) = partition_validity(values);
 
-    match values.data_type() {
+    Ok(match values.data_type() {
         DataType::Boolean => sort_boolean(values, v, n, &options, limit),
         DataType::Int8 => {
             sort_primitive::<Int8Type, _>(values, v, n, cmp, &options, limit)
@@ -278,10 +278,12 @@ pub fn sort_to_indices(
             DataType::Float64 => {
                 sort_list::<i32, Float64Type>(values, v, n, &options, limit)
             }
-            t => Err(ArrowError::ComputeError(format!(
-                "Sort not supported for list type {:?}",
-                t
-            ))),
+            t => {
+                return Err(ArrowError::ComputeError(format!(
+                    "Sort not supported for list type {:?}",
+                    t
+                )))
+            }
         },
         DataType::LargeList(field) => match field.data_type() {
             DataType::Int8 => sort_list::<i64, Int8Type>(values, v, n, &options, limit),
@@ -304,10 +306,12 @@ pub fn sort_to_indices(
             DataType::Float64 => {
                 sort_list::<i64, Float64Type>(values, v, n, &options, limit)
             }
-            t => Err(ArrowError::ComputeError(format!(
-                "Sort not supported for list type {:?}",
-                t
-            ))),
+            t => {
+                return Err(ArrowError::ComputeError(format!(
+                    "Sort not supported for list type {:?}",
+                    t
+                )))
+            }
         },
         DataType::FixedSizeList(field, _) => match field.data_type() {
             DataType::Int8 => sort_list::<i32, Int8Type>(values, v, n, &options, limit),
@@ -330,10 +334,12 @@ pub fn sort_to_indices(
             DataType::Float64 => {
                 sort_list::<i32, Float64Type>(values, v, n, &options, limit)
             }
-            t => Err(ArrowError::ComputeError(format!(
-                "Sort not supported for list type {:?}",
-                t
-            ))),
+            t => {
+                return Err(ArrowError::ComputeError(format!(
+                    "Sort not supported for list type {:?}",
+                    t
+                )))
+            }
         },
         DataType::Dictionary(key_type, value_type)
             if *value_type.as_ref() == DataType::Utf8 =>
@@ -363,17 +369,21 @@ pub fn sort_to_indices(
                 DataType::UInt64 => {
                     sort_string_dictionary::<UInt64Type>(values, v, n, &options, limit)
                 }
-                t => Err(ArrowError::ComputeError(format!(
-                    "Sort not supported for dictionary key type {:?}",
-                    t
-                ))),
+                t => {
+                    return Err(ArrowError::ComputeError(format!(
+                        "Sort not supported for dictionary key type {:?}",
+                        t
+                    )))
+                }
             }
         }
-        t => Err(ArrowError::ComputeError(format!(
-            "Sort not supported for data type {:?}",
-            t
-        ))),
-    }
+        t => {
+            return Err(ArrowError::ComputeError(format!(
+                "Sort not supported for data type {:?}",
+                t
+            )))
+        }
+    })
 }
 
 /// Options that define how sort kernels should behave
@@ -396,14 +406,13 @@ impl Default for SortOptions {
 }
 
 /// Sort primitive values
-#[allow(clippy::unnecessary_wraps)]
 fn sort_boolean(
     values: &ArrayRef,
     value_indices: Vec<u32>,
     null_indices: Vec<u32>,
     options: &SortOptions,
     limit: Option<usize>,
-) -> Result<UInt32Array> {
+) -> UInt32Array {
     let values = values
         .as_any()
         .downcast_ref::<BooleanArray>()
@@ -469,11 +478,10 @@ fn sort_boolean(
         vec![],
     );
 
-    Ok(UInt32Array::from(result_data))
+    UInt32Array::from(result_data)
 }
 
 /// Sort primitive values
-#[allow(clippy::unnecessary_wraps)]
 fn sort_primitive<T, F>(
     values: &ArrayRef,
     value_indices: Vec<u32>,
@@ -481,7 +489,7 @@ fn sort_primitive<T, F>(
     cmp: F,
     options: &SortOptions,
     limit: Option<usize>,
-) -> Result<UInt32Array>
+) -> UInt32Array
 where
     T: ArrowPrimitiveType,
     T::Native: std::cmp::PartialOrd,
@@ -549,7 +557,7 @@ where
         vec![],
     );
 
-    Ok(UInt32Array::from(result_data))
+    UInt32Array::from(result_data)
 }
 
 // insert valid and nan values in the correct order depending on the descending flag
@@ -574,7 +582,7 @@ fn sort_string<Offset: StringOffsetSizeTrait>(
     null_indices: Vec<u32>,
     options: &SortOptions,
     limit: Option<usize>,
-) -> Result<UInt32Array> {
+) -> UInt32Array {
     let values = values
         .as_any()
         .downcast_ref::<GenericStringArray<Offset>>()
@@ -597,7 +605,7 @@ fn sort_string_dictionary<T: ArrowDictionaryKeyType>(
     null_indices: Vec<u32>,
     options: &SortOptions,
     limit: Option<usize>,
-) -> Result<UInt32Array> {
+) -> UInt32Array {
     let values: &DictionaryArray<T> = as_dictionary_array::<T>(values);
 
     let keys: &PrimitiveArray<T> = &values.keys_array();
@@ -620,7 +628,6 @@ fn sort_string_dictionary<T: ArrowDictionaryKeyType>(
 
 /// shared implementation between dictionary encoded and plain string arrays
 #[inline]
-#[allow(clippy::unnecessary_wraps)]
 fn sort_string_helper<'a, A: Array, F>(
     values: &'a A,
     value_indices: Vec<u32>,
@@ -628,7 +635,7 @@ fn sort_string_helper<'a, A: Array, F>(
     options: &SortOptions,
     limit: Option<usize>,
     value_fn: F,
-) -> Result<UInt32Array>
+) -> UInt32Array
 where
     F: Fn(&'a A, u32) -> &str,
 {
@@ -661,23 +668,22 @@ where
     if options.nulls_first {
         nulls.append(&mut valid_indices);
         nulls.truncate(len);
-        return Ok(UInt32Array::from(nulls));
+        UInt32Array::from(nulls)
+    } else {
+        // no need to sort nulls as they are in the correct order already
+        valid_indices.append(&mut nulls);
+        valid_indices.truncate(len);
+        UInt32Array::from(valid_indices)
     }
-
-    // no need to sort nulls as they are in the correct order already
-    valid_indices.append(&mut nulls);
-    valid_indices.truncate(len);
-    Ok(UInt32Array::from(valid_indices))
 }
 
-#[allow(clippy::unnecessary_wraps)]
 fn sort_list<S, T>(
     values: &ArrayRef,
     value_indices: Vec<u32>,
     mut null_indices: Vec<u32>,
     options: &SortOptions,
     limit: Option<usize>,
-) -> Result<UInt32Array>
+) -> UInt32Array
 where
     S: OffsetSizeTrait,
     T: ArrowPrimitiveType,
@@ -727,12 +733,12 @@ where
     if options.nulls_first {
         null_indices.append(&mut valid_indices);
         null_indices.truncate(len);
-        return Ok(UInt32Array::from(null_indices));
+        UInt32Array::from(null_indices)
+    } else {
+        valid_indices.append(&mut null_indices);
+        valid_indices.truncate(len);
+        UInt32Array::from(valid_indices)
     }
-
-    valid_indices.append(&mut null_indices);
-    valid_indices.truncate(len);
-    Ok(UInt32Array::from(valid_indices))
 }
 
 /// Compare two `Array`s based on the ordering defined in [ord](crate::array::ord).