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

Bump to Hibernate 5.6.3.Final #22301

Merged
merged 1 commit into from
Dec 16, 2021
Merged

Bump to Hibernate 5.6.3.Final #22301

merged 1 commit into from
Dec 16, 2021

Conversation

gastaldi
Copy link
Contributor

It finally undeprecates the TypeDef annotations

It finally undeprecates the TypeDef annotations
@quarkus-bot quarkus-bot bot added the area/dependencies Pull requests that update a dependency file label Dec 16, 2021
@gastaldi gastaldi requested review from gsmet, Sanne and yrodiere December 16, 2021 17:41
@Sanne
Copy link
Member

Sanne commented Dec 16, 2021

thanks for preparing the PR!

@gastaldi gastaldi added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 16, 2021
@gastaldi
Copy link
Contributor Author

gastaldi commented Dec 16, 2021

Any chance to backport this PR to 2.6.1.Final? (#22114 (comment))

@Sanne
Copy link
Member

Sanne commented Dec 16, 2021

Any chance to backport this PR to 2.6.1.Final? (#22114 (comment))

I have no strong objections but there's no strong reason to do it either... just a little risk. IMO it's best to calm down a little with backports, unless there's good reasons, no? People will get it soon, it's not like 2.7 will take ages :)

@gastaldi
Copy link
Contributor Author

@Sanne for me the removal of the deprecation warnings is the main driver to get it asap but I think we can wait for 2.7 next year :)

@gsmet gsmet merged commit 4c3ed4c into quarkusio:main Dec 16, 2021
@quarkus-bot quarkus-bot bot added this to the 2.7 - main milestone Dec 16, 2021
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 16, 2021
@gastaldi gastaldi deleted the bump_hibernate branch December 16, 2021 21:50
@famod
Copy link
Member

famod commented Jan 16, 2022

Looking at #22925 (comment) and given this PR here is marked for imminent 2.6.3, isn't this going to break projects that are using currently latest blaze-persistence 1.6.4?

/cc @beikov @gsmet

@gsmet
Copy link
Member

gsmet commented Jan 16, 2022

Not sure to understand? Current version of BP in Quarkus is not compatible with 5.6.3?

@famod
Copy link
Member

famod commented Jan 16, 2022

@gsmet I got that impression from Blazebit/blaze-persistence#1413

@beikov
Copy link
Contributor

beikov commented Jan 16, 2022

There is a breaking change in the org.hibernate.hql.spi.id.MultiTableBulkIdStrategy SPI in 5.6.3.Final but AFAIU the change is necessary. Not sure if we can somehow make the change in Hibernate in a backwards compatible way, but I'll look into this tomorrow.

@famod
Copy link
Member

famod commented Jan 16, 2022

Just checked my project (which is using blaze-persistence-integration-hibernate-5.4) with current Quarkus main (which has Hibernate 5.6.3) and I'm no seeing any issues.
So I suppose this error only pops up if using some specific BP feature.

@beikov
Copy link
Contributor

beikov commented Jan 16, 2022

Are you overriding the hibernate.hql.bulk_id_strategy config property?

@famod
Copy link
Member

famod commented Jan 16, 2022

No, we don't.

@beikov
Copy link
Contributor

beikov commented Jan 16, 2022

Then the contributor is maybe not applied correctly when using Quarkus for some reason. I'll dig into that.

@famod
Copy link
Member

famod commented Jan 16, 2022

Hum, we are generating the ids of our entities on our own (UUIDs), so maybe that's the reason?

@gsmet
Copy link
Member

gsmet commented Jan 16, 2022

@beikov @Sanne we need this sorted out before Wednesday evening. Either by avoiding the SPI breakage or by integrating a new BP version in the Platform for 2.6.3.Final.

@beikov
Copy link
Contributor

beikov commented Jan 16, 2022

Ok, so the Quarkus Hibernate bootstrap somehow strips off configuration values contributed in MetadataContributor implementations, which is why Blazebit/blaze-persistence#1413 is not an issue for Quarkus users.

Working on a fix for that now.

Apart from that, yes, I think that we should try to get a Hibernate 5.6.4.Final out that avoids breaking SPIs. I'll work on that first thing on Monday. From my experiments in Blaze-Persistence I can see that constructing the new additional parameter is possible through the other parameters, so we could add a default method:

interface MultiTableBulkIdStrategy {
    default void prepare(JdbcServices jdbcServices, JdbcConnectionAccess connectionAccess, MetadataImplementor metadata, SessionFactoryOptions sessionFactoryOptions) {
        final SqlStringGenerationContext sqlStringGenerationContext = SqlStringGenerationContextImpl.fromExplicit(
                jdbcServices.getJdbcEnvironment(), metadata.getDatabase(),
                sessionFactoryOptions.getDefaultCatalog(), sessionFactoryOptions.getDefaultSchema());
        prepare(jdbcServices, connectionAccess, metadata, sessionFactoryOptions, sqlStringGenerationContext);
    }
}

@beikov
Copy link
Contributor

beikov commented Jan 16, 2022

FYI I created https://hibernate.atlassian.net/browse/HHH-15032 for the issue.

@Sanne
Copy link
Member

Sanne commented Jan 16, 2022

I was about to tag 5.6.4.Final but I'll hold off for now, and changed HHH-15032 to a blocker.

Please remember I won't be around in the next few days.

@gsmet gsmet modified the milestones: 2.7 - main, 2.6.3.Final Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants