Initial addition of cursored pagination for SQL #2884

andriy-dmytruk · 2024-04-15T15:03:05Z

This is an initial PR to check if this approach could be considered.

Create the CursoredPageable type.
Modify the DefaultSqlPreparedQuery to support cursored pageable for SQL. Since cursored pagination requires a new query part with parameters, the type was modified with additional query bindings.
Modify DefaultFindPageInterceptor to return the correct pageable for further pagination in the cursored case.

Other interceptors will further need to be changed for this to work in all cases, like the FindPageSpecificationInterceptor. Additionally, the cursor could be encoded to base 64 instead of a JSON array. It seems like Pageabe.hasNext() method would also be beneficial. Should that be added to both offset and cursor pagination?

graemerocher · 2024-04-15T15:59:56Z

Could you look at the Jakarta Data spec https://github.com/jakartaee/data/blob/main/api/src/main/java/jakarta/data/page/CursoredPage.java

And make sure we mirror that implementation as closely as possible

dstepanov · 2024-04-15T16:02:43Z

Please target 4.8.x with adjusted @since

andriy-dmytruk · 2024-04-15T16:35:38Z

Do you know how I can solve this error?

io.micronaut.context.exceptions.BeanInstantiationException: Bean definition [io.micronaut.data.jdbc.config.SchemaGenerator] could not be loaded: Error instantiating bean of type  [io.micronaut.data.jdbc.config.SchemaGenerator]

Message: Unable to create database schema: FATAL: sorry, too many clients already
Path Taken: new SchemaGenerator(List configurations,JdbcSchemaHandler schemaHandler)

dstepanov · 2024-04-15T16:49:33Z

Looks like DB is out of connections. How do you get it?

andriy-dmytruk · 2024-04-15T16:59:03Z

Looks like DB is out of connections. How do you get it?

The AbstractCursoredPageSpec has a few parametrized tests that perhaps create many connections. But this error appears only when testing with PostgreSQL.

andriy-dmytruk · 2024-04-15T18:17:14Z

And make sure we mirror that implementation as closely as possible

I added similar Page.hasNext() and Page.hasPrevious() methods.

...src/main/java/io/micronaut/data/runtime/operations/internal/sql/DefaultSqlPreparedQuery.java

data-model/src/main/java/io/micronaut/data/model/Page.java

...jdbc/src/test/groovy/io/micronaut/data/jdbc/sqlserver/SqlServerCursoredPaginationSpec.groovy

dstepanov · 2024-04-16T07:30:23Z

Can you please make an API closer to Jakarta Data?
We should have similar API as https://github.com/jakartaee/data/blob/main/api/src/main/java/jakarta/data/page/PageRequest.java
New class Pageable.Cursor methods afterCursor, beforeCursor, Optional<Cursor> cursor();, Mode.

It would be nice to add requestTotal, withoutTotal, withoutTotal with a logic it has - retrieving a total from the DB or not.

data-model/src/main/java/io/micronaut/data/model/DefaultCursoredPageable.java

...src/main/java/io/micronaut/data/runtime/operations/internal/sql/DefaultSqlPreparedQuery.java

data-model/src/main/java/io/micronaut/data/model/Pageable.java

data-model/src/main/java/io/micronaut/data/model/CursoredPageable.java

data-model/src/main/java/io/micronaut/data/model/DefaultPageable.java

data-model/src/main/java/io/micronaut/data/model/Page.java

data-model/src/main/java/io/micronaut/data/model/Pageable.java

...src/main/java/io/micronaut/data/runtime/operations/internal/sql/DefaultSqlPreparedQuery.java

data-model/src/main/java/io/micronaut/data/model/DefaultCursoredPageable.java

dstepanov · 2024-04-17T15:37:08Z

Can you also add tests for R2dbc and Hibernate?
Not sure how it should behave for other implementations like Mongo, maybe it should throw an error

- Create the CursoredPageable type. - Modify the DefaultSqlPreparedQuery to support cursored pageable for SQL. - Modify DefaultFindPageInterceptor to return correct pageable for further pagination in cursored case.

…t supported

data-model/src/test/groovy/io/micronaut/data/model/PageSpec.groovy

…d previousPageable implementation from the CursoredPageable

data-model/src/main/java/io/micronaut/data/model/DefaultCursoredPageable.java

