[nexus] Experiment with using closer-to-raw-SQL pagination by smklein · Pull Request #7717 · oxidecomputer/omicron

smklein · 2025-03-04T01:48:22Z

The pagination functions - in particular, the paginated_multicolumn function - is notorious
for having incredibly complex generic parameters.

For context, see:

omicron/nexus/db-queries/src/db/pagination.rs

Lines 82 to 177 in feae60c

    
           pub fn paginated_multicolumn<T, C1, C2, M1, M2>( 
        
               query: T, 
        
               (c1, c2): (C1, C2), 
        
               pagparams: &DataPageParams<'_, (M1, M2)>, 
        
           ) -> <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output 
        
           where 
        
           // T is a table^H^H^H^H^Hquery source which can create a BoxedQuery. 
        
               T: QuerySource, 
        
               T: AsQuery, 
        
               <T as QuerySource>::DefaultSelection: 
        
                   Expression<SqlType = <T as AsQuery>::SqlType>, 
        
               T::Query: query_methods::BoxedDsl<'static, Pg>, 
        
           // Required for...everything. 
        
               <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output: QueryDsl, 
        
           // C1 & C2 are columns which appear in T. 
        
               C1: 'static + Column + Copy + ExpressionMethods, 
        
               C2: 'static + Column + Copy + ExpressionMethods, 
        
           // Required to compare the columns with the marker types. 
        
               C1::SqlType: SqlType, 
        
               C2::SqlType: SqlType, 
        
               M1: Clone + AsExpression<C1::SqlType>, 
        
               M2: Clone + AsExpression<C2::SqlType>, 
        
           // Necessary for `query.limit(...)` 
        
               <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output: 
        
                   query_methods::LimitDsl< 
        
                       Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output, 
        
                   >, 
        
           // Necessary for "query.order(c1.desc())" 
        
               <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output: 
        
                   query_methods::OrderDsl< 
        
                       Desc<C1>, 
        
                       Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output, 
        
                   >, 
        
           // Necessary for "query.order(...).then_order_by(c2.desc())" 
        
               <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output: 
        
                   query_methods::ThenOrderDsl< 
        
                       Desc<C2>, 
        
                       Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output, 
        
                   >, 
        
           // Necessary for "query.order(c1.asc())" 
        
               <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output: 
        
                   query_methods::OrderDsl< 
        
                       Asc<C1>, 
        
                       Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output, 
        
                   >, 
        
           // Necessary for "query.order(...).then_order_by(c2.asc())" 
        
               <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output: 
        
                   query_methods::ThenOrderDsl< 
        
                       Asc<C2>, 
        
                       Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output, 
        
                   >, 
        
           // We'd like to be able to call: 
        
           // 
        
           //  c1.eq(v1).and(c2.gt(v2)) 
        
           // 
        
           // This means "c1.eq(v1)" must implement BoolExpressionMethods, and 
        
           // satisfy the requirements of the ".and" method. 
        
           // 
        
           // The LHS (c1.eq(v1)) must be a boolean expression: 
        
               Eq<C1, M1>: Expression<SqlType = Bool>, 
        
           // The RHS (c2.gt(v2)) must be a boolean expression: 
        
               Gt<C2, M2>: Expression<SqlType = Bool>, 
        
           // Putting it together, we should be able to filter by LHS.and(RHS): 
        
               <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output: 
        
                   query_methods::FilterDsl< 
        
                       And<Eq<C1, M1>, Gt<C2, M2>>, 
        
                       Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output, 
        
                   >, 
        
           // We'd also like to be able to call: 
        
           // 
        
           //  c1.eq(v1).and(c2.lt(v2)) 
        
           // 
        
           // We've already defined the bound on the LHS, so we add the equivalent 
        
           // bounds on the RHS for the "Less than" variant. 
        
               Lt<C2, M2>: Expression<SqlType = Bool>, 
        
               <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output: 
        
                   query_methods::FilterDsl< 
        
                       And<Eq<C1, M1>, Lt<C2, M2>>, 
        
                       Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output, 
        
                   >, 
        
           // Necessary for "query.or_filter(c1.gt(v1))" 
        
               <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output: 
        
                   query_methods::OrFilterDsl< 
        
                       Gt<C1, M1>, 
        
                       Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output, 
        
                   >, 
        
           // Necessary for "query.or_filter(c1.lt(v1))" 
        
               <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output: 
        
                   query_methods::OrFilterDsl< 
        
                       Lt<C1, M1>, 
        
                       Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output, 
        
                   >, 
        
           {

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.

smklein · 2025-03-04T01:51:15Z

 // But this relies on nightly features.
 pub struct QueryBuilder {
-    query: BoxedQuery,
+    query: Option<BoxedQuery>,


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.

smklein · 2025-03-04T01:52:25Z

+}
+
+impl<M1, M2> RawPaginatorWithParams<'_, (M1, M2)> {
+    pub fn paginate_by_columns<BindSt1, BindSt2>(


It seems way for feasible to make this generic across a tuple of an arbitrary length -- the generic parameters are much simpler.

smklein · 2025-03-04T01:53:41Z

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.

smklein · 2025-03-04T01:54:59Z

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.

smklein added 30 commits January 30, 2025 12:08

[nexus] Add Affinity/Anti-Affinity Groups to API (unimplemented)

c9fb7a6

[nexus] Add Affinity/Anti-Affinity groups to database

4020517

[nexus] Add CRUD implementations for Affinity/Anti-Affinity Groups

8f1d37c

[nexus] Consider Affinity/Anti-Affinity Groups during instance placement

772e64f

[nexus] Implement Affinity/Anti-Affinity Groups in external API

d8cff32

fix policy tests

161f9d6

Merge branch 'affinity-db-crud' into affinity-instance-integration

df119b6

Merge branch 'affinity-instance-integration' into affinity-integration

83a26a4

fmt

8dc0825

Merge branch 'affinity-api' into affinity-db-model

e3113ff

Merge branch 'affinity-db-model' into affinity-db-crud

789bc97

Merge branch 'affinity-db-crud' into affinity-instance-integration

5e21f34

Merge branch 'affinity-instance-integration' into affinity-integration

fa9461b

tags

4e9cebc

doc comments

6cfca2d

Merge branch 'affinity-api' into affinity-db-model

a1c97d4

Merge branch 'main' into affinity-api

050b4c5

typed UUID

4b08032

Merge branch 'affinity-api' into affinity-db-model

8bc8f0c

Typed UUID

900f09c

Merge branch 'affinity-db-model' into affinity-db-crud

195e167

Typed UUID

f2ebe31

Merge branch 'affinity-db-crud' into affinity-instance-integration

62c38ec

Merge branch 'affinity-instance-integration' into affinity-integration

85985c1

UUID typing

1326116

comments

aba9596

Merge branch 'affinity-db-model' into affinity-db-crud

a271f1d

review feedback

1ad0101

comment

4d26262

clippy

6ae1910

smklein added 2 commits March 3, 2025 11:40

Merge branch 'aa-group-of-group' into affinity-expose-name

fa358bf

[nexus] Experiment with using closer-to-raw-SQL pagination

b15b883

smklein commented Mar 4, 2025

View reviewed changes

smklein added 8 commits March 3, 2025 17:57

Merge branch 'main' into vmm-reserve-bench

a3e885a

Merge branch 'vmm-reserve-bench' into vmm-reduce-contention

72ecbf2

Merge branch 'vmm-reduce-contention' into aa-group-of-group

c39e421

Merge branch 'aa-group-of-group' into affinity-expose-name

15b67bd

Merge branch 'affinity-expose-name' into paginated-manually

cf6fad1

Fix query

0becc08

docs

8d729c5

Finish paginating instance.rs

6ac39e5

smklein mentioned this pull request Mar 5, 2025

(9/5) [nexus] Allow anti-affinity members to be affinity groups #7572

Closed

smklein added 5 commits March 7, 2025 13:58

Merge branch 'main' into vmm-reserve-bench

4ed71c3

Merge branch 'vmm-reserve-bench' into vmm-reduce-contention

d8f173d

Merge branch 'vmm-reduce-contention' into aa-group-of-group

22c3eb0

Merge branch 'aa-group-of-group' into affinity-expose-name

e765503

Merge branch 'affinity-expose-name' into paginated-manually

797d340

smklein mentioned this pull request Mar 7, 2025

More manual pagination #7763

Open

smklein added 7 commits March 10, 2025 09:07

Merge branch 'main' into vmm-reduce-contention

37fa3fb

Merge branch 'main' into vmm-reduce-contention

dec670d

Merge branch 'aa-group-of-group' into affinity-expose-name

c9f38d4

Undo group-of-groups

6e369d9

shrinking the diff

af5c3fa

Update nexus.json

08801b1

Merge branch 'affinity-expose-name' into paginated-manually

5b4e822

smklein mentioned this pull request Mar 21, 2025

Refactor raw paginator for affinity specifically #7851

Merged

Base automatically changed from affinity-expose-name to main March 24, 2025 23:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[nexus] Experiment with using closer-to-raw-SQL pagination#7717

[nexus] Experiment with using closer-to-raw-SQL pagination#7717
smklein wants to merge 209 commits into
mainfrom
paginated-manually

smklein commented Mar 4, 2025 •

edited

Loading

Uh oh!

smklein Mar 4, 2025

Uh oh!

smklein Mar 4, 2025

Uh oh!

smklein Mar 4, 2025

Uh oh!

smklein Mar 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	pub fn paginated_multicolumn<T, C1, C2, M1, M2>(
	query: T,
	(c1, c2): (C1, C2),
	pagparams: &DataPageParams<'_, (M1, M2)>,
	) -> <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output
	where
	// T is a table^H^H^H^H^Hquery source which can create a BoxedQuery.
	T: QuerySource,
	T: AsQuery,
	<T as QuerySource>::DefaultSelection:
	Expression<SqlType = <T as AsQuery>::SqlType>,
	T::Query: query_methods::BoxedDsl<'static, Pg>,
	// Required for...everything.
	<T::Query as query_methods::BoxedDsl<'static, Pg>>::Output: QueryDsl,
	// C1 & C2 are columns which appear in T.
	C1: 'static + Column + Copy + ExpressionMethods,
	C2: 'static + Column + Copy + ExpressionMethods,
	// Required to compare the columns with the marker types.
	C1::SqlType: SqlType,
	C2::SqlType: SqlType,
	M1: Clone + AsExpression<C1::SqlType>,
	M2: Clone + AsExpression<C2::SqlType>,
	// Necessary for `query.limit(...)`
	<T::Query as query_methods::BoxedDsl<'static, Pg>>::Output:
	query_methods::LimitDsl<
	Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output,
	>,
	// Necessary for "query.order(c1.desc())"
	<T::Query as query_methods::BoxedDsl<'static, Pg>>::Output:
	query_methods::OrderDsl<
	Desc<C1>,
	Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output,
	>,
	// Necessary for "query.order(...).then_order_by(c2.desc())"
	<T::Query as query_methods::BoxedDsl<'static, Pg>>::Output:
	query_methods::ThenOrderDsl<
	Desc<C2>,
	Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output,
	>,
	// Necessary for "query.order(c1.asc())"
	<T::Query as query_methods::BoxedDsl<'static, Pg>>::Output:
	query_methods::OrderDsl<
	Asc<C1>,
	Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output,
	>,
	// Necessary for "query.order(...).then_order_by(c2.asc())"
	<T::Query as query_methods::BoxedDsl<'static, Pg>>::Output:
	query_methods::ThenOrderDsl<
	Asc<C2>,
	Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output,
	>,

	// We'd like to be able to call:
	//
	// c1.eq(v1).and(c2.gt(v2))
	//
	// This means "c1.eq(v1)" must implement BoolExpressionMethods, and
	// satisfy the requirements of the ".and" method.
	//
	// The LHS (c1.eq(v1)) must be a boolean expression:
	Eq<C1, M1>: Expression<SqlType = Bool>,
	// The RHS (c2.gt(v2)) must be a boolean expression:
	Gt<C2, M2>: Expression<SqlType = Bool>,
	// Putting it together, we should be able to filter by LHS.and(RHS):
	<T::Query as query_methods::BoxedDsl<'static, Pg>>::Output:
	query_methods::FilterDsl<
	And<Eq<C1, M1>, Gt<C2, M2>>,
	Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output,
	>,

	// We'd also like to be able to call:
	//
	// c1.eq(v1).and(c2.lt(v2))
	//
	// We've already defined the bound on the LHS, so we add the equivalent
	// bounds on the RHS for the "Less than" variant.
	Lt<C2, M2>: Expression<SqlType = Bool>,
	<T::Query as query_methods::BoxedDsl<'static, Pg>>::Output:
	query_methods::FilterDsl<
	And<Eq<C1, M1>, Lt<C2, M2>>,
	Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output,
	>,

	// Necessary for "query.or_filter(c1.gt(v1))"
	<T::Query as query_methods::BoxedDsl<'static, Pg>>::Output:
	query_methods::OrFilterDsl<
	Gt<C1, M1>,
	Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output,
	>,
	// Necessary for "query.or_filter(c1.lt(v1))"
	<T::Query as query_methods::BoxedDsl<'static, Pg>>::Output:
	query_methods::OrFilterDsl<
	Lt<C1, M1>,
	Output = <T::Query as query_methods::BoxedDsl<'static, Pg>>::Output,
	>,
	{

Conversation

smklein commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smklein Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

smklein Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

smklein Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

smklein Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

smklein commented Mar 4, 2025 •

edited

Loading