Add backward seek-based pagination support for the search endpoint #10793
Conversation
This makes "seek=" and "seek=-" valid, and "seek=-" could be treated as paginating backward from the tail.
...based on direction Most of this should be intuitive, as it simply flips the comparison operator. To make it easier to identify the difference, I chose to implement it in a single function and place it next to the forward ordering conditions. The only part that requires a bit of thought is the case involving nulls last.
…` fn This also remove the obsolete `Paginated::is_explicit_page()`.
…d pagination ...when seeking backward involved.
…e to page For instance, given rows with the following: | a | b | c | ... | 1 | 2 | 3 | When performing forward seek pagination with page size as 3 to the last page, let's say we have the last page as 1, 2, 3. When performing backward seek pagination, we reverse the order of the query so that we can retrieve the data from the tail. This results in 3, 2, 1. However, we would like to maintain consistent ordering as we view it with forward pagination, namely 1, 2, 3. Therefore, we need to reverse the results again to maintain the original ordering.
eth3lbert
added
C-enhancement ✨
eth3lbert
force-pushed
the
seek-pagination-backward
branch
from
a05bcbf to
44d5e67
Compare
eth3lbert
requested review from
Turbo87 and
LawnGnome
and removed request for
Turbo87
There was a problem hiding this comment.
This is currently only tested with a page size of 1. It would be better to expand the tests to include varying page sizes.
|
Sorry, I skimmed this yesterday, but didn't have time to give it a full review. I still haven't, but my rough thought process was basically "oh, this is way better than I would have thought", then I realised that GitHub had collapsed the That said, the changes are mostly mechanical, and feel like we could eventually abstract at least some of them behind a macro or two. Against that, So, I guess I wouldn't block this, but I'm not super enthusiastic about it, either. Do we have a strong use case for this? (Maybe more a question for @GuillaumeGomez.) I'm probably -0 right now, but could be convinced, particularly if we came up with a way to abstract some of the details out of the controllers proper. |
|
Thanks for the feedback!
Cursor-based pagination does indeed complicate things, especially with
This currently serves mainly for docs.rs, and I'm not personally pressing for it :D |
|
Well, the navigation experience is suboptimal, so that would be very welcome. However if you think that a better (technical) solution could be achieved, then we can wait for it. |
Previously, we were returning
nullforprev_pagein search endpoint. With this PR, we now provide the URL forprev_page. For other endpoints to support backward seeking, they must have the page options with seek backward enabled (.enable_seek_backward(true)). Otherwise, without this, any seek param starting with-(indicating backward seeking) will result in an error!It would be easier to review this PR commit by commit 😉
Resolves #10349.