-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[nexus] Experiment with using closer-to-raw-SQL pagination #7717
base: main
Are you sure you want to change the base?
Conversation
@@ -88,14 +88,14 @@ type BoxedQuery = diesel::query_builder::BoxedSqlQuery< | |||
// | |||
// But this relies on nightly features. | |||
pub struct QueryBuilder { | |||
query: BoxedQuery, | |||
query: Option<BoxedQuery>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make the RawPaginator
API with the interface that exists, these functions needed to act on &mut self
instead of self
.
This was a breaking change, and it requires using an Option<BoxQuery>
internally, but that inner type detail should not be visible to callers.
} | ||
|
||
impl<M1, M2> RawPaginatorWithParams<'_, (M1, M2)> { | ||
pub fn paginate_by_columns<BindSt1, BindSt2>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems way for feasible to make this generic across a tuple of an arbitrary length -- the generic parameters are much simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I picked the affinity "list" APIs because those couldn't use the old pagination functions anyway - they had to be manual, to paginate over a UNION
subquery.
I've converted everything in this file to use the RawPaginator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file also uses a combination of paginated
and paginated_multicolumn
.
I picked it as a case where "there are a lot of joins + filters", to see if the new helpers can work.
The
pagination
functions - in particular, thepaginated_multicolumn
function - is notoriousfor having incredibly complex generic parameters.
For context, see:
omicron/nexus/db-queries/src/db/pagination.rs
Lines 82 to 177 in feae60c
To reduce this pain, this PR provides pagination helpers which side-step Diesel's type-checking,
aiming to provide a more flexible option for pagination which can work across arbitrary JOINs,
UNIONs, etc, without relying on compile-time type-checking.