[WIP] QUARKUS-3363 - Offline startup for Hibernate ORM in Quarkus

[sebersole]

https://issues.redhat.com/browse/QUARKUS-3363
Fixes #13522

Handle ORM DialectSpecificSettings

[quarkus-bot]

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

• description should not be empty, describe your intent or provide links to the issues this PR is fixing (using Fixes #NNNNN) or changelogs

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

[quarkus-bot]

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

[github-actions]

🎊 PR Preview 11f6c5b has been successfully built and deployed to https://quarkus-pr-main-43396-preview.surge.sh/version/main/guides/

• Images of blog posts older than 3 months are not available.
• Newsletters older than 3 months are not available.

[yrodiere]

Thanks! I added a few comments below, we can discuss on Zulip if you want.

Regarding documentation, it would probably make sense to add a subsection under "Dialects > Supported databases"; see here for the source, here for the rendered version for this PR (updates take a few minutes/hours after you push).
Note the subsection we'd add to the documentation does not need to list all configuration properties -- that's already done in this automatically-generated list. We would just need to mention that some dialects can be configured in more details to match the DB configuration, and link to the list.

Regarding tests, I think your only solution ATM would be to add something under integration-tests/jpa-oracle, and integration-tests/jpa-mysql, and so on.

You can take inspiration from DefaultSchemaTest for your test; I picked this as an example because it overrides some settings compared to other tests, which I think you will need to do. Relevant classes:

• DefaultSchemaTest
• DefaultSchemaInGraalITCase (same test extending the first, but run in native mode)
• main package with the entity and REST resource being tested: yes it's a bit weird, but that's how we can test even a native image, where the binary cannot (reasonably) run JUnit by itself.

EDIT: Actually, since we're talking about build-time properties here, this won't work. I think your only option would be to add a second persistence unit to application.properties. You may need to have this new PU start offline if the settings you need to test won't work with the DB we test against -- but that's no big deal, as you mainly want to make sure the Dialect's state matches the settings you set.

Starting a PU offline will require an additional config property though, to explicitly ask Hibernate ORM to "start offline"... which was the point of #13522, and the reason we're introducing all these dialect-specific settings.

[yrodiere]

Note we have use dedicated configuration for MariaDB in Quarkus, e.g. there is a db-kind named mariadb, distinct from mysql. It would probably make sense to do the same here?

FWIW you might be able to use inheritance with config groups if you want to avoid duplicating code... I think Guillaume added that support in his recent changes to the config annotation processor. We'll need to double checked by having a look at the resulting documentation, and of course through tests.

[sebersole]

I could just add MySQLDialectConfig mariadb();, no?

[yrodiere]

True, for now this would work.

[yrodiere]

Note:

1. Such links won't appear in generated documentation.
2. Application users are unlikely to reach this javadoc directly, they'll most likely read the rendered version included in the Quarkus documentation -- and in IDE plugins.

That's why we tend to do things like this:

quarkus/extensions/hibernate-search-orm-outbox-polling/runtime/src/main/java/io/quarkus/hibernate/search/orm/outboxpolling/runtime/HibernateSearchOutboxPollingBuildTimeConfigPersistenceUnit.java

Lines 88 to 90 in eeaecac

	* Refer to
	* link:{hibernate-orm-docs-url}#basic-uuid[this section of the Hibernate ORM documentation]
	* to see the possible UUID representations.

Reminder that Asciidoc syntax requires @asciidoctlet in the Javadoc comment.

hibernate-orm-docs-url is defined here, in case you want to add links to Hibernate ORM Javadocs -- but since nowadays you have a list of documentation properties in the Hibernate ORM user guide, I guess it could be more practical to link directly there?

[sebersole]

Reminder that Asciidoc syntax requires @asciidoctlet in the Javadoc comment.

+1, sure just have not been using asciidoc formatting yet. But clearly I will need to start :)

[sebersole]

but since nowadays you have a list of documentation properties in the Hibernate ORM user guide, I guess it could be more practical to link directly there?

I think that's a good idea, Currently we don't generate url anchors for each setting as we'd need to, but https://hibernate.atlassian.net/browse/HHH-18654

[yrodiere]

DatabaseKind.isDB2(name) seems more appropriate? Same for others.

[sebersole]

No, because this is also called with the Dialect name.

[yrodiere]

I think this will not work well for db-kind then, because some database kinds (e.g. postgresql) have more than one corresponding string (e.g. pg). You'll probably need two different classes.

For what it's worth, we already have code that infers the "db product name" (the Hibernate ORM one) from the explicit dialect here:

quarkus/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java

