Repository navigation
Hibernate 7: leave the criteria's orders out of a count query - #16731
Merged
Merged
Conversation
A count has one row whatever the order, and an ORDER BY on a column that is
not aggregated fails on H2 and most other databases, so `count()` on a
DetachedCriteria or a criteria builder with an `order` threw a
SQLGrammarException ("Column ... must be in the GROUP BY list"). Only the
paged-list total escaped it, because it clears the orders first.
JpaCriteriaQueryCreator now skips the orders when every projection of the
query is a count, as `count()` and `countDistinct()` set it up. A query that
groups and counts keeps its orders.
✅ All tests passed ✅Test SummaryTip Inspect all details with the TestLens CLI: CI / Build Grails-Core (Windows JDK 25 shard 2) > :grails-shell-cli:test
Groovy Snapshot Canary Build / Build Grails (shard 0) > :grails-test-examples-mongodb-hibernate5:test
Groovy Snapshot Canary Build / Build Grails (shard 1) > :grails-data-mongodb-core:test (first 40 of 225)
Groovy Snapshot Canary Build / Build Grails (shard 1) > :grails-test-examples-mongodb-test-data-service:integrationTest
Groovy Snapshot Canary Build / Build Grails (shard 2) > :grails-data-mongodb-spring-boot:test
Groovy Snapshot Canary Build / Build Grails (shard 2) > :grails-test-examples-mongodb-base:test
🏷️ Commit: ebc4b2a Test Failures (first 10 of 236)BookFongoSpec (:grails-test-examples-mongodb-hibernate5:test in Groovy Snapshot Canary Build / Build Grails (shard 0) | Attempt 1/2)AggregateMethodSpec (:grails-data-mongodb-core:test in Groovy Snapshot Canary Build / Build Grails (shard 1) | Attempt 1/2)AssignedIdentifierSpec (:grails-data-mongodb-core:test in Groovy Snapshot Canary Build / Build Grails (shard 1) | Attempt 1/2)AttachMethodSpec (:grails-data-mongodb-core:test in Groovy Snapshot Canary Build / Build Grails (shard 1) | Attempt 1/2)AutowireServicesSpec (:grails-data-mongodb-core:test in Groovy Snapshot Canary Build / Build Grails (shard 1) | Attempt 1/2)BasicArraySpec (:grails-data-mongodb-core:test in Groovy Snapshot Canary Build / Build Grails (shard 1) | Attempt 1/2)BasicCollectionTypeSpec (:grails-data-mongodb-core:test in Groovy Snapshot Canary Build / Build Grails (shard 1) | Attempt 1/2)BasicCollectionsSpec (:grails-data-mongodb-core:test in Groovy Snapshot Canary Build / Build Grails (shard 1) | Attempt 1/2)BatchUpdateDeleteSpec (:grails-data-mongodb-core:test in Groovy Snapshot Canary Build / Build Grails (shard 1) | Attempt 1/2)BeforeInsertUpdateSpec (:grails-data-mongodb-core:test in Groovy Snapshot Canary Build / Build Grails (shard 1) | Attempt 1/2)Learn more about TestLens at testlens.app/docs. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #16731 +/- ##
==================================================
+ Coverage 62.1821% 62.2245% +0.0423%
- Complexity 26900 26926 +26
==================================================
Files 2241 2241
Lines 110736 110741 +5
Branches 20218 20218
==================================================
+ Hits 68858 68908 +50
+ Misses 32615 32568 -47
- Partials 9263 9265 +2 🚀 New features to boost your workflow:
|
jamesfredley
approved these changes
Oct 22, 2026
jamesfredley
left a comment
jamesfredley
left a comment
Contributor
There was a problem hiding this comment.
Approving. A count-only projection no longer gets the criteria's ORDER BY. The stored orders are unchanged, so the list query still sorts. DISTINCT and GROUP BY stay on the count path that needs them.
jdaugherty
reviewed
Oct 23, 2026
jdaugherty
left a comment
jdaugherty
left a comment
Contributor
There was a problem hiding this comment.
@borinquenkid two follow-ups from running this against 8.0.x after the merge. Neither is blocking, and the fix itself checks out: asBoolean(), countDistinct and an order on an association path are covered as well, and the grouped count and the derived-table countResults() path are unchanged.
-
Hibernate 5 has the same bug. On 8.0.x,
grails-data-hibernate5emitsselect count(*) as y0_ from count_order_probe_entity this_ order by this_.amount descfor
DetachedCriteria.count(),createCriteria().count {}andasBoolean()with an order, and H2 2.x rejects it with the same 90016. There the orders go onto the HibernateCriteriaasorder()is called (AbstractHibernateQuery.order), so the H5 fix would drop them incountResults()and the builder's count branch rather than at build time. Would you take that as a sibling PR? -
#16696 can now drop the "return null on a count" advice from the docs and the
OrderHandlerjavadoc and add acount()case with an order handler, as noted on that PR.
| private boolean isCountOnly() { | ||
| List<Query.Projection> projectionList = projections.getProjectionList(); | ||
| return !projectionList.isEmpty() && projectionList.stream() | ||
| .allMatch(projection -> projection instanceof Query.CountProjection || |
Contributor
There was a problem hiding this comment.
The predicate covers count and countDistinct, so a query whose only projections are other aggregates still carries the orders and fails the same way:
c.get { projections { max('amount') }; order('amount') }
Column "CTE1_0.AMOUNT" must be in the GROUP BY list
select max(cte1_0.amount) from criteria_test_entity cte1_0 order by cte1_0.amount
That one is user-written rather than added by the framework, so it is less pressing than count(), but the rule could be "no group by and every projection is an aggregate" (count, countDistinct, max, min, sum, avg) at no extra cost. A follow-up PR is fine.
borinquenkid
added a commit
that referenced
this pull request
Oct 23, 2026
10.0.x changed three of the Java files this branch converts: #16574 gave BidirectionalOneToManyLinker a CompositeIdentifierToManyToOneBinder, aligned the to-one side first and marks the copied key sorted, and #16731 leaves the criteria's orders out of a count-only query in JpaCriteriaQueryCreator. CollectionBinder merged on its own; the other two are ported into their Groovy files by hand, in the style of the conversion, and JpaCriteriaQueryCreator.java stays deleted. Claude-Session: https://claude.ai/code/session_01MDHn7fC3hsGJ27vFHD2o1p
13 of 16 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
count()on aDetachedCriteriaor a criteria builder that also has anorderthrew on H2:A count has one row whatever the order, and an ORDER BY on a column that is not aggregated fails on H2 and most other databases. Only the paged-list total escaped it, because
HibernatePagedResultListclears the orders before it counts.JpaCriteriaQueryCreatornow leaves the criteria's orders out when every projection of the query is a count, which is howcount()andcountDistinct()set the query up. A query that groups and counts keeps its orders, soprojections { groupProperty(...); count(...) }with anorderis unchanged.Found while reviewing #16696 (jdaugherty's comment on
assignOrderBy): that PR adds order handlers, whose orders run into the same count path, and its docs currently tell every handler to returnnullfor a count. With this fix the framework drops the orders instead, so that advice can go.Verification
HibernateCriteriaBuilderDirectSpec:DetachedCriteria.count()with an order, with a criterion and an order,rowCount()with an order through the builder,createCriteria().count { order(...) }, and the order still applying tolist(). It failed with the error above before the fix.:grails-data-hibernate7-core:test: 3,794 tests, 0 failures, 59 skipped.checkstyleMainandcodenarcTestfor the module: clean.