Ensure proper hint is applied for projections of paginated queries for Querydsl fluent queries #2827
Conversation
When using Querydsl predicates to build a fluent query, be sure to apply the projection hint that generates a fetch graph. Resolves #2820.
gregturn
changed the title
There was a problem hiding this comment.
The PR looks good to me.
Regarding the test challenge, I see the following options:
- use a Datasource that captures the executed SQL statements.
- inspect the logs for the SQL statements.
- It seems H2 supports triggers on select statements. So one could presumably create a trigger that throws an exception, when certain columns get accessed. Of course, we currently use HSQLDB for testing and that doesn't seem to support such triggers.
Not sure if we consider it worth the effort ...
There was a problem hiding this comment.
I left a few comments. Probably, if we had unit tests with mocking we could test things way better. Introducing SQL-capturing tests introduces a lot of maintenance afterwards because SQL can change with each Hibernate release. Probably, we're good with relying on what we do now.
| import static org.springframework.data.domain.ExampleMatcher.matching; | ||
| import static org.springframework.data.domain.Sort.Direction.ASC; | ||
| import static org.springframework.data.domain.Sort.Direction.DESC; | ||
| import static java.util.Arrays.*; |
There was a problem hiding this comment.
The import reformatting indicates a different threshold for * imports. Please revisit your formatter settings.
There was a problem hiding this comment.
I believe our standard is 1 for static imports, and I didn't have this properly configured in the past.
There was a problem hiding this comment.
Please ignore my comment. I missed the static.
| List<R> paginatedResults = convert(pagedQuery.fetch()); | ||
| AbstractJPAQuery<?, ?> query = pagedFinder.apply(sort, pageable); | ||
|
|
||
| if (!properties.isEmpty()) { |
There was a problem hiding this comment.
Do we need to do the same for Query by Example and Specifications?
There was a problem hiding this comment.
To be more precise, FetchableFluentQueryByExample 's readPage method taps the createSortedAndProjectedQuery method, which applies the expected hint in all scenarios.
The same can be said for the Specification-based variant.
This one is the only one that doesn't do that, probably due to the nature of Querydsl's APIs.
| import java.util.stream.Stream; | ||
|
|
||
| import org.springframework.data.domain.*; |
There was a problem hiding this comment.
These changes seem fine as we have less than 10 imports.
|
That's merged and backported now. |
Resolves #2820
I have added the application of hints in case of a projection to the paginated query. I cannot seem to find a logging option to SHOW me it's working, but OP has verified it works as expected on their end. Any tips to expose this happening in our test suite would be appreciate. But apart from that, I believe test by analysis is of use in this situation.
And please alert me to any other issues with this PR.