HHH-17706 Optimize FK comparison to eliminate unnecessary left join

[quaff]

Fix regression introduced by ef155c2

https://hibernate.atlassian.net/browse/HHH-17706

[mbladel]

As I mentioned in more details on the Jira, we opted to avoid optimizing away explicit joins (see discussion on previous pr) to prevent problematic queries. I also fear the checks you're using won't be enough to ensure we can avoid using the target side key to create correct queries, except for very simple cases like the test you're introducing.

Closing this PR for now.

[quaff]

I’m also skeptical about the actual performance impact of creating the join since primary keys should be indexed and the query results are the same.

@scordio Could you take some time to verify that as @mbladel said?

[scordio]

@quaff sure, I'm getting in touch with our Db2 team for a more detailed analysis – will get back to you as soon as I have the details.

[gavinking]

But .... why wouldn't you guys just change the query to use get() instead of join()? That's the most natural way to write this query if you don't want the join!

[scordio]

@gavinking I'm totally with you if I would define the query manually but the case I originally reported is spring-projects/spring-data-jpa#3349 so Spring Data JPA generates the query.

That's why I was asking if an enhancement should be considered in there (spring-projects/spring-data-jpa#3349 (comment)) but obviously workarounds are possible in the client code, like spring-projects/spring-data-jpa#3349 (comment) where get() is used.

[gavinking]

Look, I'm just saying.....

package org.example;

import org.hibernate.StatelessSession;
import org.hibernate.annotations.processing.Find;

import java.util.List;

interface BookRepository {

  StatelessSession session();

  @Find
  List<Book> findAllByPublisher(long publisher$id);

}

Generated by annotation processor:

package org.example;

import jakarta.annotation.Generated;
import jakarta.annotation.Nonnull;
import jakarta.enterprise.context.Dependent;
import jakarta.inject.Inject;
import jakarta.persistence.metamodel.StaticMetamodel;
import java.util.List;
import org.hibernate.StatelessSession;

@Dependent
@StaticMetamodel(BookRepository.class)
@Generated("org.hibernate.jpamodelgen.JPAMetaModelEntityProcessor")
public class BookRepository_ implements BookRepository {

	
	private final @Nonnull StatelessSession entityManager;
	
	@Inject
	public BookRepository_(@Nonnull StatelessSession entityManager) {
		this.entityManager = entityManager;
	}
	
	public @Nonnull StatelessSession session() {
		return entityManager;
	}
	
	/**
	 * Find {@link Book} by {@link Book#publisher.id publisher.id}.
	 *
	 * @see org.example.BookRepository#findAllByPublisher(long)
	 **/
	@Override
	public List<Book> findAllByPublisher(long publisher$id) {
		var builder = entityManager.getFactory().getCriteriaBuilder();
		var query = builder.createQuery(Book.class);
		var entity = query.from(Book.class);
		query.where(
				builder.equal(entity.get(Book_.publisher).get(Publisher_.id), publisher$id)
		);
		return entityManager.createQuery(query).getResultList();
	}


}

Not only can freely rename the method findAllByPublisher() to whatever you like, but you can easily debug that generated code.

SQL:

    select
        b1_0.isbn,
        b1_0.duration,
        b1_0.price,
        b1_0.publication_date,
        b1_0.publisher_id,
        b1_0.quantitySold,
        b1_0.text,
        b1_0.title,
        b1_0.type 
    from
        books b1_0 
    where
        b1_0.publisher_id=?

But I can't force you to use this. It's something you have to realize for yourself.

[gavinking]

(You can of course use your own preferred flavor of "session" here. I personally like StatelessSession for DAO-style repositories.)

[scordio]

Absolutely, I understand that a Hibernate-only solution would work without issues, and I do appreciate the hint!

I'm just coming from a codebase strongly connected to Spring Data JPA and I'd like to understand how the Spring Data team sees the current issue before evaluating a tech change, given that there are already other ways in Spring Data to get the right query generated.

(I know, Hibernate is already there and features can be used, but maybe you get what I mean 🙂)

[gavinking]

Of course, I understand.