Skip to content

Conversation

mbellade
Copy link
Member

@mbellade mbellade commented Aug 7, 2025

This is only a rebase of @lucamolteni's work in #48130 now that Hibernate has been upgraded to 7.1 via #49344.

To be added in the Migration Guide:

Hibernate ORM

MySQL/MariaDB storage engine

Setting the MySQL/MariaDB storage engine through property quarkus.hibernate-orm.dialect.storage-engine has been deprecated. Use one of these configuration keys instead:

quarkus.hibernate-orm.dialect.mariadb.storage-engine=...
quarkus.hibernate-orm.dialect.dialect.mysql.storage-engine=...

…Hibernate Reactive quarkusio#48130

Added a `quarkus.hibernate-orm.database.start-offline` to avoid connecting to the database

* Disable schema validation
* Disable temporary table creation at startup, gives precedence to local temporary tables using Hibernate 7.1 local mutation strategies
* Disable schema management in offline mode for DevServices as well

New way to handle storage engine from both mysql and mariadb

Added tests for specific override of Dialect Settings

Removed unused hack MultiplePersistenceUnitsInconsistentStorageEnginesTest$H2DialectWithMySQLInTheName

Register for reflection the local temp strategies otherwise Native compilation is broken (to be removed after https://hibernate.atlassian.net/browse/HHH-15525)

Included initial draft by Steve Ebersole <[email protected]> quarkusio#43396

Co-authored-by: Steve Ebersole <[email protected]>
Co-authored-by: Yoann Rodière <[email protected]>
@quarkus-bot quarkus-bot bot added area/docstyle issues related for manual docstyle review area/documentation area/hibernate-orm Hibernate ORM area/hibernate-reactive Hibernate Reactive labels Aug 7, 2025
Copy link

quarkus-bot bot commented Aug 7, 2025

/cc @gsmet (hibernate-orm)

Copy link

quarkus-bot bot commented Aug 7, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 3e0bad9.

✅ 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.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

github-actions bot commented Aug 7, 2025

🙈 The PR is closed and the preview is expired.

.application.properties
----
quarkus.hibernate-orm."offline".dialect.mariadb.bytes-per-character=1
quarkus.hibernate-orm."offline".dialect.mariadb.no-backslash-escapes=true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it really needs these quotes? I understand the technical aspect but as a user, this feels off.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the "offline" is just to denote a PU-specific setting, the quotes are not strictly necessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohh that's much better, thanks 👍

Copy link
Member

@Sanne Sanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this will have to do for now, but the tests seem a bit odd to me:
they are testing that, having set a certain property, the property is set.
Surely we need to test their impact on different semantics and behaviour?

As follow ups we should test that the dialect specific options are being applied in practice, not just that the property has been set.

@mbellade
Copy link
Member Author

mbellade commented Aug 7, 2025

I'll let @lucamolteni elaborate on the tests topic, but those asserting dialect specific-properties were not meant as full-on "integration tests', i.e. validating that Hibernate ORM and its dialects act according to the configuration, but more like pseudo-unit tests providing validation that the dialect checks within Quarkus' ORM extension work correctly and set the properties on the session factory - then it's Hibernate's concern whether they work correctly within the dialect.

I also had some concerns regarding those, especially since they use QuarkusUnitTest thus trigger a full Quarkus startup, so definitely +1 on reviewing them in a next iteration.

Copy link

quarkus-bot bot commented Aug 7, 2025

Status for workflow Quarkus CI

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

✅ 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.


Flaky tests - Develocity

⚙️ JVM Integration Tests - JDK 17 Windows

📦 integration-tests/grpc-hibernate

com.example.grpc.hibernate.BlockingRawTest.shouldAdd - History

  • Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1160)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1129)
	at com.example.grpc.hibernate.BlockingRawTestBase.shouldAdd(BlockingRawTestBase.java:59)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

@Sanne Sanne merged commit fd19048 into quarkusio:main Aug 7, 2025
57 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.26 - main milestone Aug 7, 2025
@yrodiere
Copy link
Member

To be added in the Migration Guide:

Added to https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.26

@yrodiere
Copy link
Member

This also fixes #13522 . Linking...

@lucamolteni
Copy link
Contributor

I'll let @lucamolteni elaborate on the tests topic, but those asserting dialect specific-properties were not meant as full-on "integration tests', i.e. validating that Hibernate ORM and its dialects act according to the configuration, but more like pseudo-unit tests providing validation that the dialect checks within Quarkus' ORM extension work correctly and set the properties on the session factory - then it's Hibernate's concern whether they work correctly within the dialect.

Yes exactly, they test a transformation (key seems identical, but one it's a quarkus configuration property, the other is an hibernate session property) and a side-effect (configuration is inside the hibernate session).

It's worth testing IMHO

@yrodiere
Copy link
Member

yrodiere commented Sep 4, 2025

I'll let @lucamolteni elaborate on the tests topic, but those asserting dialect specific-properties were not meant as full-on "integration tests', i.e. validating that Hibernate ORM and its dialects act according to the configuration, but more like pseudo-unit tests providing validation that the dialect checks within Quarkus' ORM extension work correctly and set the properties on the session factory - then it's Hibernate's concern whether they work correctly within the dialect.

Yes exactly, they test a transformation (key seems identical, but one it's a quarkus configuration property, the other is an hibernate session property) and a side-effect (configuration is inside the hibernate session).

It's worth testing IMHO

+1. Testing the full behavior would be nice, but would mostly be useful if we tested in native mode (where the risk is highest), and we obviously cannot have a native integration test for every single property. So when the risk is small, testing that we propagate the setting is IMO a good strategy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants