Skip to content

Commit

Permalink
implement the full set of sorts on QueryManyIter (#13443)
Browse files Browse the repository at this point in the history
# Objective

~Blocked on #13417~

Motivation is the same as in #13417. If users can sort `QueryIter`, to
only makes sense to also allow them to use this functionality on
`QueryManyIter`.

## Solution

Also implement the sorts on `QueryManyIter`. 

The implementation of the sorts themselves are mostly the same as with
`QueryIter` in #13417.
They differ in that they re-use the `entity_iter` passed to the
`iter_many`, and internally call `iter_many_unchecked_manual` on the
lens `QueryState` with it.
These methods also return a different struct, `QuerySortedManyIter`,
because there is no longer a guarantee of unique entities.
`QuerySortedManyIter` implements the various `Iterator` traits for
read-only iteration, as `QueryManyIter` does + `DoubleEndedIterator`.
For mutable iteration, there is both a `fetch_next` and a
`fetch_next_back` method. However, they only become available after the
user calls `collect_inner` on `QuerySortedManyIter` first. This collects
the inner `entity_iter` (this is the sorted one, **not** the original
the user passed) to drop all query lens items to avoid aliasing.
When TAITs are available this `collect_inner` could be hidden away,
until then it is unfortunately not possible to elide this without either
regressing read-only iteration, or introducing a whole new type, mostly
being a copy of `QuerySortedIter`.

As a follow-up we could add a `entities_all_unique` method to check
whether the entity list consists of only unique entities, and then
return a `QuerySortedIter` from it (under opaque impl Trait if need be),
*allowing mutable `Iterator` trait iteration* over what was originally
an `iter_many` call.
Such a method can also be added to `QueryManyIter`, albeit needing a
separate, new return type.

## Testing

I've switched the third example/doc test under `sort` out for one that
shows the collect_inner/fetch_next_back functionality, otherwise the
examples are the same as in #13417, adjusted to use `iter_many` instead
of `iter`.

The `query-iter-many-sorts` test checks for equivalence to the
underlying sorts.
The test after shows that these sorts *do not* panic after
`fetch`/`fetch_next` calls.

## Changelog

Added `sort`, `sort_unstable`, `sort_by`, `sort_unstable_by`,
`sort_by_key`, `sort_by_cached_key` to `QueryManyIter`.
Added `QuerySortedManyIter`.
  • Loading branch information
Victoronz authored Dec 3, 2024
1 parent d4883a9 commit aa600ae
Showing 1 changed file with 940 additions and 32 deletions.
Loading

0 comments on commit aa600ae

Please sign in to comment.