data-model/src/main/java/io/micronaut/data/model/DefaultCursoredPage.java

sdelamo

It misses documentation.

radovanradic · 2024-04-22T08:23:15Z

data-jdbc/src/test/groovy/io/micronaut/data/jdbc/mysql/MysqlCursoredPaginationSpec.groovy

+
+    @Override
+    void init() {
+        context = ApplicationContext.run(properties)


Less important, I think you could move context creation in the abstract class

@AutoCleanup @Shared ApplicationContext context = ApplicationContext.run(properties)

and then use it in all tests (usually how we do it) so init() method might not be needed.

...r2dbc/src/test/groovy/io/micronaut/data/r2dbc/postgres/PostgresCursoredPaginationSpec.groovy

dstepanov · 2024-04-23T13:06:25Z

@andriy-dmytruk Let's try to finish this PR, the only thing that is missing is the documentation and maybe the changes you mentioned regarding the DefaultCursoredPage

sonarqubecloud · 2024-04-24T18:57:23Z

Quality Gate failed

Failed conditions
3 New Bugs (required ≤ 0)
2 New Blocker Issues (required ≤ 0)
7 New Critical Issues (required ≤ 0)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

andriy-dmytruk · 2024-04-29T14:10:00Z

@dstepanov and @sdelamo could you take another look?

Guide for the Cursor-based pagination support added to Micronaut Data JDBC by this [PR #2884](micronaut-projects/micronaut-data#2884) We cannot merge this PR until the framework 4.5.0 release which will contain a Micronaut Data release with that feature. You can generate the guide’s HTML locally with : ``` ./gradlew build open build/dist/micronaut-data-cursored-page-gradle-java.html ``` To run the sample code locally, publish to maven local Micronaut Data PR and use: ``` annotationProcessor("io.micronaut.data:micronaut-data-processor:4.8.0-SNAPSHOT") implementation("io.micronaut.data:micronaut-data-jdbc:4.8.0-SNAPSHOT") ```

sdelamo

I wrote a guide which we can publish once we have framework release which contains a data release with this feature.

dstepanov · 2024-05-22T09:38:54Z

@sdelamo It would be nice to show how to propagate the cursor to the FE

andriy-dmytruk requested a review from dstepanov April 15, 2024 15:03

andriy-dmytruk changed the base branch from 4.8.x to 4.7.x April 15, 2024 15:27

andriy-dmytruk force-pushed the andriy/cursored-pagination branch from c1c1fe6 to e3896d2 Compare April 15, 2024 15:27

dstepanov requested review from graemerocher and radovanradic April 15, 2024 15:55

andriy-dmytruk changed the base branch from 4.7.x to 4.8.x April 15, 2024 16:32

andriy-dmytruk force-pushed the andriy/cursored-pagination branch from e3896d2 to 449d0d2 Compare April 15, 2024 16:34

dstepanov requested changes Apr 16, 2024

View reviewed changes

graemerocher requested changes Apr 16, 2024

View reviewed changes

data-model/src/main/java/io/micronaut/data/model/DefaultCursoredPageable.java Outdated Show resolved Hide resolved

...src/main/java/io/micronaut/data/runtime/operations/internal/sql/DefaultSqlPreparedQuery.java Outdated Show resolved Hide resolved

andriy-dmytruk commented Apr 16, 2024

View reviewed changes

data-model/src/main/java/io/micronaut/data/model/Pageable.java Outdated Show resolved Hide resolved

dstepanov requested changes Apr 17, 2024

View reviewed changes

andriy-dmytruk added 11 commits April 17, 2024 13:29

Initial addition of cursored pagination

68bc521

- Create the CursoredPageable type. - Modify the DefaultSqlPreparedQuery to support cursored pageable for SQL. - Modify DefaultFindPageInterceptor to return correct pageable for further pagination in cursored case.

Checkstyle

2a88b3e

Change base to 4.8.x

34f73e7

Add hasNext and hasPrevious methods to Page

b5294fc

Checkstyle fixes and disable PostgreSQL test

f5d2c5a

Implement some review comments

2e8e813

Add methods corresponding to the jakarta PageRequest API

9964e4c

Add requestTotal property to the pageable

6167f12

Update Page to account for cases when total size is not queried

17f81d3

Fix build

b9aa459

Implement more review comments

5146d90

andriy-dmytruk force-pushed the andriy/cursored-pagination branch from ff2acd6 to 5146d90 Compare April 17, 2024 18:15

andriy-dmytruk added 3 commits April 17, 2024 15:28

Add tests

6bcabba

Fix checkstyle

fbc8570

Throw UnsupportedOperationException where cursored pageable is not ye…

019dacd

…t supported

andriy-dmytruk force-pushed the andriy/cursored-pagination branch from a881e77 to 019dacd Compare April 18, 2024 01:09

dstepanov reviewed Apr 18, 2024

View reviewed changes

data-model/src/test/groovy/io/micronaut/data/model/PageSpec.groovy Show resolved Hide resolved

andriy-dmytruk added 2 commits April 18, 2024 19:14

Add all cursors to the page implementation and remove nextPageable an…

db7cc08

…d previousPageable implementation from the CursoredPageable

Slightly improve the test

6d261ea

andriy-dmytruk commented Apr 19, 2024

View reviewed changes

data-model/src/main/java/io/micronaut/data/model/DefaultCursoredPageable.java Show resolved Hide resolved

andriy-dmytruk commented Apr 19, 2024

View reviewed changes

data-model/src/main/java/io/micronaut/data/model/DefaultCursoredPage.java Outdated Show resolved Hide resolved

sdelamo added the type: enhancement New feature or request label Apr 19, 2024

sdelamo requested changes Apr 19, 2024

View reviewed changes

radovanradic reviewed Apr 22, 2024

View reviewed changes

...r2dbc/src/test/groovy/io/micronaut/data/r2dbc/postgres/PostgresCursoredPaginationSpec.groovy Outdated Show resolved Hide resolved

andriy-dmytruk added 4 commits April 23, 2024 17:00

Fix for postgres r2dbc test

b6278a9

Add CursoredPage and interceptor

8aa2078

Add documentation for cursored pageable

0ccf54a

Fix interceptors

0d38e7d

graemerocher approved these changes Apr 24, 2024

View reviewed changes

graemerocher requested review from sdelamo and dstepanov April 25, 2024 00:12

sdelamo mentioned this pull request May 9, 2024

Guide for the Cursor-based pagination micronaut-projects/micronaut-guides#1463

Draft

sdelamo approved these changes May 9, 2024

View reviewed changes

dstepanov merged commit 696cc25 into 4.8.x May 22, 2024
40 of 43 checks passed

dstepanov deleted the andriy/cursored-pagination branch May 22, 2024 09:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial addition of cursored pagination for SQL #2884

Initial addition of cursored pagination for SQL #2884

andriy-dmytruk commented Apr 15, 2024

graemerocher commented Apr 15, 2024

dstepanov commented Apr 15, 2024

andriy-dmytruk commented Apr 15, 2024

dstepanov commented Apr 15, 2024

andriy-dmytruk commented Apr 15, 2024

andriy-dmytruk commented Apr 15, 2024

dstepanov commented Apr 16, 2024

dstepanov commented Apr 17, 2024

sdelamo left a comment

radovanradic Apr 22, 2024

dstepanov commented Apr 23, 2024

sonarqubecloud bot commented Apr 24, 2024

andriy-dmytruk commented Apr 29, 2024

sdelamo left a comment

dstepanov commented May 22, 2024

Initial addition of cursored pagination for SQL #2884

Initial addition of cursored pagination for SQL #2884

Conversation

andriy-dmytruk commented Apr 15, 2024

graemerocher commented Apr 15, 2024

dstepanov commented Apr 15, 2024

andriy-dmytruk commented Apr 15, 2024

dstepanov commented Apr 15, 2024

andriy-dmytruk commented Apr 15, 2024

andriy-dmytruk commented Apr 15, 2024

dstepanov commented Apr 16, 2024

dstepanov commented Apr 17, 2024

sdelamo left a comment

Choose a reason for hiding this comment

radovanradic Apr 22, 2024

Choose a reason for hiding this comment

dstepanov commented Apr 23, 2024

sonarqubecloud bot commented Apr 24, 2024

Quality Gate failed

andriy-dmytruk commented Apr 29, 2024

sdelamo left a comment

Choose a reason for hiding this comment

dstepanov commented May 22, 2024