jorgecarleitao commented on a change in pull request #8517:
URL: https://github.com/apache/arrow/pull/8517#discussion_r511491453
##########
File path: rust/arrow/src/compute/kernels/sort.rs
##########
@@ -453,49 +466,46 @@ pub fn lexsort(columns: &[SortColumn]) ->
Result<Vec<ArrayRef>> {
/// Sort elements lexicographically from a list of `ArrayRef` into an unsigned
integer
/// (`UInt32Array`) of indices.
pub fn lexsort_to_indices(columns: &[SortColumn]) -> Result<UInt32Array> {
+ if columns.len() == 0 {
+ return Err(ArrowError::InvalidArgumentError(
+ "Sort requires at least one column".to_string(),
+ ));
+ }
if columns.len() == 1 {
// fallback to non-lexical sort
let column = &columns[0];
return sort_to_indices(&column.values, column.options);
}
- let mut row_count = None;
+ let row_count = columns[0].values.len();
+ if columns.iter().any(|item| item.values.len() != row_count) {
+ return Err(ArrowError::ComputeError(
+ "lexical sort columns have different row counts".to_string(),
+ ));
+ };
+
// convert ArrayRefs to OrdArray trait objects and perform row count check
let flat_columns = columns
.iter()
- .map(|column| -> Result<(&Array, Box<OrdArray>, SortOptions)> {
- // row count check
- let curr_row_count = column.values.len() - column.values.offset();
- match row_count {
- None => {
- row_count = Some(curr_row_count);
- }
- Some(cnt) => {
- if curr_row_count != cnt {
- return Err(ArrowError::ComputeError(
- "lexical sort columns have different row
counts".to_string(),
- ));
- }
- }
- }
- // flatten and convert to OrdArray
+ .map(|column| -> Result<(&Array, DynComparator, SortOptions)> {
+ // flatten and convert build comparators
Ok((
column.values.as_ref(),
- as_ordarray(&column.values)?,
+ build_compare(column.values.as_ref(), column.values.as_ref())?,
Review comment:
This is the main change: we no longer create an `OrdArray`. Instead,
return a heap-allocated function (`DynComparator`) that can compare stuff.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
