Affects: All versions
DeferredQueryInvocationHandler should return method.invoke(this.target, args); in an else branch for the unwrap method, otherwise the returned object might not be of the requested type. Consider e.g.
QuerySqmImpl<?> hqlQuery = query.unwrap(QuerySqmImpl.class);
This will fail with a class cast exception when using a proxy with DeferredQueryInvocationHandler.
Comment From: jhoeller
As far as I can infer, this proceeds down to the common method.invoke(target, args) call in line 320 already, it just happens to post-process the returned Query instance for a non-transactional EntityManager - in order to implicitly close that EntityManager for a query-terminating method. And that's the part that does not work when unwrap is being called with a concrete class rather than an interface type: since we create an interface-based proxy there, the result is not castable to the implementation type.
Now, the question is what to do on our side in such a case: We could reject the unwrap attempt for a concrete class, providing a clearer exception message... or we could return a raw unwrapped Query instance without proxying, at the expense of not explicitly closing the EntityManager when that Query object is then used with a query-terminating method.
I'm wondering what the goal of that unwrap(QuerySqmImpl.class) attempt actually is. Does this really have to be unwrapped to the concrete class, or could it also work with the SqmQueryImplementor interface? And what does the caller do with the returned Query instance then, in particular outside of a transaction?
Comment From: beikov
The goal is to potentially call some Hibernate ORM specific or internal methods. Why does the implementation return the target object if I pass null to unwrap?
Whatever you do, return the target or throw an explicit exception, that will help users to understand what they have to do, which is to e.g. create an explicit transaction. From the ClassCastException that is thrown right now, it's not clear at all what the reason for this behavior is.
Comment From: jhoeller
Fair enough, the existing code path for unwrap(null) does indeed return the target instance unmodified. So we accept returning a non-decorated raw Query instance in that code path already.
Also, scratch the above in terms of details, I mixed up the EntityManager decoration of Query instances with the Query-backed proxy behavior there. The code path actually ends up in method.invoke(this.target, args) in line 416 and then hard-switches to the proxy instance if the call returned the target Query instance through return (retVal == this.target ? proxy : retVal) which is where I suppose your scenario actually fails. We can revise this to check the expected type there indeed.
As for calling internal provider methods: Hibernate exposes its extended operations on interfaces, this does not usually require a cast to the actual implementation class. I'm just wondering whether Hibernate's SqmQueryImplementor interface could be used instead of the QuerySqmImpl type there. It's certainly recommended to use interfaces as far as possible, especially for provider-specific operations on JDBC connections (where the connection pool often decorates via interfaces as well) but also for JPA.
That said, we are going to fix this through raw target Query exposure in case of a proxy mismatch. To be released in 6.1.7 and backported to 6.0.20 and 5.3.35.
Comment From: beikov
Dankeschön 😊