borinquenkid commented on code in PR #16520:
URL: https://github.com/apache/grails-core/pull/16520#discussion_r4190731608
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/HibernateQuery.java:
##########
@@ -526,7 +532,8 @@ public Number countResults() {
Number result;
if (projections.getProjectionList().isEmpty()) {
projections().count();
- result = (Number) executeSingleResult();
+ var creator = createJpaCriteriaQueryCreator();
+ result = (Number)
getCountQueryExecutor().singleResult(getCurrentSession(),
creator.createQuery(), creator.getParameterValues());
Review Comment:
Keeping `var`: this file already uses it throughout and the module compiles
at Java 21, so there is no source-level concern.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/HibernateQuery.java:
##########
@@ -526,7 +532,8 @@ public Number countResults() {
Number result;
if (projections.getProjectionList().isEmpty()) {
projections().count();
- result = (Number) executeSingleResult();
+ var creator = createJpaCriteriaQueryCreator();
+ result = (Number)
getCountQueryExecutor().singleResult(getCurrentSession(),
creator.createQuery(), creator.getParameterValues());
Review Comment:
Extracted into a private `executeCount` helper shared by both branches in
9f199f217a.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/HibernateQuery.java:
##########
@@ -539,7 +546,7 @@ public Number countResults() {
countQuery.from(innerSubquery);
countQuery.select(cb.count(cb.literal(1)));
- result = (Number)
getHibernateQueryExecutor().singleResult(getCurrentSession(), countQuery,
creator.getParameterValues());
+ result = (Number)
getCountQueryExecutor().singleResult(getCurrentSession(), countQuery,
creator.getParameterValues());
Review Comment:
Extracted into a private `executeCount` helper shared by both branches in
9f199f217a.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/HibernateQuery.java:
##########
@@ -539,7 +546,7 @@ public Number countResults() {
countQuery.from(innerSubquery);
countQuery.select(cb.count(cb.literal(1)));
- result = (Number)
getHibernateQueryExecutor().singleResult(getCurrentSession(), countQuery,
creator.getParameterValues());
+ result = (Number)
getCountQueryExecutor().singleResult(getCurrentSession(), countQuery,
creator.getParameterValues());
Review Comment:
Added "grouped count ignores max and offset and returns the number of
groups" to `DetachedCriteriaCountSpec` in 29ed6d5973, covering offset 1 and an
offset past the number of groups. It returns 5 and fails if the projection
branch pages again.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/HibernateQuery.java:
##########
@@ -488,6 +488,12 @@ private HibernateQueryExecutor getHibernateQueryExecutor()
{
offset, max, lockResult, queryCache, fetchSize, timeout,
flushMode, readOnly, proxyHandler);
}
+ /** An executor that never pages, because max and offset do not apply to a
count. */
+ private HibernateQueryExecutor getCountQueryExecutor() {
Review Comment:
Fixed in 25981e6b50. `CriteriaMethodInvoker` now calls `countResults()`, so
your three `CountItem` examples return 58, 58 and 10.
`CriteriaMethodInvokerSpec` is updated and `HibernateCriteriaBuilderSpec` has
`firstResult` cases. Note: `count { projections { groupProperty ... } }` now
counts the groups instead of adding a count projection on top of the closure's
projections.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
