Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression for method-based query where predicate compares primary key of @ManyToOne relationship #3349

Open
scordio opened this issue Feb 2, 2024 · 12 comments · May be fixed by #3374
Open
Assignees
Labels
status: waiting-for-triage An issue we've not yet triaged

Comments

@scordio
Copy link
Contributor

scordio commented Feb 2, 2024

Given the following entities:

@Entity
class Author {

  @Id @GeneratedValue(strategy = GenerationType.AUTO)
  private Long id;

  private String name;

  // getters and setters omitted
}

@Entity
class Book {

  @Id @GeneratedValue(strategy = GenerationType.AUTO)
  private Long id;

  private String name;

  @ManyToOne(fetch = FetchType.LAZY)
  private Author author;

  // getters and setters omitted
}

and the following repository:

interface BookRepository extends Repository<Book, Long> {

  Book save(Book book);

  List<Book> findAllByAuthorId(Long authorId);

}

findAllByAuthorId seems to trigger an unnecessary JOIN.

I initially noticed this behavior on Spring Boot 2.7.14 with DB2 for z/OS so I tried to reproduce it locally on newer Spring Boot versions.

Surprisingly, 3.0.x and 3.1.x seem to work properly, i.e., no unnecessary JOIN, while the JOIN is back again on 3.2.x and 3.3.x. That's why I mentioned it as a regression in the title.

Here are the derived queries for each Spring Boot version with an H2 database, captured with P6Spy:

2.7.18

select book0_.id as id1_1_, book0_.author_id as author_i3_1_, book0_.name as name2_1_ from book book0_ left outer join author author1_ on book0_.author_id=author1_.id where author1_.id=1;

3.0.13

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 where b1_0.author_id=1;

3.1.8

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 where b1_0.author_id=1;

3.2.2

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 left join author a1_0 on a1_0.id=b1_0.author_id where a1_0.id=1;

3.3.0-SNAPSHOT

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 left join author a1_0 on a1_0.id=b1_0.author_id where a1_0.id=1;

Reproducers:

@scordio scordio changed the title Regression for method-based query where predicate compares primary key of @ManyToOne relationship Regression for method-based query where predicate compares primary key of @ManyToOne relationship Feb 2, 2024
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 2, 2024
@christophstrobl
Copy link
Member

thanks for reporting and the reproducer - have you tried to do the same query with plain JPA/hibernate as well?

@scordio
Copy link
Contributor Author

scordio commented Feb 3, 2024

I suppose you're referring to JPQL, right @christophstrobl?

I've just added the following to the reproducers:

@Query("SELECT b FROM Book b WHERE b.author.id = :authorId")
List<Book> findAllByAuthorIdWithJPQL(@Param("authorId") Long authorId);

It looks good on all versions:

2.7.18

select book0_.id as id1_1_, book0_.author_id as author_i3_1_, book0_.name as name2_1_ from book book0_ where book0_.author_id=1;

3.0.13

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 where b1_0.author_id=1;

3.1.8

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 where b1_0.author_id=1;

3.2.2

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 where b1_0.author_id=1;

3.3.0-SNAPSHOT

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 where b1_0.author_id=1;

@scordio
Copy link
Contributor Author

scordio commented Feb 3, 2024

For completeness, I also tried with JpaSpecificationExecutor and a Specification:

private static Specification<Book> byAuthorId(Long authorId) {
  return (root, query, builder) -> builder.equal(root.get("author").get("id"), authorId);
}

It also looks good on all versions:

2.7.18

select book0_.id as id1_1_, book0_.author_id as author_i3_1_, book0_.name as name2_1_ from book book0_ where book0_.author_id=1;

3.0.13

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 where b1_0.author_id=1;

3.1.8

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 where b1_0.author_id=1;

3.2.2

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 where b1_0.author_id=1;

3.3.0-SNAPSHOT

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 where b1_0.author_id=1;

@quaff
Copy link
Contributor

quaff commented Feb 4, 2024

have you tried to do the same query with plain JPA/hibernate as well?

	CriteriaBuilder cb = entityManager.getCriteriaBuilder();
	CriteriaQuery<Book> cq = cb.createQuery(Book.class);
	Root<Book> root = cq.from(Book.class);
	cq.select(root).where(cb.equal(root.get("author").get("id"), homer.getId()));
	entityManager.createQuery(cq).getResultList();

will produce expected sql

select b1_0.id,b1_0.author_id,b1_0.name from book b1_0 where b1_0.author_id=?

@quaff
Copy link
Contributor

quaff commented Feb 4, 2024

@scordio It works as expected if you downgrade hibernate to 6.2.20.Final which is used by 3.1.8, I confirm it is a regression introduced by hibernate 6.4.1.Final, I reported it to hibernate team, see https://hibernate.atlassian.net/browse/HHH-17706.

@scordio
Copy link
Contributor Author

scordio commented Feb 4, 2024

Thanks @quaff for helping pinpoint the root cause!

Closing in favor of HHH-17706 and hibernate/hibernate-orm#7782.

@scordio scordio closed this as not planned Won't fix, can't repro, duplicate, stale Feb 4, 2024
@scordio
Copy link
Contributor Author

scordio commented Feb 5, 2024

I am reopening this issue as hibernate/hibernate-orm#7782 has been rejected.

@mbladel mentioned in HHH-17706:

I would suggest using implicit join paths (i.e. root.simpleEntity.id or root.get( "simpleEntity" ).get( "id" ) with Criteria) to take advantage of foreign-key optimization if really needed.

Is it something that could be done in Spring Data JPA?

@scordio scordio reopened this Feb 5, 2024
@quaff
Copy link
Contributor

quaff commented Feb 6, 2024

Here is workaround:

interface BookRepository extends JpaRepository<Book, Long> {

    default List<Book> findAllByAuthorId(Long authorId) {
        Author author = new Author();
        author.setId(authorId);
        return findAllByAuthor(author);
    }

    List<Book> findAllByAuthor(Author author);

}

@christophstrobl
Copy link
Member

Thank you both @scordio & @quaff - we'll see if there's anything we can do on our side.

@christophstrobl christophstrobl self-assigned this Feb 6, 2024
@scordio
Copy link
Contributor Author

scordio commented Feb 6, 2024

Thank you, @christophstrobl! Unless someone is working on it soon, I'm also happy to take a look and come up with a proposal.

@christophstrobl
Copy link
Member

thank you @scordio - PRs are always welcome :)

quaff added a commit to quaff/spring-data-jpa that referenced this issue Feb 19, 2024
@quaff quaff linked a pull request Feb 19, 2024 that will close this issue
@quaff
Copy link
Contributor

quaff commented Feb 19, 2024

@scordio Could you try PR #3374 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants