spring-projects / spring-data-jpa

Simplifies the development of creating a JPA-based data access layer.
https://spring.io/projects/spring-data-jpa/
Apache License 2.0
2.98k stars 1.41k forks source link

Query Generation Regression when using EntityGraphs on Version 3.2.3 and Hibernate 6.4.4.Final #3403

Closed SchillingSebastian closed 6 months ago

SchillingSebastian commented 6 months ago

Hi there 👋 We did notice that the upgrade from Spring Data Jpa 3.2.2 to 3.2.3 comes with an upgrade of underlying Hibernate dependencies. This change somehow results in a failing test case in our code. When fixating the Hibernate dependency to 6.4.2.Final the test passes again, even when using 3.2.3.

In our application code we are using a JpaRepository with a Jpa Query Method and an @EntityGraph. The general code can be simplified to:

@Entity
class Author(
    @Id val id: UUID = UUID.randomUUID(),
    @OneToMany(mappedBy = "author")
    val books: MutableList<Book> = mutableListOf()
)

@Entity
class Book(
    @Id val id: UUID = UUID.randomUUID(),
    @ManyToOne(fetch = FetchType.LAZY)
    @JoinColumn(name = "author_id")
    var author: Author,
)
// Repository
interface AuthorRepository: JpaRepository<Author, UUID> {

    @EntityGraph(
        attributePaths = ["books"]
    )
    fun findAllByBooksIn(books: List<Book>): List<Author>
}
interface BookRepository:JpaRepository<Book, UUID>

The testcase would then be

@SpringBootTest
class AuthorRepositoryIntegrationTest(
    @Autowired private val authorRepository: AuthorRepository,
    @Autowired private val bookRepository: BookRepository,
) {
    lateinit var author1Book1: Book
    lateinit var author2Book1: Book

    @BeforeEach
    fun setup(){
        val author1 = authorRepository.save(Author())
        val author2 = authorRepository.save(Author())
        // create two books for each author
        author1Book1 = bookRepository.save(Book(author = author1,))
        bookRepository.save(Book(author = author1))
        author2Book1 = bookRepository.save(Book(author = author2))
        bookRepository.save(Book(author = author2))
    }

    @Test
    fun `WHEN using the entityGraph THEN every book is present`(){
        val found = authorRepository.findAllByBooksIn(listOf(author1Book1, author2Book1))
        for(author in found){
            assert(author.books.size == 2) // fails on 3.2.3 and Hibernate 6.4.4.Final ❌
        }
    }
}

The query issued to the DB is also different when using 6.4.4.Final

// logging.level.org.hibernate.SQL=DEBUG
// using 6.4.4.Final
select a1_0.id,b1_0.author_id,b1_0.id from author a1_0 left join book b1_0 on a1_0.id=b1_0.author_id where b1_0.id in (?,?)
// using 6.4.2.Final
select a1_0.id,b2_0.author_id,b2_0.id from author a1_0 left join book b1_0 on a1_0.id=b1_0.author_id left join book b2_0 on a1_0.id=b2_0.author_id where b1_0.id in (?,?)

