Projected criteria query: Useless and problematic inner join on optional referenced entity

Description

Entity org.hibernate.jpa.test.metamodel.Customer refers org.hibernate.jpa.test.metamodel.Address via a OneToOne attribute named home.

The following test case fails:

The generated failing sql query is:

The test fails because a useless (IMO) inner join is generated while customer references no home address.
I tried to fix it myself to provide a patch but I am lost between antlr and the hql AST.
If you don't have the time to fix this, please give me some hints to fix it myself

Environment

None

Activity

Show:
Vlad Mihalcea
October 17, 2018, 7:09 AM

The result should be 1 since the implicit join given by parentRoot.get( "child" ) is rendered as an INNER JOIN.

If you want a LEFT JOIN, you need to do it explicitly.

So, if we modify the example like this:

Everything will work as expected since the generated SQL will use LEFT JOIN:

Jens Schauder
October 17, 2018, 8:38 AM

> The result should be 1 since the implicit join given by parentRoot.get( "child" ) is rendered as an INNER JOIN.

That is exactly where we disagree over the interpretation of the specification.

I created an issue with the specification https://github.com/eclipse-ee4j/jpa-api/issues/189 to get the semantics clarified.

Thanks for the discussion.

Vlad Mihalcea
October 17, 2018, 9:14 AM

Well, Hibernate used to work like that even before JPA 1.0.

Now, changing from INNER to LEFT OUTER will complicate most implicit join queries since the underlying SQL must make sure that no JOINED column is referenced in the WHERE clause other than joined_table.column IS NULL OR joined_table.column = ?, otherwise, if you do a LEFT JOIN and the WHERE clause does something like joined_table.column = ?, the end result is equivalent to an INNER JOIN.

But that's not all. Selecting an entity as a DTO projection is also something that Hibernate treats in a very specific manner, as explained by Steve. In 6.0, we might offer a property so that, instead of the entity identifier, the entity itself could be passed in the DTO projection.

As for the implicit join, unless the JPA spec will ever demand that the provider must use LEFT JOIN, Hibernate will most likely use INNER JOIN, as it has been doing for the past 17 years.

Steve Ebersole
October 17, 2018, 12:58 PM

It looks like that JPA specification on this subject is largely subject to interpretation, I don't see from where you get such certainty about what it doesn't tell? Maybe I am not in the secrets of the ORM gods like you ?

We need to stop with this point of view of spec interpretation. That is not the way a spec works. If a spec does not say something specifically, then it is open to interpretation. The spec says absolutely nothing about this, as you astutely point out. So we as a team are perfectly fine making our interpretation. You may not like it, but that != "non spec compliant" or any such phrasing. What we do clearly does not violate the spec.

Is it intuitive? Meh, that's open to debate. I can certainly see that point of view, so much so that I planned on adding that ability in 6. But as Vlad points out this has been the behavior in Hibernate well before JPA 1.0 even. It's funny how people hate changes in behavior unless its the one thing they want changed So you have a user base out there relying on the behavior as it is now and always has been. Why on earth do you think we would change that based on this (inaccurate) spec argument?

The Tuple option is very much a viable work around and the entity in the Tuple is in fact the entity instance.

That is exactly where we disagree over the interpretation of the specification.

Exactly - interpretation of the spec. Not "something the spec explicitly says".

Steve Ebersole
October 17, 2018, 1:08 PM

I'll give you an inverse argument...

In 6.0 (this is all already implemented btw) you can do all kinds of things with dynamic instantiation (extra features) that the spec actually explicitly says you should not be able to do[1]. We offer these as value-add. Since these value-adds are technically illegal according to the spec, should we just drop them? (rhetorical question - I'm not dropping those new features)

[1] Few examples:

  1. In JPA, if you use a dynamic-instantiation it can be the only return. IOW select new DTO(...) from... is fine, but select a.prop, new DTO(...) from ... is not

  2. In JPA, dynamic-instantiations cannot be nested. IOW, according to JPA this is illegal but we suppport in 6: select new DTO( newDTO2(...), ...) from ...

  3. In JPA the DTO must have a matching constructor. In 6 we also support dynamic-instantiation through no-arg ctor + setters.

Again, all of this is not required by the spec. In fact, one could easily argue that the spec explicitly says they are not legal. So should we drop things like this as well?

Assignee

Steve Ebersole

Reporter

Réda Housni Alaoui

Fix versions

None

Labels

None

backPortable

Backport?

Suitable for new contributors

None

Requires Release Note

None

Pull Request

None

backportDecision

None

Components

Affects versions

Priority

Major
Configure