clean up Query APIs

[gavinking]

In parallel with thinking about this idea, I've also been thinking about options for cleaning up the Query API as it exists today. I'm concerned about a number of things.

Accumulated cruft on the Query interface

Query now has ~42 different methods for binding parameters. The main offender seems to be date/time related methods. A couple, like

setParameter(int position, Calendar value, TemporalType temporalType)

were maybe once useful back in the day, but aren't relevant anymore.

Some of the others don't look to me like they could have ever been useful, for example, about 8 methods like:

setParameter(int position, LocalDateTime value, TemporalType temporalType);

where I can't imagine why you would ever need to pass the TemporalType explicitly for a java.time type.

That is to say, we seem to have 8 methods on Query to support the important usecause of people abusing DateTimes to represent dates. That doesn't seem right at all.

Furthermore, we have both list() and getResultList(), and uniqueResult() and getSingleResult(), along with the scroll() methods and ResultTransformer stuff that probably aren't useful.

Let's consider deprecating some of these operations, with an eye to eventually removing them.

Too many ways to get a named query

QueryProducer defines competing createNamedQuery() and getNamedQuery() operations, along with getNamedNativeQuery() methods which don't especially look useful to me. This stuff is just fallout from the introduction of JPA, which happened 15 years ago.

If, as I suppose, some of these methods are redundant, let's deprecate them.

Issues with raw types

My biggest problem, however, is that it's too easy to get a raw Query. The versions of createQuery() which take just a string all return raw types, which results in at least one compiler warning on the user side. Sometimes you get even more warnings when you call methods with generic parameter types or wildcards. This is much more of a problem with NativeQuery than with Query, especially with NativeQuery.addEntity().

This problem is not fixable (I tried really hard) due to the way Session inherits EntityManager. I would love to be able to declare createQuery() as:

<T> Query<T> createQuery(String query)

but that's just not possible.

Now, in principle this isn't so bad, I suppose, since you can call the version of createQuery() that accepts a class. It's your own fault that you used the untyped version! What are you even complaining about? Well, yeah, except that:

• it's not documented anywhere, but createQuery() doesn't let me pass Object[].class as an argument (this might just be a bug),
• createQuery(CriteriaDelete) and createQuery(CriteriaUpdate) always return a raw type, and
• there's no version of createNativeQuery() that accepts a resultsetmapping and a class.

So in at least those cases I'm SOL.

My proposed actions here are:

• remove wildcards from parameter types of methods of Query and NativeQuery (I've already done this on a branch),
• make createQuery(CriteriaDelete) and createQuery(CriteriaUpdate) return Query<Void> or Query<Object>,
• allow Object[].class as an argument (and check jakarta.persistence.Tuple, I don't recall if that works or not),
• add a new method createNativeQuery(String,String,Class), and
• very seriously consider deprecating the createQuery() methods that return a raw type.

[beikov]

Query now has ~42 different methods for binding parameters. The main offender seems to be date/time related methods. A couple, like

Many of these come from JPA contracts which we just covariantly override.

Some of the others don't look to me like they could have ever been useful, for example, about 8 methods like:

I agree that passing a TemporalType, when passing a java.time type, is not very useful, so +1 for removing that

Furthermore, we have both list() and getResultList(), and uniqueResult() and getSingleResult(), along with the scroll() methods and ResultTransformer stuff that probably aren't useful.

+1

QueryProducer defines competing createNamedQuery() and getNamedQuery() operations, along with getNamedNativeQuery() methods which don't especially look useful to me. This stuff is just fallout from the introduction of JPA, which happened 15 years ago.

+1

remove wildcards from parameter types of methods of Query and NativeQuery (I've already done this on a branch),

Not sure which "wildcards" you mean here. Could you elaborate?

make createQuery(CriteriaDelete) and createQuery(CriteriaUpdate) return Query<Void> or Query<Object>

This problem stems from the fact that org.hibernate.Query extends from jakarta.persistence.TypedQuery and we return org.hibernate.Query from createQuery(CriteriaDelete) and createQuery(CriteriaUpdate), whereas EntityManager returns jakarta.persistence.Query which has no type parameters. IMO we should maybe introduce our own Query/TypedQuery types to mirror the JPA modelling here, which would avoid the raw types.

allow Object[].class as an argument (and check jakarta.persistence.Tuple, I don't recall if that works or not),

This should work already. If not, that would be a bug IMO.

add a new method createNativeQuery(String,String,Class), and

I guess that would return a TypedQuery? That's fine IMO.

very seriously consider deprecating the createQuery() methods that return a raw type.

Like stated above, the raw type issue comes from the fact that we don't have a pendant for jakarta.persistence.Query. If we introduce that, the raw types go away.

[gavinking]

Many of these come from JPA contracts which we just covariantly override.

Yes, I know, that's what I meant when I said:

"This problem is not fixable (I tried really hard) due to the way Session inherits EntityManager."

Not sure which "wildcards" you mean here.

For example, addEntity(Class<?> entityClass). There's no good reason at all for the Java compiler to complain when I call this method on a raw NativeQuery, but it does. Sucks.

IMO we should maybe introduce our own Query/TypedQuery types to mirror the JPA modelling here, which would avoid the raw types.

Unfortunately, I can't think of a way to do this without breaking a lot of user code.

This should work already. If not, that would be a bug IMO.

Yeah, probably. Anyway, that one is easy to fix.

[beikov]

Unfortunately, I can't think of a way to do this without breaking a lot of user code.

The breakage would be on the binary level which we already do with 6.0. We can do this in a source compatible way though. We just need to use a different name for the untyped query e.g. UntypedQuery :D

[gavinking]

Well I tried that and frankly it did not work very well. (Perhaps there's a way to make it work, by overriding all the methods or something.)

But worse, I just couldn't think of a non-awful name for that type (UntypedQuery was exactly what I called it when I tried).

[beikov]

by overriding all the methods or something

Yep, that is exactly what is needed 😆

non-awful name

At least then we can get rid of compiler warnings :)

[gavinking]

See work-in-progress here: #4516

[gavinking]

These problems are partially alleviated by #4516.

[gavinking]

This cleanup was done in H6.