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

Fetchgraph does not prevent Hibernate from fetching lazy one-to-one relationships #42425

Closed
KevinSeroux opened this issue Aug 8, 2024 · 7 comments · Fixed by #43348
Closed
Labels
area/hibernate-orm Hibernate ORM kind/bug Something isn't working triage/upstream Used for issues which are caused by issues in upstream projects/dependency
Milestone

Comments

@KevinSeroux
Copy link

KevinSeroux commented Aug 8, 2024

Describe the bug

In a monolith using WebLogic (as application server) and Eclipselink (as ORM), there was a very slow SQL query (that could takes more than 10 seconds) that can returns thousand of rows. We have been able to optimize it so it takes less than 1 second when using a fetch graph. Prior the fix, a mapper after the method javax.persistence.TypedQuery#getResultList was submitting dozen of SQL queries (fetching lazy relationships and lazy of lazy...) for each row when using the getters of the main entity class. The fetch graph allowed to eagerly fetch lazy relationships in a single query.

Now, the entire feature is planned to be moved into a Quarkus 2 project while still keeping the DDL and the database for iterative migrations. Unfortunately, the fetch graph is not working as expected.

Yet the Hibernate documentation states (source 6, see all links below):

fetch graph : In this case, all attributes specified in the entity graph will be treated as FetchType.EAGER, and all attributes not specified will ALWAYS be treated as FetchType.LAZY.

There was a bug which ignored the hint but it seems solved since the 5.4.22: https://hibernate.atlassian.net/browse/HHH-8776

What I tried :

  • Using @LazyToOne annotations (source 1, 5 or 8), but this was required for Hibernate < 5.5.
  • Using attribute optional = false for @OneToOne (source 3 or 4)
  • Setting hibernate.enhancer.enableLazyInitialization to true in application.properties (source 7)

I can't update the DDL, so it prevents using the annotation @MapsId (source 3 or 4).

According to source 2:

Starting with Hibernate 5.5, the FetchType.EAGER fetching strategy can be overridden at query time via the fetchgraph property.

According to source 8:

However, if you really need to use a bidirectional association and want to make sure that this is always going to be fetched lazily, then you need to enable lazy state initialization bytecode enhancement and use the @LazyToOne annotation as well.

I don't know why it does not work, can it be related to some internals Quarkus bytecode enhancement incompatible with Hibernate?

Sources :

  1. https://vladmihalcea.com/hibernate-lazytoone-annotation/
  2. https://vladmihalcea.com/fetchtype-eager-fetchgraph/
  3. https://vladmihalcea.com/the-best-way-to-map-a-onetoone-relationship-with-jpa-and-hibernate/
  4. https://thorben-janssen.com/hibernate-tip-lazy-loading-one-to-one/
  5. http://justonjava.blogspot.com/2010/09/lazy-one-to-one-and-one-to-many.html
  6. https://docs.jboss.org/hibernate/orm/5.6/userguide/html_single/Hibernate_User_Guide.html#fetching-strategies-dynamic-fetching-entity-graph
  7. https://docs.jboss.org/hibernate/orm/5.6/userguide/html_single/Hibernate_User_Guide.html#configurations-bytecode-enhancement
  8. https://docs.jboss.org/hibernate/orm/5.6/userguide/html_single/Hibernate_User_Guide.html#associations-one-to-one-bidirectional-lazy

Expected behavior

I expect only one query in the logs:

Hibernate:
    select
        fruit0_.id as id1_0_0_,
        fruit0_.name as name2_0_0_
    from
        known_fruits fruit0_
    where
        fruit0_.id=?

Actual behavior

There is an additional request to fetch the lazy @OneToOne relationship:

Hibernate:
    select
        fruit0_.id as id1_0_0_,
        fruit0_.name as name2_0_0_
    from
        known_fruits fruit0_
    where
        fruit0_.id=?
Hibernate:
    select
        fruitdetai0_.id as id1_1_0_,
        fruitdetai0_.description as descript2_1_0_,
        fruitdetai0_.fruit_id as fruit_id3_1_0_
    from
        known_fruits_detail fruitdetai0_
    where
        fruitdetai0_.fruit_id=?

How to Reproduce?

Here is a minimal project to reproduce the issue (branch 2.16): https:/KevinSeroux/quarkus-quickstarts/tree/2.16

If you prefer an archive: hibernate-orm-quickstart-fetchgraph.tar.gz

If you only want the diffs from hibernate-orm-quickstart (might be easier to locate the issue): quarkusio/quarkus-quickstarts@2.16...KevinSeroux:quarkus-quickstarts:2.16

I reproduce the issue also with Quarkus 3.13.1.

Output of uname -a or ver

Linux ITEM-S131685 5.15.153.1-microsoft-standard-WSL2 #1 SMP Fri Mar 29 23:14:13 UTC 2024 x86_64 GNU/Linux

