-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add support for Hibernate's hibernate.hbm2ddl.extra_physical_table_types
#47921
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
base: main
Are you sure you want to change the base?
Add support for Hibernate's hibernate.hbm2ddl.extra_physical_table_types
#47921
Conversation
/cc @gsmet (hibernate-orm) |
hibernate.hbm2ddl.extra_physical_table_types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thanks for the pull request!
...src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRuntimeConfigPersistenceUnit.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. It looks good overall; I added a few minor comments below.
We'll need a test, though. But it can be very simple, see #47854 (comment)
...src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRuntimeConfigPersistenceUnit.java
Outdated
Show resolved
Hide resolved
...src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRuntimeConfigPersistenceUnit.java
Outdated
Show resolved
Hide resolved
...src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRuntimeConfigPersistenceUnit.java
Outdated
Show resolved
Hide resolved
...src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRuntimeConfigPersistenceUnit.java
Outdated
Show resolved
Hide resolved
@yrodiere would you check now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added two comments, though they're mostly the same as on the previous review :)
Also, did you see my answer to your question "how can I test this"? There is a suggestion for a simple way of testing your change.
...src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRuntimeConfigPersistenceUnit.java
Outdated
Show resolved
Hide resolved
...src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRuntimeConfigPersistenceUnit.java
Outdated
Show resolved
Hide resolved
@yrodiere sorry for being late, already added what you aforementioned above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. The implementation looks good, but as I commented below I think the test will fail -- let's try though :)
...eployment/src/test/java/io/quarkus/hibernate/orm/config/properties/ConfigPropertiesTest.java
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🎊 PR Preview b57f54e has been successfully built and deployed to https://quarkus-pr-main-47921-preview.surge.sh/version/main/guides/
|
@Chu3laMan it seems your code contains formatting/import issues. Please have a look at https://github.com/quarkusio/quarkus/blob/main/CONTRIBUTING.md#ide-config-and-code-style Please also see my other comment though; I expect tests will fail. |
overrideConfigKey("quarkus.hibernate-orm.\"overridesPU\".database.extra-physical-table-types", | ||
"MATERIALIZED VIEW,FOREIGN TABLE"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be in this method, but next to the other calls to overrideConfigKey
higher up in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yrodiere I already fixed could you check again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You removed the call to overrideConfigKey
, but didn't add it higher up in this file... ?
As is, the test will just fail.
Can you please fix this and rebase?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Status for workflow
|
Status | Name | Step | Failures | Logs | Raw logs | Build scan |
---|---|---|---|---|---|---|
✖ | Initial JDK 17 Build | Build |
Failures | Logs | Raw logs | 🔍 |
You can consult the Develocity build scans.
Failures
⚙️ Initial JDK 17 Build #
- Failing: extensions/hibernate-orm/deployment
! Skipped: docs extensions/flyway/deployment extensions/hibernate-envers/deployment and 97 more
📦 extensions/hibernate-orm/deployment
✖ Failed to execute goal net.revelc.code.formatter:formatter-maven-plugin:2.26.0:validate (default) on project quarkus-hibernate-orm-deployment: File '/home/runner/_work/quarkus/quarkus/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/config/properties/ConfigPropertiesTest.java' has not been previously formatted. Please format file (for example by invoking `mvn -f extensions/hibernate-orm/deployment net.revelc.code.formatter:formatter-maven-plugin:2.26.0:format`) and commit before running validation!
Status for workflow
|
This MR adds support for configuring additional database object types to be included
in schema management operations, such as materialized views or other database-specific objects.
Changes:
Example usage: quarkus.hibernate-orm.extra-physical-table-types=MATERIALIZED VIEW,VIEW