Fix count query for Page response and criteria

[radovanradic]

Changes fix for an earlier issue and adds fix for recently reported issue. Basically, when returning Page model will execute DISTINCT COUNT <identity> because when there are joins, results return more rows and we cannot count that since will return more results (while returning results using mapper returns correct number of records).

[dstepanov]

How is pagination supposed to work here? The limit would need to be distinct, too, somehow.

[radovanradic]

How is pagination supposed to work here? The limit would need to be distinct, too, somehow.

This is needed for totalSize in Page response only. Paging will work like before, or I am missing something.

[sonarqubecloud[bot]]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
73.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[dstepanov]

I don't think pagination like we have it now will work with JOINs, because our implementation just adds LIMIT to the end of the query. You would expect it to limit the root entity rows, but that will limit the entity records + other rows, so you can cut the relationship in the middle and fetch Author1(Book1, Book2) and then Author1(Book3), Author2(..). The correct way would be first to limit the entity table and join the other one.

[radovanradic]

I don't think pagination like we have it now will work with JOINs, because our implementation just adds LIMIT to the end of the query. You would expect it to limit the root entity rows, but that will limit the entity records + other rows, so you can cut the relationship in the middle and fetch Author1(Book1, Book2) and then Author1(Book3), Author2(..). The correct way would be first to limit the entity table and join the other one.

Yes, but this PR does not change that behavior, only count query that returns total size. It changes previous fix (this is better now I think) and fixes count for criteria query. It just counts main table records (distinct ids) from the join query. Before this it would count all main table + joined results and produce invalid total size.

We could try to handle pagination in separate ticket.

[dstepanov]

I wonder how Hibernate is doing; it looks like a pretty complex problem.

[radovanradic]

I wonder how Hibernate is doing; it looks like a pretty complex problem.

Yes, I thought I should spend some time and see how spring is doing it in their paging methods.

[radovanradic]

I wonder how Hibernate is doing; it looks like a pretty complex problem.

I looked at how spring and hibernate are doing this. Looks like in relation author->books they run count against author only but also when retrieve data, first fetch results for author with given offset/limit and then load joined collection of books for each author record. This is quite different from what we are doing (as we generate JOIN in the query and execute it). So, to be accurate we might need to change query execution with paging in jdbc. Looks like significant change and amount of work to me.

[kareemelzayat]

@radovanradic @dstepanov I have noticed this has been here for a while. Is there anything I can do to help?

[radovanradic]

I think this fixes count issue, but there are other issues with how we retrieve paged items incorrectly when there are joins. That would be major change in the code, but don't know how we want to handle that. It would require rewriting SQL builder, logic in fetching (like Hibernate loads joins separately with another call to the db) etc.

[kareemelzayat]

@radovanradic could you be more specific? What exactly are we experiencing? What are the use cases that cause such issues to happen? From what I understood, some issue arise when users write a query like

val predicateSpecification = { root.fetch(childEntities) }
findAll(PredicateSpecification, Pageable) --> wrong results are returned

Is that so? Could it be related to the issue we have here: https://github.com/micronaut-projects/micronaut-data/issues/2780?

[radovanradic]

@kareemelzayat It is what Denis commented above https://github.com/micronaut-projects/micronaut-data/pull/2740#issuecomment-1898445888 We use LIMIT on the result that returns parent + child records and then we don't return correct number. Hibernate does limit on parent records so it returns correct number of records and then fetches children for each parent.

[kareemelzayat]

@radovanradic so the problem is only with JDBC? Can't it be solved with a countDistinct?

The downside will probably be a slower query

[radovanradic]

Yes, only JDBC. It won't help, still will count author(s) + books as this is what is retrieved, need to return authors that satisfy given condition and then join books.

[dstepanov]

The issue is only with Micronaut Data JDBC / R2DBC, paginating of joined tables is a bit problematic

[kareemelzayat]

I mean, JDBC/R2DBC is for users who want to have full control of their SQL queries. They know what they are doing and what to expect.

This means, they know they will need to use distinct if they want correct results in case of *-to-Many joins. This means they know what a paginated result of a join query will look like.

So they will need to adapt their queries (via distinct) for example.

Or am I missing something?

[CLAassistant]

All committers have signed the CLA.

[ibmsoft]

there is any plan date to resolve this problem?

[radovanradic]

I think this could be merged, but Denis noted that actual paging code will not return correct data with current implementation and this is another big and kind of complex issue. Just checked spring paging and they basically have similar issue with joined entities spring-hibernate-paging.zip However, count could be fixed with this and then paging handled (if there is a good solution) in separate ticket.

[radovanradic]

Closing this for now, solving the whole issue might take much more time and perhaps different approach.