codeconsole commented on PR #16323:
URL: https://github.com/apache/grails-core/pull/16323#issuecomment-5593710845
You're right that it's a breaking change for statically compiled callers —
that's intentional, and `8.0.x` is the branch where it's allowed. But the
generic form is worth testing rather than reasoning about, so I ran all three
signatures against Groovy 5.1.2, each called from `@CompileStatic` code written
against the old API (`Integer n = api.count()`), with an implementation
returning `3_000_000_000L`:
| Signature | Result for `Integer n = api.count()` |
| --- | --- |
| `<T extends Number> T count()` | **compiles**, returns `-1294967296` |
| `Long count()` | compile error — `loss of precision from java.lang.Long to
java.lang.Integer` |
| `Number count()` | compile error — `Cannot assign value of type
java.lang.Number to variable of type java.lang.Integer` |
The generic signature does remove the compile error, but it doesn't add
coercion — it erases to `Object` and lets the call site insert the cast. `T` is
chosen by the caller, while the implementation can only ever return the one
type the datastore produced, so the truncation survives and now shows up as a
negative row count at runtime instead of a build failure. That is the bug this
PR exists to remove, made invisible. Erasure also means the implementation
can't inspect `T` and convert to it, so there's no version of that signature
that could coerce.
`Number` breaks the same callers, so it doesn't avoid the migration either —
it just hands callers a type they have to unwrap before using.
So there's no non-breaking option here: any type wide enough to hold what
the datastores actually return is unassignable to `Integer` under STC. Given
that, the compile error seems like the feature rather than the cost — it points
at precisely the call sites that were silently truncating, and the fix at each
is a one-word type change. §54 of the upgrade notes covers the migration,
including the Java-caller and GraphQL-client cases.
Worth noting where this actually bites: MongoDB aggregates counts with
`{$sum: 1}`, which returns Int32 and promotes to Int64 once the total exceeds
it. So on a large collection GORM was receiving the correct 64-bit count and
`intValue()` was wrapping it — no error, just a wrong number.
Happy to go a different way if you'd still prefer the smaller blast radius.
--
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]