Lines 1137 to 1141 in c09e69a

	if (dbKind.isPresent() && DatabaseKind.is(dbKind.get(), item.getDbKind())
	// Set the default version based on the dialect when we don't have a datasource
	// (i.e. for database multi-tenancy)
	|| explicitDialect.isPresent() && item.getMatchingDialects().contains(explicitDialect.get())) {
	dbProductName = item.getDatabaseProductName();

So maybe you should base your code on this dbProductName instead?

[sebersole]

I think this will not work well for db-kind then, because some database kinds (e.g. postgresql) have more than one corresponding string (e.g. pg).

Look at the callers. One works with a passed "database kind" and manually does the normalize which handles the case you were talking about (multiple names). The other works with the Dialect name.

So maybe you should base your code on this dbProductName instead?

Maybe, I'll take a look.

[sebersole]

Currently this code handles (1) db-kind and (2) explicit Dialect name.

Are you saying that it is legal to specify a dbProductName but not db-kind?

[sebersole]

So the "problem" with using dbProductName is that, unless Quarkus does this somewhere, there is no simple mapping of dbProductName to Dialect or SupportedDatabaseKind.

Assuming that it is legal to have just dbProductName without either db-kind or dialect, we'd need a method that handles mapping dbProductName to a SupportedDatabaseKind

[sebersole]

there is no simple mapping of dbProductName to Dialect or SupportedDatabaseKind

Well, actually, I could leverage org.hibernate.dialect.Database to implement this...

[sebersole]

Ok, I added handling based on database product name as well.

[yrodiere]

Thanks. Then if tests pass, we're all set.

FYI, the only way the database product name can be missing is if someone has no assigned datasource (because it's dynamic, see database multi-tenancy). Even then, users will need to set a dialect explicitly, and as long as they use one of the "standard" dialects, the database product name will be inferred from that dialect.

[sebersole]

Was the integration test I added run? I'm not sure which Action check runs those integration tests.

[yrodiere]

Was the integration test I added run? I'm not sure which Action check runs those integration tests.

CI does not run on drafts, you need to mark as ready for review and add [WIP] to the title. I'll do it.

[yrodiere]

Was the integration test I added run? I'm not sure which Action check runs those integration tests.

CI does not run on drafts, you need to mark as ready for review and add [WIP] to the title. I'll do it.

Though the recommended way of working if your code is not ready for review is to run GitHub Actions in your fork instead.

[sebersole]

Imports are not sorted in /home/runner/work/quarkus/quarkus/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java

Um, I ran the formatter before I pushed. Even now, locally, I run the formatter and nothing changes.

What do I need to do here? I find it strange that the IJ format stuff does not handle this either?

[yrodiere]

Imports are not sorted in /home/runner/work/quarkus/quarkus/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java

Um, I ran the formatter before I pushed. Even now, locally, I run the formatter and nothing changes.

What do I need to do here? I find it strange that the IJ format stuff does not handle this either?

How did you format code?

The formatter does not handle import order, that's a different setting.

If you installed the Eclipse Formatter plugin, you need to point to the import order config explicitly, it's a different file.
If you format from the command line, you must make sure to call the right goal/plugin.

[sebersole]

I followed the directions from CONTRIBUTING :

Formatting

Open the Preferences window (or Settings depending on your edition), navigate to Plugins and install the [Adapter for Eclipse Code Formatter](https://plugins.jetbrains.com/plugin/6546-eclipse-code-formatter) from the Marketplace.

Restart your IDE, open the Preferences (or Settings) window again and navigate to Adapter for Eclipse Code Formatter section on the left pane.

Select Use Eclipse's Code Formatter, then change the Eclipse workspace/project folder or config file to point to the eclipse-format.xml file in the independent-projects/ide-config/src/main/resources directory. Make sure the Optimize Imports box is ticked. Then, select Import Order from file and make it point to the eclipse.importorder file in the independent-projects/ide-config/src/main/resources directory.

[sebersole]

I "format" using mvnw -f extensions/hibernate-orm/deployment net.revelc.code.formatter:formatter-maven-plugin:2.24.1:format because that's what the actions suggested earlier.

[yrodiere]

I "format" using mvnw -f extensions/hibernate-orm/deployment net.revelc.code.formatter:formatter-maven-plugin:2.24.1:format because that's what the actions suggested earlier.

I suppose the formatting plugin failed and suggested it. Here it's another plugin failing.

To run formatting + import sorting:

quarkus/CONTRIBUTING.md

Line 273 in 4eee652

If you want to run the formatting without doing a full build, you can run `./mvnw process-sources`.

Or if you want to limit to one module:

./mvnw process-sources -f extensions/hibernate-orm

[yrodiere]

This part should have ensured that imports get sorted when you run auto-formatting in your IDE:

Then, select Import Order from file and make it point to the eclipse.importorder file in the independent-projects/ide-config/src/main/resources directory.

There something wrong if CTRL+ALT+O / CTRL+ALT+L still doesn't format code correctly...

[yrodiere]

Hey @sebersole , sorry I left this aside... were you waiting for feedback from me?

From what I can see you finished implementing most setting, and were trying to run tests.

Though I suspect in order to run tests, we will need to introduce the config property that QUARKUS-3363 actually is about (the dialect config just happens to be a prerequisite): something that tells Hibernate ORM to start offline, i.e. that sets hibernate.boot.allow_jdbc_metadata_access to false (on runtime startup), and probably triggers exceptions if users requested automatic schema creation/update/validation.

[yrodiere]

Suggested change

	quarkus.hibernate."offline".mariadb.bytesPerCharacter=1
	quarkus.hibernate."offline".mariadb.noBackslashEscapes=true
	quarkus.hibernate."offline".mariadb.storageEngine=MY_CUSTOM_ENGINE
	quarkus.hibernate-orm."offline".mariadb.bytesPerCharacter=1
	quarkus.hibernate-orm."offline".mariadb.noBackslashEscapes=true
	quarkus.hibernate-orm."offline".mariadb.storageEngine=MY_CUSTOM_ENGINE