You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "borinquenkid (via GitHub)" <gi...@apache.org> on 2026/10/05 14:43:39 UTC
[PR] Hibernate 7: count() ignores max and offset [grails-core]
borinquenkid opened a new pull request, #16520:
URL: https://github.com/apache/grails-core/pull/16520
## Problem
`Club.where {}.max(10).offset(10).count()` returns `null` on Hibernate 7. `HibernateQuery.countResults()` ran the count through the paging executor, so `max`/`offset` applied to the single-row count result. Hibernate 5 ignores both for `count()`.
## Fix
A count-specific executor (no offset, no max) used by both branches of `countResults()`.
## Tests
Restored the two `offset(10)` assertions in `DetachedCriteriaJoinSpec` ("check if count works as expected"); they failed before the fix. `detachedcriteria.*`, `query.*` and `*Count*`: 297 passed.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hibernate 7: count() ignores max and offset [grails-core]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #16520:
URL: https://github.com/apache/grails-core/pull/16520#discussion_r4185448146
##########
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:
The same count-execution pattern is duplicated across both branches. Consider extracting a small private helper (e.g., `executeCountSingleResult(query, params)`) to centralize how count queries are run, making future changes (executor options, parameter handling) less error-prone.
##########
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:
The same count-execution pattern is duplicated across both branches. Consider extracting a small private helper (e.g., `executeCountSingleResult(query, params)`) to centralize how count queries are run, making future changes (executor options, parameter handling) less error-prone.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hibernate 7: count() ignores max and offset [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16520:
URL: https://github.com/apache/grails-core/pull/16520#discussion_r4185621287
##########
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:
This change also fixes the projection branch, but the new assertions in `DetachedCriteriaJoinSpec` only exercise the plain-count branch above. On 8.0.x a grouped count with any offset returned `null` here:
```groovy
new DetachedCriteria(CountItem).build { projections { groupProperty 'itemGroup' } }.max(2).offset(1).count()
// 8.0.x: null, this PR: 5 (the number of groups)
```
Could you add a feature to `DetachedCriteriaCountSpec` asserting that a grouped count with `max`/`offset` still returns the number of groups, including an offset past the number of groups?
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hibernate 7: count() ignores max and offset [grails-core]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #16520:
URL: https://github.com/apache/grails-core/pull/16520#discussion_r4185447989
##########
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:
Using `var` requires a sufficiently new Java source level and is atypical in mixed Groovy/Java modules where compilation targets may vary. To avoid source/target compatibility issues and keep style consistent, consider using an explicit type here (e.g., `JpaCriteriaQueryCreator<?> creator = ...`).
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hibernate 7: count() ignores max and offset [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16520:
URL: https://github.com/apache/grails-core/pull/16520#discussion_r4185964289
##########
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:
Commenting here because the call site isn't in the diff: the criteria builder's `count {}` still pages. `CriteriaMethodInvoker` runs `count` as `hibernateQuery.projections().count()` followed by `hibernateQuery.singleResult()`, which uses the paging executor, so a `firstResult` inside the closure skips the single count row:
```groovy
CountItem.createCriteria().count { firstResult(10) } // null, expected 58
CountItem.createCriteria().count { maxResults(2); firstResult(10) } // null, expected 58
CountItem.createCriteria().count { eq('itemGroup', 1); firstResult(5) } // null, expected 10
```
(This uses the 58-row `CountItem` data from `DetachedCriteriaCountSpec`.) Calling `hibernateQuery.countResults()` there instead returns 58, 58 and 10 locally. The criteria and count specs still pass, except `CriteriaMethodInvokerSpec` "test invokeMethod handles count call", which mocks `projections()`/`singleResult()` and would need updating. Could you include that in this PR, with a `HibernateCriteriaBuilderSpec` case for `count` with `firstResult`?
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hibernate 7: count() ignores max and offset [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16520:
URL: https://github.com/apache/grails-core/pull/16520#issuecomment-5999913032
## ✅ All tests passed ✅
🏷️ Commit: fc9ae73ddc96944860798d829e9f7ac4355919af
▶️ Tests: 94728 executed
⚪️ Checks: 86/86 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hibernate 7: count() ignores max and offset [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16520:
URL: https://github.com/apache/grails-core/pull/16520#issuecomment-6008845854
## ✅ All tests passed ✅
🏷️ Commit: 9f199f217af438b1cd64ec4f00f01d8b91e917eb
▶️ Tests: 94785 executed
⚪️ Checks: 92/92 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hibernate 7: count() ignores max and offset [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid merged PR #16520:
URL: https://github.com/apache/grails-core/pull/16520
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hibernate 7: count() ignores max and offset [grails-core]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #16520:
URL: https://github.com/apache/grails-core/pull/16520#issuecomment-5999140117
## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/16520?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 59.5497%. Comparing base ([`df80e81`](https://app.codecov.io/gh/apache/grails-core/commit/df80e8128b03c029c6e9b40158d9a9b95c6a8110?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`fc9ae73`](https://app.codecov.io/gh/apache/grails-core/commit/fc9ae73ddc96944860798d829e9f7ac4355919af?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 4 commits behind head on 8.0.x.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/grails-core/pull/16520?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 8.0.x #16520 +/- ##
==================================================
+ Coverage 59.5377% 59.5497% +0.0120%
- Complexity 25019 25024 +5
==================================================
Files 2175 2175
Lines 108637 108637
Branches 19823 19823
==================================================
+ Hits 64680 64693 +13
+ Misses 35002 34992 -10
+ Partials 8955 8952 -3
```
[see 6 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/16520/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hibernate 7: count() ignores max and offset [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org