Differentiate between JPQL and native queries in count query derivation #2777
Conversation
We now consider whether a query is a native one when deriving a count query for pagination. Previously, the generated queries used JPQL syntax that doesn't comply with native SQL syntax rules.
De-duplicate code, use parametrized tests instead of test methods to verify individual fixtures. Ensure all variants are tested with JSQLparser and the default enhancer.
spring-projects-issues
added
the
status: waiting-for-triage
| } | ||
|
|
||
| @Test // GH-2578 | ||
| void deeplyNestedcomplexSetOperationListWorks() { |
There was a problem hiding this comment.
Perhaps make it deeplyNestedComplexSetOperationListWorks?
| } | ||
|
|
||
| @Test // GH-2578 | ||
| void valuesStatementsWorks() { |
There was a problem hiding this comment.
Perhaps valuesStatementWorks?
| } | ||
|
|
||
| @Test // GH-2578 | ||
| void withStatementsWorks() { |
There was a problem hiding this comment.
Perhaps withStatementWorks?
| } | ||
|
|
||
| @Test // GH-2578 | ||
| void multipleWithStatementsWorks() { |
There was a problem hiding this comment.
Perhaps multipleWithStatementsWork?
| * @return a query String to be used a count query for pagination. Guaranteed to be not {@literal null}. | ||
| * @since 2.7.8 | ||
| */ | ||
| static String createCountQueryFor(String originalQuery, @Nullable String countProjection, boolean nativeQuery) { |
There was a problem hiding this comment.
I'd prefer two separate methods createCountQueryForNative and createCountQueryForJpql plus extracting the common parts into a new method.
There was a problem hiding this comment.
My gut feeling is that we should rethink QueryUtils entirely and eventually eliminate the most methods (or create delegates). Therefore, I would want to refrain from introducing additional methods for the time being unless new methods heavily improve the code.
|
That's merged and back ported to 3.0.x |
| String alias = QueryUtils.detectAlias(originalQuery); | ||
| if ("*".equals(variable) && alias != null) { | ||
| replacement = alias; | ||
| if (nativeQuery && (variable.contains(",") || "*".equals(variable))) { |
There was a problem hiding this comment.
What if it is a native query and the variable is null 😬
There was a problem hiding this comment.
If you run into a bug or you can reproduce such a case, please file a bug report.
|
will it be back ported to 2.7.x? |
|
Yes, we will backport the fix to 2.7.x. |
We now consider whether a query is a native one when deriving a count query for pagination. Previously, the generated queries used JPQL syntax that doesn't comply with native SQL syntax rules.
Also, cleanup tests, deduplicate test code, and rearrange test cases into a maintainable form.
Closes #2773