Output of java -version

openjdk version "17.0.11" 2024-04-16 OpenJDK Runtime Environment (build 17.0.11+9-Debian-1deb12u1) OpenJDK 64-Bit Server VM (build 17.0.11+9-Debian-1deb12u1, mixed mode, sharing)

Quarkus version or git rev

2.16

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.6 (84538c9988a25aec085021c365c560670ad80f63)

Additional information

No response

@KevinSeroux KevinSeroux added the kind/bug Something isn't working label Aug 8, 2024
@quarkus-bot quarkus-bot bot added area/hibernate-orm Hibernate ORM area/persistence OBSOLETE, DO NOT USE labels Aug 8, 2024
@quarkus-bot
Copy link

quarkus-bot bot commented Aug 8, 2024

/cc @gsmet (hibernate-orm), @yrodiere (hibernate-orm)

@gsmet
Copy link
Member

gsmet commented Aug 8, 2024

Could you attach a zip with the reproducer to your comment (or point to a personal GitHub repo)? Will be easier than a diff.

Also I would recommend to try with the latest 3.13.1 to see how it goes there.

@KevinSeroux
Copy link
Author

KevinSeroux commented Aug 8, 2024

I've both updated the issue with a link to the Git repository and attached an archive.

I forgot to mention it, but yes I had reproduced the issue with Quarkus 3.13.1.

@yrodiere
Copy link
Member

yrodiere commented Aug 13, 2024

Thanks, but your reproducer serializes the fruit details in the REST endpoint's response, that's why it gets initialized.

That being said, the problem you mention does exist, and I managed to change your reproducer to demonstrate that:

In all cases I get something like this, showing the association does get loaded unexpectedly:

[Hibernate] 
    select
        f1_0.id,
        f1_0.name 
    from
        known_fruits f1_0
[Hibernate] 
    select
        fd1_0.id,
        fd1_0.description,
        fd1_0.fruit_id 
    from
        known_fruits_detail fd1_0 
    where
        fd1_0.fruit_id=?
[Hibernate] 
    select
        fd1_0.id,
        fd1_0.description,
        fd1_0.fruit_id 
    from
        known_fruits_detail fd1_0 
    where
        fd1_0.fruit_id=?
[Hibernate] 
    select
        fd1_0.id,
        fd1_0.description,
        fd1_0.fruit_id 
    from
        known_fruits_detail fd1_0 
    where
        fd1_0.fruit_id=?

I didn't try to find out what's going on on 2.16, because that's Hibernate ORM 5, and that version is known to not handle fetch graphs correctly.

However, that was supposed to be fixed in Hibernate ORM 6 (used in Quarkus 3.13), so that's weird. I tried to debug the reproducer a bit, and here is what I found:

  1. The interpretation of the fetch graph correctly detects that the #fruitDetails association should be fetched lazily ("delayed" initialization) here: https:/hibernate/hibernate-orm/blob/4da2659292f140d290b51b2b178039cf09c1c4c2/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java#L8337
  2. BUT... Hibernate ORM then goes through a shouldExplicitFetch check here: https:/hibernate/hibernate-orm/blob/4da2659292f140d290b51b2b178039cf09c1c4c2/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java#L8348
  3. ... and that check actually forces initialization here: https:/hibernate/hibernate-orm/blob/4da2659292f140d290b51b2b178039cf09c1c4c2/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java#L8571-L8575

That last step looks like a bug, because we already decided that the association shouldn't be initialized (which is correct, according to fetch graph semantics). However, it's been implemented as part of a bugfix for HHH-15391, so this may actually be an undocumented limitation... I'm not sure. @dreab8, who worked on HHH-15391, may be able to help? Andrea, I think we should open a bug report upstream, no?

@dreab8
Copy link
Contributor

dreab8 commented Aug 13, 2024

@yrodiere it seems a bug to me, when bytecode enhancement is enabled the one-to-one association should be fecthed lazily,
Please open a bug report, thanks

@yrodiere
Copy link
Member

Thanks @dreab8 , I reported this as https://hibernate.atlassian.net/browse/HHH-18489 , with a reproducer.

@yrodiere yrodiere added triage/upstream Used for issues which are caused by issues in upstream projects/dependency and removed area/persistence OBSOLETE, DO NOT USE labels Aug 14, 2024
@KevinSeroux
Copy link
Author

Thank you, @yrodiere, for improving the reproducer and opening the issue on the Hibernate bug tracker. In the meantime, we developed a fully functional workaround using a tuple query with javax.persistence.criteria.CriteriaBuilder#createTupleQuery. However, this came at the cost of many lines of code and required us to reimplement a mechanism to convert OneToMany relationships from a flat table representation back into a structure of nested Java objects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hibernate-orm Hibernate ORM kind/bug Something isn't working triage/upstream Used for issues which are caused by issues in upstream projects/dependency
Projects
None yet
4 participants