Upgrade to Hibernate ORM 6.5 / Hibernate Reactive 2.3 #40102

yrodiere · 2024-04-16T09:38:18Z

Draft because we only have CRs of these libraries for now, no Final version.

This should fix the following upstream issues:

We'll need to have a look at #37575 once we have some time; it's possible the new, official property for disabling JDBC metadata retrieval works a bit better (but I'm not sure). Related: #30002 , #13522.

quarkus-bot · 2024-04-16T09:38:23Z

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

title should preferably start with an uppercase character (if it makes sense!)

_{This message is automatically generated by a bot.}

quarkus-bot · 2024-04-16T09:38:38Z

/cc @gsmet (hibernate-orm)

...rm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/StatelessSessionLazyDelegator.java

yrodiere · 2024-04-16T13:57:45Z

Marking as ready for review to get a full CI run. Please don't merge just yet.

Sanne · 2024-04-22T10:28:19Z

...rc/main/java/io/quarkus/hibernate/orm/runtime/session/TransactionScopedStatelessSession.java

@@ -394,6 +396,30 @@ public Object get(String entityName, Object id, LockMode lockMode) {
        }
    }

+    @Override
+    public Filter enableFilter(String filterName) {


No rush, but it might be worth adding a base class in ORM for these use cases.

I always considered the "Session API" would be rock stable and not worth the hassle, but in practice it seems there's some adjustment needed quite often..

See https://hibernate.atlassian.net/browse/HHH-17990. Please have a look at the comments too, as it's not as straightforward as one might think, given what Quarkus does in such "proxy" classes.

BTW if only we had support for generating proxies at extension build time, we could solve this much more easily on the Quarkus side... Provided we have some metadata regarding which method implies data mutation or DB access.

yrodiere · 2024-04-22T11:44:28Z

CI doesn't get executed and I don't understand why. I've removed [WIP] from the title and the on-ice label, let's see how it goes now...

EDIT: Please don't merge yet though!

yrodiere · 2024-04-23T08:23:16Z

FYI I'm currently investing the failures.

The JVM failures are about a bug in ORM where H2 native enum order is handled incorrectly (@beikov is on it) and a bug in Reactive's implementation of getRefrence (@DavideD is on it).

Now I'll look at the native failures...

yrodiere · 2024-04-23T12:28:32Z

Related upstream fixes so far:

hibernate/hibernate-orm@1166e64
hibernate/hibernate-reactive#1892
hibernate/hibernate-orm#8226 (needs backport)

yrodiere · 2024-04-24T14:44:28Z

Another image metrics change:

2024-04-24T10:08:57.6758269Z [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 10.31 s <<< FAILURE! -- in io.quarkus.it.jpa.postgresql.ImageMetricsITCase
2024-04-24T10:08:57.6779385Z [ERROR] io.quarkus.it.jpa.postgresql.ImageMetricsITCase.verifyImageMetrics -- Time elapsed: 0.330 s <<< FAILURE!
2024-04-24T10:08:57.6782495Z org.opentest4j.AssertionFailedError: Expected analysis_results.types.reflection to be within range [6522 +- 3%] but was 6732 ==> expected: <true> but was: <false>

I just checked on main: the number before even upgrading to Hibernate ORM 6.5 is 6704 with Mandrel 23.1.2.0 and 6705 with Mandrel 23.1.1.0.

So Hibernate ORM 6.5 only accounts for 28 new types registered for reflection. Not great, but certainly not a reason to fail the build.

For now I'll just bump the numbers to whatever native image generation reports before the upgrade.

zakkak · 2024-04-25T07:27:13Z

@yrodiere please note that previous Hibernate ORM updates have also increased the elements registered for reflection (and this the binary size), see:

jpa-postgresql native image size increase (on Mandrel-23.1.2.0-Final) #39674 (comment)
jpa-postgresql native image size increase (on Mandrel-23.1.2.0-Final) #39674 (comment)

It looks like there is a trend with newer versions bringing in more and more things.

yrodiere · 2024-04-25T07:31:01Z

It looks like there is a trend with newer versions bringing in more and more things.

Yes, new versions come with new features.

zakkak · 2024-04-25T07:40:02Z

Yes, new versions come with new features.

Sure, but in theory we don't use all these new features (e.g. the integration tests didn't change) so ideally Quarkus should be able to contain their impact on the resulting native executables.

yrodiere · 2024-04-25T07:50:49Z

Yes, ideally. I'll add this to the stack of items to work on... no promises :/

…elines

yrodiere · 2024-04-26T08:14:31Z

.Final releases are out, and the PR is now using them.

Let's see whether CI is happy...

quarkus-bot · 2024-04-26T12:58:10Z

Status for workflow `Quarkus CI`

This is the status report for running Quarkus CI on commit c44e9e1.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/hibernate-orm Hibernate ORM area/persistence OBSOLETE, DO NOT USE labels Apr 16, 2024

marko-bekhta reviewed Apr 16, 2024

View reviewed changes

...rm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/StatelessSessionLazyDelegator.java Outdated Show resolved Hide resolved

yrodiere force-pushed the orm6.5 branch from d9833ed to 1a1f64e Compare April 16, 2024 09:51

yrodiere marked this pull request as ready for review April 16, 2024 13:57

yrodiere added the triage/on-ice Frozen until external concerns are resolved label Apr 16, 2024