As you can see, the required second left join on the book table is missing. So I am guessing that there has to be an issue with the Query generation. As the fixation of Hibernate to an older release "fixes" this issue I was trying to replicate this issue within Hibernate only, but I wasn't able to. I wasn't able to replicate the query generation with just the plain EntityManager. Hence I am opening the Issue in this repository. I did not find a note in a changelog mentioning this as a breaking change. Additionally I also looked into the diff from 3.2.2 to 3.2.3 and did not spot a change which would affect the query generation. I did find the hint in the Spring Boot release notes (https://github.com/spring-projects/spring-boot/releases/tag/v3.2.3), but there only a potential issue with native images is mentioned. In general having an EntityGraph on the relation which is already in the join due to the repository method seems no longer to work.

Thanks a lot in advance!

SchillingSebastian commented 6 months ago

Here is the testcase I tried to create with the Hibernate JPAUnitTestCase which fails for both 6.4.1.Final, 6.4.2.Final and 6.4.4.Final

// same setup, two authors with two books
fun test(){
        val entityManager = entityManagerFactory.createEntityManager()
        val graph = entityManager.createEntityGraph(Author::class.java)
        graph.addAttributeNodes("books")
        entityManager.transaction.begin()
        val cb = entityManager.criteriaBuilder
        val query = cb.createQuery(Author::class.java)
        val root:Root<Author> = query.from(Author::class.java)
        val emQuery =  entityManager
            .createQuery(
                query.select(root)
                    .where(
                        root.get<MutableList<Book>>("books").get<UUID>("id").`in`(listOf(author1Book1Id, author2Book1Id))
                    )
            )
            .setHint("jakarta.persistence.fetchgraph",graph)
        val authors = emQuery.resultList
        entityManager.transaction.commit()
        entityManager.close()
        for(author in authors){
            assert(author.books.size == 2) // fails even on 6.4.2.Final
        }
}
christophstrobl commented 6 months ago

Thank you @SchillingSebastian for reporting and taking the time to look into this! Since this seems to be a hibernate issue would you mind reporting it in the hibernate bug tracker and add comment with the link to it here?

SchillingSebastian commented 6 months ago

I opened up the issue there 📝 For tracking purposes here is the link: https://hibernate.atlassian.net/browse/HHH-17869

schauder commented 6 months ago

Thanks for creating the Hibernate issue.

odrotbohm commented 6 months ago

I guess the sample would need to be reduced quite a bit to be picked up by the Hibernate team. The ticket currently contains a lot of stuff that they probably would want to be removed as it's just "in the way". I think something like the sole HIbernate-based test case you show here, but in Java instead of Kotlin would get them to look at the problem much earlier.

Also, I think your example is not reflecting the way we create the query correctly. Judging from the log output you show, we seem to use an explicit join, as 6.4.2 seems to reflect that in the SQL logged. I.e., your example would need to be tweaked to produce exactly that log output on 6.4.2, probably by defining an explicit join. That should then fail on 6.4.4. I suspect something got in the way of translating that JPA Criteria API based join set up to eventually end up the actual SQL tree in 6.4.4

SchillingSebastian commented 6 months ago

@odrotbohm Thanks for the reply! Yes I tried getting down to the core of the creation of the query which is then issued to hibernate. But, as far as I remember (it has been some days) I could not spot a difference in query generation. I am not sure where to go with that sentence:

Also, I think your example is not reflecting the way we create the query correctly

There is no other modification done by me in any way. The project has basically just the spring-boot-starter-data-jpa and spring-boot-starter-test. The only thing "out of ordinary" I do, is fixating the hibernate-core version.

The response in the Hibernate Issue was/is:

The way you're structuring your original query is flawed, independently if it's generated from HQL, the Criteria API or any third party framework

@schauder I am not sure if this now points back to Spring JPA issuing an incorrect query using the criteria API. This is a little bit over my head right now. In general I do not consider the method in the JpaRepository out of ordinary: Give me all authors which wrote one of the books in the list and directly load all of the other books of found authors

SchillingSebastian commented 5 months ago

I managed to reproduce it with a plain Hibernate query (I'll skip the test setup, it is basically the same)

@Test
    fun `WHEN using entityGraph THEN all books are returned`(){
        setup() // 2 authors with 2 books each
        val entityManager = entityManagerFactory.createEntityManager()
        val graph = entityManager.createEntityGraph(Author::class.java)
        graph.addAttributeNodes("books")
        entityManager.transaction.begin()

        // The query generated by Spring Data JPA
        val em = entityManager.createQuery("select a from Author a left join a.books books where books in :books")
            .setHint("jakarta.persistence.fetchgraph",graph)
            .setParameter("books", listOf(author1Book1Id, author2Book1Id))

        val authors = em.resultList
        entityManager.transaction.commit()
        entityManager.close()
        assert(authors.size == 2)
        for(author in authors as List<Author>){
            assert(author.books.size == 2) // fails on 6.4.4
        }
    }

On the Hibernate ticket it was mentioned that the query generated by Spring Data Jpa is probably not correct but I also think it is not a bug. Doing IN queries on nested *ToMany relations will result in a flawed query string due to the generated left join in the query. This a developer probably just has to know. A correct query for our case would be: select a from Author a left join fetch a.books where a.id in (select a2.id from Author a2 where element(a2.books).id in :books) (which is I think too complex for Spring Data JPA to generate). It is weird that it was working until now by change, but thats just how things sometimes are.

@schauder So I think closing it was the right call

Thanks guys!