Add support of sorting dictionary of other primitive arrays #2701
Conversation
arrow/src/compute/kernels/sort.rs
Outdated
| let value_indices_map = prepare_indices_map(&sorted_value_indices); | ||
| sort_primitive_dictionary::<_, _>(values, &value_indices_map, v, n, options, limit, cmp) | ||
| }, | ||
| DataType::Float32 => { |
There was a problem hiding this comment.
Perhaps we could collapse these match statements together, they appear to all be identical?
There was a problem hiding this comment.
Yea, forgot that this matches value types. 😅
arrow/src/compute/kernels/sort.rs
Outdated
| sort_primitive_dictionary::<_, _>(values, &value_indices_map, v, n, options, limit, cmp) | ||
| }, | ||
| DataType::Int16 => { | ||
| DataType::Int8 | DataType::Int16 | DataType::Int32 | DataType::Int64 | DataType::UInt8 | |
There was a problem hiding this comment.
I wonder if this should be a member function somewhere...
There was a problem hiding this comment.
Added one helper function for that.
arrow/src/compute/kernels/sort.rs
Outdated
| 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, _) | |
There was a problem hiding this comment.
Do we need to match the specific TimeUnits/IntervalUnits?
There was a problem hiding this comment.
No, I think. Just copied from above match statements. Removed them now.
|
Thanks. |
|
Benchmark runs are scheduled for baseline = 5146663 and contender = 2a0fc77. 2a0fc77 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #2700.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?