viirya commented on code in PR #2701:
URL: https://github.com/apache/arrow-rs/pull/2701#discussion_r971262523
##########
arrow/src/compute/kernels/sort.rs:
##########
@@ -314,119 +314,40 @@ pub fn sort_to_indices(
}
},
DataType::Dictionary(_, _) => {
+ let value_null_first = if options.descending {
+ // When sorting dictionary in descending order, we take
inverse of of null ordering
+ // when sorting the values. Because if `nulls_first` is true,
null must be in front
+ // of non-null value. As we take the sorted order of value
array to sort dictionary
+ // keys, these null values will be treated as smallest ones
and be sorted to the end
+ // of sorted result. So we set `nulls_first` to false when
sorting dictionary value
+ // array to make them as largest ones, then null values will
be put at the beginning
+ // of sorted dictionary result.
+ !options.nulls_first
+ } else {
+ options.nulls_first
+ };
+ let value_options = Some(SortOptions {
+ descending: false,
+ nulls_first: value_null_first,
+ });
downcast_dictionary_array!(
values => match values.values().data_type() {
- DataType::Int8 => {
- let dict_values = values.values();
- let value_null_first = if options.descending {
- // When sorting dictionary in descending order, we
take inverse of of null ordering
- // when sorting the values. Because if
`nulls_first` is true, null must be in front
- // of non-null value. As we take the sorted order
of value array to sort dictionary
- // keys, these null values will be treated as
smallest ones and be sorted to the end
- // of sorted result. So we set `nulls_first` to
false when sorting dictionary value
- // array to make them as largest ones, then null
values will be put at the beginning
- // of sorted dictionary result.
- !options.nulls_first
- } else {
- options.nulls_first
- };
- let value_options = Some(SortOptions { descending:
false, nulls_first: value_null_first });
- let sorted_value_indices =
sort_to_indices(dict_values, value_options, None)?;
- let value_indices_map =
prepare_indices_map(&sorted_value_indices);
- sort_primitive_dictionary::<_, _>(values,
&value_indices_map, v, n, options, limit, cmp)
- },
- DataType::Int16 => {
+ DataType::Int8 | DataType::Int16 | DataType::Int32 |
DataType::Int64 | DataType::UInt8 |
+ DataType::UInt16 | DataType::UInt32 | DataType::UInt64 |
DataType::Float32 | DataType::Float64 |
+ DataType::Date32 | DataType::Date64 |
DataType::Time32(Second) | DataType::Time32(Millisecond) |
+ DataType::Time64(Microsecond) |
DataType::Time64(Nanosecond) | DataType::Timestamp(Second, _) |
Review Comment:
No, I think. Just copied from above match statements. Removed them now.
##########
arrow/src/compute/kernels/sort.rs:
##########
@@ -314,119 +314,40 @@ pub fn sort_to_indices(
}
},
DataType::Dictionary(_, _) => {
+ let value_null_first = if options.descending {
+ // When sorting dictionary in descending order, we take
inverse of of null ordering
+ // when sorting the values. Because if `nulls_first` is true,
null must be in front
+ // of non-null value. As we take the sorted order of value
array to sort dictionary
+ // keys, these null values will be treated as smallest ones
and be sorted to the end
+ // of sorted result. So we set `nulls_first` to false when
sorting dictionary value
+ // array to make them as largest ones, then null values will
be put at the beginning
+ // of sorted dictionary result.
+ !options.nulls_first
+ } else {
+ options.nulls_first
+ };
+ let value_options = Some(SortOptions {
+ descending: false,
+ nulls_first: value_null_first,
+ });
downcast_dictionary_array!(
values => match values.values().data_type() {
- DataType::Int8 => {
- let dict_values = values.values();
- let value_null_first = if options.descending {
- // When sorting dictionary in descending order, we
take inverse of of null ordering
- // when sorting the values. Because if
`nulls_first` is true, null must be in front
- // of non-null value. As we take the sorted order
of value array to sort dictionary
- // keys, these null values will be treated as
smallest ones and be sorted to the end
- // of sorted result. So we set `nulls_first` to
false when sorting dictionary value
- // array to make them as largest ones, then null
values will be put at the beginning
- // of sorted dictionary result.
- !options.nulls_first
- } else {
- options.nulls_first
- };
- let value_options = Some(SortOptions { descending:
false, nulls_first: value_null_first });
- let sorted_value_indices =
sort_to_indices(dict_values, value_options, None)?;
- let value_indices_map =
prepare_indices_map(&sorted_value_indices);
- sort_primitive_dictionary::<_, _>(values,
&value_indices_map, v, n, options, limit, cmp)
- },
- DataType::Int16 => {
+ DataType::Int8 | DataType::Int16 | DataType::Int32 |
DataType::Int64 | DataType::UInt8 |
Review Comment:
Added one helper function for that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
