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

schema update or validation #536

Closed
yrodiere opened this issue Feb 4, 2021 · 24 comments
Closed

schema update or validation #536

yrodiere opened this issue Feb 4, 2021 · 24 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@yrodiere
Copy link
Member

yrodiere commented Feb 4, 2021

See quarkusio/quarkus#14264 (comment)

Whenever one sets hibernate.hbm2ddl.auto to validate or update, the schema management tool apparently tries to get a JDBC connection and ultimately fails with an error like this:

java.lang.RuntimeException: java.lang.RuntimeException: Failed to start quarkus
        at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:502)
        at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeBeforeAllCallbacks$8(ClassBasedTestDescriptor.java:368)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.invokeBeforeAllCallbacks(ClassBasedTestDescriptor.java:368)
        at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:192)
        at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:78)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:136)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
        at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
        at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
        at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:143)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
        at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
        at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
        at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
        at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:108)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
        at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:96)
        at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:75)
        at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.execute(JUnitPlatformProvider.java:188)
        at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invokeAllTests(JUnitPlatformProvider.java:154)
        at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:128)
        at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:428)
        at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
        at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:562)
        at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:548)
Caused by: java.lang.RuntimeException: Failed to start quarkus
        at io.quarkus.runner.ApplicationImpl.doStart(ApplicationImpl.zig:707)
        at io.quarkus.runtime.Application.start(Application.java:90)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
        at io.quarkus.runner.bootstrap.StartupActionImpl.run(StartupActionImpl.java:212)
        at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:458)
        ... 40 more
Caused by: javax.persistence.PersistenceException: Unable to build EntityManagerFactory
        at io.quarkus.hibernate.reactive.runtime.FastBootHibernateReactivePersistenceProvider.createEntityManagerFactory(FastBootHibernateReactivePersistenceProvider.java:93)
        at javax.persistence.Persistence.createEntityManagerFactory(Persistence.java:80)
        at javax.persistence.Persistence.createEntityManagerFactory(Persistence.java:55)
        at io.quarkus.hibernate.orm.runtime.JPAConfig$LazyPersistenceUnit.get(JPAConfig.java:117)
        at io.quarkus.hibernate.orm.runtime.JPAConfig.startAll(JPAConfig.java:41)
        at io.quarkus.hibernate.orm.runtime.HibernateOrmRecorder.startAllPersistenceUnits(HibernateOrmRecorder.java:88)
        at io.quarkus.deployment.steps.HibernateOrmProcessor$startPersistenceUnits951856026.deploy_0(HibernateOrmProcessor$startPersistenceUnits951856026.zig:74)
        at io.quarkus.deployment.steps.HibernateOrmProcessor$startPersistenceUnits951856026.deploy(HibernateOrmProcessor$startPersistenceUnits951856026.zig:40)
        at io.quarkus.runner.ApplicationImpl.doStart(ApplicationImpl.zig:586)
        ... 47 more
Caused by: java.lang.UnsupportedOperationException: The application must supply JDBC connections
        at org.hibernate.engine.jdbc.connections.internal.UserSuppliedConnectionProviderImpl.getConnection(UserSuppliedConnectionProviderImpl.java:44)
        at org.hibernate.engine.jdbc.env.internal.JdbcEnvironmentInitiator$ConnectionProviderJdbcConnectionAccess.obtainConnection(JdbcEnvironmentInitiator.java:180)
        at org.hibernate.resource.transaction.backend.jdbc.internal.DdlTransactionIsolatorNonJtaImpl.getIsolatedConnection(DdlTransactionIsolatorNonJtaImpl.java:43)
        at org.hibernate.tool.schema.internal.exec.ImprovedExtractionContextImpl.getJdbcConnection(ImprovedExtractionContextImpl.java:60)
        at org.hibernate.tool.schema.extract.internal.SequenceInformationExtractorLegacyImpl.extractMetadata(SequenceInformationExtractorLegacyImpl.java:40)
        at org.hibernate.tool.schema.extract.internal.DatabaseInformationImpl.initializeSequences(DatabaseInformationImpl.java:65)
        at org.hibernate.tool.schema.extract.internal.DatabaseInformationImpl.<init>(DatabaseInformationImpl.java:59)
        at org.hibernate.tool.schema.internal.Helper.buildDatabaseInformation(Helper.java:155)
        at org.hibernate.tool.schema.internal.AbstractSchemaValidator.doValidation(AbstractSchemaValidator.java:61)
        at org.hibernate.tool.schema.spi.SchemaManagementToolCoordinator.performDatabaseAction(SchemaManagementToolCoordinator.java:192)
        at org.hibernate.tool.schema.spi.SchemaManagementToolCoordinator.process(SchemaManagementToolCoordinator.java:73)
        at org.hibernate.internal.SessionFactoryImpl.<init>(SessionFactoryImpl.java:316)
        at org.hibernate.reactive.session.impl.ReactiveSessionFactoryImpl.<init>(ReactiveSessionFactoryImpl.java:33)
        at io.quarkus.hibernate.reactive.runtime.boot.FastBootReactiveEntityManagerFactoryBuilder.build(FastBootReactiveEntityManagerFactoryBuilder.java:31)
        at io.quarkus.hibernate.reactive.runtime.FastBootHibernateReactivePersistenceProvider.createEntityManagerFactory(FastBootHibernateReactivePersistenceProvider.java:83)
        ... 55 more

