Fix List Sorting, Revert Removal of Rank Kernels #4747
Conversation
This reverts commit c06786f.
| out[*val as usize] = ix as u32; | ||
| } | ||
| Ok(out) | ||
| rank(values, value_options) |
There was a problem hiding this comment.
This is the fix for #4746
The issue is caused by the previous logic assigning distinct ranks to equal values. This was fine when used for dictionaries, as it just made the sort less stable, but the changes in #4613 used the same logic for the lexicographic style comparison for lists, which made this incorrect
There was a problem hiding this comment.
I think I get the gist of this. However sort.rs is very hard to review. Core methods aren't documented (esp. their parameters, like what is valids and why is it a combination of T and a u32? sort_impl for example has 5 parameters), the approach of ranking child values and using these ranks for the actual sorting is also quite implicit (but actually important to understand for this very bug).
Which issue does this PR close?
Closes #4746
Rationale for this change
Reverts #4703 and uses this to fix #4746
What changes are included in this PR?
Are there any user-facing changes?