Everything works fine with other modes that only rely on GenerationTarget and only push "write" commands, because Hibernate Reactive uses a ReactiveGenerationTarget to redirect the commands to the reactive driver.

But validate and update need to read the database schema, and that code doesn't rely on the GenerationTarget, but on SQL commands sent through JDBC directly.

I believe we need another abstraction in the schema management tool to make it use the reactive driver instead of JDBC?

@yrodiere yrodiere added the bug Something isn't working label Feb 4, 2021
@gavinking
Copy link
Member

Right, there's nothing like the JDBC metadata API in Vert.x currently so this would have to be done by queries against the information schema. So it's probably a fair bit of work.

OTOH if you do have a JDBC driver, it should happily just use it.

@gavinking gavinking added enhancement New feature or request and removed bug Something isn't working labels Feb 10, 2021
@gavinking gavinking changed the title Schema generation fails when set to "validate" or "update" due to lack of JDBC connection schema update or validation Feb 10, 2021
@gavinking
Copy link
Member

We should document this in limitations.

@gavinking
Copy link
Member

So for this issue we need to provide a partial implementation of either the JDBC metadata API, or of whatever abstraction Hibernate core has of the JDBC metadata (I forget and I have not checked), for each of the dialects we support, by directly querying the information schema.

(Well, we don't need to support all 3 dialects initially, we would start with postgres.)

Note that it doesn't actually need to be non-blocking, but it does need to use the Vert.x client to execute the SQL.

@DavideD would you like to take a stab at that work?

@DavideD
Copy link
Member

DavideD commented Feb 24, 2021 via email

@DavideD DavideD self-assigned this Feb 24, 2021
@gavinking gavinking added this to the After 1.0 milestone Mar 4, 2021
@gbadner gbadner assigned gbadner and unassigned DavideD May 5, 2021
@gbadner
Copy link
Contributor

gbadner commented May 26, 2021

@gavinking, I am able to query sequence metadata using PostgreSQL, but the results are not processed properly.

Hibernate ORM uses SequenceInformationExtractorLegacyImpl to process the metadata. It uses ResultSet#getLong to get values for start_value, minimum_value, maximum_value, and increment columns.

Currently, the values for those columns in the ResultSetAdapter are Strings. io.vertx.sqlclient.Row#getLong(int pos) ends up casting the String value as a Long and throwing a ClassCastException.

In the debugger using ORM, I can see that these column values are arrays of ASCII values. The JDBC driver knows how to convert an array to a long value, so there's no problem using SequenceInformationExtractorLegacyImpl for ORM.

Where would you consider the bug to be? In io.vertx.sqlclient.Row#getLong(int pos), in HR's ResultSetAdapter, or should HR use a different implementation for SequenceInformationExtractor for PostgreSQL that expects String values?

@gavinking
Copy link
Member

How are those columns defined in the database schema itself?

@gavinking
Copy link
Member

Oh, I see, it's the schema itself that's wrong:

                              View "information_schema.sequences"
         Column          |                Type                | Collation | Nullable | Default 
-------------------------+------------------------------------+-----------+----------+---------
 sequence_catalog        | information_schema.sql_identifier  |           |          | 
 sequence_schema         | information_schema.sql_identifier  |           |          | 
 sequence_name           | information_schema.sql_identifier  |           |          | 
 data_type               | information_schema.character_data  |           |          | 
 numeric_precision       | information_schema.cardinal_number |           |          | 
 numeric_precision_radix | information_schema.cardinal_number |           |          | 
 numeric_scale           | information_schema.cardinal_number |           |          | 
 start_value             | information_schema.character_data  |           |          | 
 minimum_value           | information_schema.character_data  |           |          | 
 maximum_value           | information_schema.character_data  |           |          | 
 increment               | information_schema.character_data  |           |          | 
 cycle_option            | information_schema.yes_or_no       |           |          | 

@gavinking
Copy link
Member

So I know the Vert.x guys don't want to do type conversions in the driver, so we need to do the conversion at a higher level. So I guess either subclass ResultSetAdaptor or SequenceInformationExtractor. I'm not sure which is more convenient.

@gbadner
Copy link
Contributor

gbadner commented May 26, 2021

@gavinking, thanks for the feedback. I think, for now, that I will subclass ResultSetAdaptor to see what all is needed for the various dialects, then I'll see which is more convenient.

@edeandrea
Copy link
Contributor

Hi - I just wanted to see if there was any progress on this?

@gavinking
Copy link
Member

Hi - I just wanted to see if there was any progress on this?

@gbadner?

@gbadner
Copy link
Contributor

gbadner commented Jul 21, 2021

@edeandrea, yes, there has been good progress for PostgreSQL. @DavideD has looked at a draft, and I am incorporating his recommendations. If nothing else gets in the way, I will have a PR by the end of the week.

@edeandrea
Copy link
Contributor

Thanks @gbadner for the update! Would this issue not be considered complete until support for all the relational databases is available?

@gavinking
Copy link
Member

yes, there has been good progress for PostgreSQL

Great! 👍

@gavinking
Copy link
Member

Would this issue not be considered complete until support for all the relational databases is available?

I mean eventually we would want to support all databases, but IMO that shouldn't block merging of the Postgres support.

Furthermore, my bet is it will be pretty straightforward to add additional databases once we have it working for one of them.

@edeandrea
Copy link
Contributor

I mean eventually we would want to support all databases, but IMO that shouldn't block merging of the Postgres support.

Sweet! Postgres is my current target so that's good news to me :) I don't want to be greedy though!

@gbadner
Copy link
Contributor

gbadner commented Jul 21, 2021

I believe it will be straightforward, at least for those dialects that support information_schema, and have good documentation for querying system tables (like PostgreSQL has).

I've read that DB2 doesn't support information_schema. I'm not sure yet what would be required to get things working for DB2.

@gavinking
Copy link
Member

I mean as far as I know it's part of the definition of being an RDBMS that you have the schema reified as a meta-schema (forget the correct term for that).

@gavinking
Copy link
Member

I've read that DB2 doesn't support information_schema.

I think it does, but it also has syscat and sysibm and I don't know what are the differences between these things.

@gbadner
Copy link
Contributor

gbadner commented Jul 26, 2021

@gavinking , @edeandrea , in case you didn't see, I'm just letting you know that I posted a draft PR for this: #903

@gbadner
Copy link
Contributor

gbadner commented Aug 13, 2021

Oops, meant to delete my previous comment, not close the issue...

@gbadner
Copy link
Contributor

gbadner commented Aug 17, 2021

@gavinking, I just read over comments and noticed:

OTOH if you do have a JDBC driver, it should happily just use it.

Currently, that will not happen. It will always use Vert.X. I'll have to think about this some more.

@gavinking
Copy link
Member

@gbadner I don't think that's important. I mean, it was useful before your work, but now I can't think of a strong reason anyone would prefer to use JDBC.

@gavinking gavinking modified the milestones: After 1.0, 1.0.0.CR10 Aug 31, 2021
@gavinking
Copy link
Member

Done by #903.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants