retiring legacy config properties

[gavinking]

Hibernate has way too many toggles and switches. I bet we could get rid of at least some of them without anyone screaming at us.

Here's a rough list of some of the ones that look like maybe we don't need them anymore:

• hibernate.jpa.*—do we really need to diverge from the JPA spec in all these things?
• hibernate.ejb.*—EJB!? If these are still useful they should at least be renamed.
• hibernate.jmx.*—does anyone still use JMX?
• hibernate.connection.acquisition_mode—this is already deprecated.
• hibernate.connection.release_mode—this is already deprecated.
• hibernate.jdbc.lob_noncontextual_creation—should be a Dialect attribute, it seems to me.
• hibernate.jdbc.use_scrollable_resultset—should be a Dialect attribute, it seems to me.
• hibernate.jdbc.use_get_generated_keys—should be a Dialect attribute, it seems to me.
• hibernate.jdbc.use_streams_for_binary—should be a Dialect attribute, it seems to me.
• hibernate.jdbc.wrap_result_sets—should be a Dialect attribute, it seems to me.
• hibernate.dialect.oracle.prefer_long_raw—I have a philosophical objection to this one: settings don't belong to dialects!
• hibernate.id.new_generator_mappings—if the new ones are better, let's just use them.
• hibernate.jpaql_strict_compliance
• hibernate.query.substitutions—we don't just pass through tokens anymore in the "new" HQL, so I don't think this really makes much sense anymore.
• hibernate.query.validate_parameters
• hibernate.query.conventional_java_constants
• hibernate.legacy_limit_handler—we don't need legacy things.
• hibernate.transaction.factory_class—this is already deprecated.
• hibernate.native_exception_handling_51_compliance—ugh, the name says it all.

Surely at least some of these are still needed, but I doubt all of them are. Certain of them should be made attributes of the Dialect rather than config properties.

[yrodiere]

I agree with the general goal. But as the devil is in the details, I'd like to point out a few things...

Things like use_get_generated_keys are detected from JDBC connection metadata by default. But... the setting is there to cope with incorrect detection, which means at least in some cases, we can't detect it correctly. I know for a fact that we cannot detect use_get_generated_keys setting if the database is down when the application starts. In that case, ORM will silently give up on detecting the setting and use some hard-coded default (false? I don't remember).

More importantly, moving these settings to a dialect attribute may not be correct either. From what I understand, whether features like use_get_generated_keys are supported or not depends on the JDBC driver, not on the database or dialect.

So you can have two drivers for the same database/dialect, one of which supports the feature, while the other doesn't. I know postgresql used to have at least two competing JDBC drivers, but I don't know if it's still the case or if it's a widespread problem.
I suppose that a more likely situation is a database with only one driver, but whose older versions don't support some features. Then, if we stuff these things into the dialect, what should the dialect tell us?

[gavinking]

From what I understand, whether features like use_get_generated_keys are supported or not depends on the JDBC driver, not on the database or dialect.

Yes, that's correct, but they were introduced > 15 years ago for the (quite flaky) JDBC drivers as they existed at that time. I doubt that they're needed anymore, now that the JDBC drivers are all much more mature.

So you can have two drivers for the same database/dialect, one of which supports the feature, while the other doesn't.

Sure, and that used to be a concern for stuff like SQL Server where lots of people were using a third-party driver. But is that still relevant today?

Then, if we stuff these things into the dialect, what should the dialect tell us?

Well remember that the Dialects in H6 are in principle allowed to inspect metadata about the database version and decide what to do. It's not a great stretch to let them decide what to do based on the driver version.

[yrodiere]

the JDBC drivers are all much more mature

Yes of course they all are 😁

But is that still relevant today?

Same question here. I know it was relevant ~6 years ago. Not sure about today.

It's probably not that important anyway, since the problem of different versions of the same driver providing different capabilities will always be valid. But as you answered:

It's not a great stretch to let them decide what to do based on the driver version

Agreed.

[sebersole]

Hibernate has way too many toggles and switches. I bet we could get rid of at least some of them without anyone screaming at us.

+100. In principle

Overall, I assume you are specifically discussing 6 here. You never said specifically though.

Here's a rough list of some of the ones that look like maybe we don't need them anymore:

• hibernate.jpa.*—do we really need to diverge from the JPA spec in all these things?

Some, absolutely. Especially the ones represented by JpaCompliance

E.g. we just talked about how to recognize bags versus lists. That is controlled by a "compliance setting" atm. Personally, for 6, I like the alternative I suggested of making choices based on whether the attribute is typed as a List or a Collections. So that one could go away.

• hibernate.ejb.*—EJB!? If these are still useful they should at least be renamed.

Ha. That was the naming you and Emmanuel decided on. I completely agree

• hibernate.jmx.*—does anyone still use JMX?

These can probably go away, but is really a larger discussion then about dropping all of the JMX support classes

• hibernate.jdbc.lob_noncontextual_creation—should be a Dialect attribute, it seems to me.
• hibernate.jdbc.use_scrollable_resultset—should be a Dialect attribute, it seems to me.
• hibernate.jdbc.use_get_generated_keys—should be a Dialect attribute, it seems to me.
• hibernate.jdbc.use_streams_for_binary—should be a Dialect attribute, it seems to me.
• hibernate.jdbc.wrap_result_sets—should be a Dialect attribute, it seems to me.

Generally I agree. Sometimes though it is useful to turn some of these behaviors off. And I am not a fan (as a user) of having to write and maintain a new Dialect just to disable one little thing.

Some like wrapping ResultSets are completely unnecessary in 6. The main point of 6 in fact is about removing the need for that one in particular

• hibernate.id.new_generator_mappings—if the new ones are better, let's just use them.

Again, for 6 fine. But let's keep in mind that dropping backwards compatibility should not be taken lightly. And this one in particular effects what the expected database set up is.

• hibernate.jpaql_strict_compliance

This is handled now via the more complete JpaCompliance settings, so I agree we can drop this one in favor of hibernate.jpa.compliance.query

• hibernate.query.substitutions—we don't just pass through tokens anymore in the "new" HQL, so I don't think this really makes much sense anymore.

• 2000. Hm, that may not be strong enough. +10,000

• hibernate.query.validate_parameters
• hibernate.query.conventional_java_constants
• hibernate.legacy_limit_handler—we don't need legacy things.
• hibernate.transaction.factory_class—this is already deprecated.

+1

• hibernate.native_exception_handling_51_compliance—ugh, the name says it all.

No need to be rude here. We all have made past mistakes.

[gavinking]

I assume you are specifically discussing 6 here

Yes, of course.

[gavinking]

Folks, how do we make progress on this?

I'm happy to do the work, but I need to get permission from ya'll as to which settings I can nuke.

[sebersole]

I think someone just needs to go through them and come up with a preliminary list to consider. At least I don't see another way to start. And I think that is best done here as opposed to say creating a PR removing a bunch and discussing from there

…

On Wed, Sep 8, 2021, 4:03 AM Gavin King ***@***.***> wrote: Folks, how do we make progress on this? I'm happy to do the work, but I need to get permission from ya'll as to which settings I can nuke. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3791 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABZIE7UC3PGGBYCFZOMUUTUA4YGZANCNFSM4YVLXEKA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[sebersole]

I know you started. I'll go through the ones you've listed, but having difficulty with my laptop atm and it's impossible to deal with that list from phone

[gavinking]

Yeah, I mean if you—and others—could just go through and cross off that list the settings you think are still useful or that too many users rely on, then that would already be a good start.

[beikov]

hibernate.jpa.static_metamodel.population

I think this setting is a nice way to improve startup time, but I have no strong opinion.

hibernate.jpa.compliance.query

I guess this is relevant for people that want to avoid using non-JPA features, so this one should probably stay.

hibernate.jpa.compliance.list

Removing and enabling this by default would cause mappings to change their semantics, so I don't think we should remove that.

hibernate.jpa.compliance.proxy

Removing and enabling this by default would be a major pain IMO as I know tons of applications that rely on id access not causing a DB query. So this should stay.

hibernate.jpa.compliance.global_id_generators

I know of applications that rely on this setting to be disabled to avoid duplicating sequence names in the generator names, but I guess we can get rid of this now and enable it by default as the default sequence name is now derived from the generator name.

hibernate.jpa.compliance.transaction
hibernate.jpa.compliance.closed
hibernate.jpa.compliance.caching

I have no strong opinion on those. IMO we can remove and enable these settings by default.

hibernate.ejb.*

The deprecated configs certainly can be removed. I don't mind renaming other properties

hibernate.jmx.*

I think it is still used by some tools. Since JMX is part of Java SE, I tend to keep it, but I never used it, so I don't have a strong opinion on that.

hibernate.connection.acquisition_mode
hibernate.connection.release_mode

Remove all the deprecated ones :)

hibernate.jdbc.*

I don't know for sure, but I think Quarkus is a user for some of these config options to improve startup time. Specifying these values allows to continue the startup without a JDBC connection, but maybe @Sanne knows more.

hibernate.dialect.oracle.prefer_long_raw

Well, there are some other settings for other dialects as well, like for HANA and MySQL. The Oracle setting can IMO go away. Some other settings like hibernate.dialect.storage_engine will probably have to stay for Quarkus, but we could at least determine a default based on the current default engine by querying as per https://dev.mysql.com/doc/refman/5.7/en/innodb-check-availability.html
The HANA settings seem to be preference or performance related, so I guess they should stay around as well.

hibernate.id.new_generator_mappings

+1 for removing and enabling by default

hibernate.query.jpaql_strict_compliance
hibernate.query.substitutions

+1 for removing without replacement

hibernate.query.validate_parameters

+1 for removing and enabling by default

hibernate.query.conventional_java_constants

Not sure about this one, but it seems like a strange setting to me which we don't even make use of anymore in 6.0. So I'm +0 for removing this without replacement.

hibernate.legacy_limit_handler

+1000 for removing this. We need to cleanup the Dialect and some other classes anyway.

hibernate.transaction.factory_class

+1 for removing without replacement

hibernate.native_exception_handling_51_compliance

+1 for removing without replacement

[dreab8]

Agree with Christian to have also an opinion from @Sanne.

[sebersole]

I already gave replies initially, but I'll comment here again based on these discussions..

== hibernate.jpa.compliance.*

I'd prefer to keep the JPA compliance settings, for the most part. I added hibernate.jpa.compliance.load_by_id in 6, but frankly don't care particularly much if we want to drop that if noone sees it as beneficial.

In general, I'd suggest looking at JpaCompliance to get a feel for the purpose of these. I personally think they mostly still make sense. E.g. do we really want to disallow access to Transaction in JTA environment just because JPA says we should?

hibernate.jpa.compliance.global_id_generators
I know of applications that rely on this setting to be disabled to avoid duplicating sequence names in the generator names, but I > guess we can get rid of this now and enable it by default as the default sequence name is now derived from the generator name.

But this setting is not limited to derived names. In fact, its actually much more relevant for explicitly named ones. IMO this one should stay - we have a much more sane approach here

== hibernate.jpa.*

I generally agree with dropping the non-compliance-related ones.

hibernate.jpa.static_metamodel.population - The only point here to discuss is what to do with "non standard" mappings (ANY, etc).

== hibernate.jmx.*

I think it is still used by some tools. Since JMX is part of Java SE, I tend to keep it, but I never used it, so I don't have a strong opinion on that.

Its more a question of whether people use our JmxService. I cannot answer that definitively. Jira would suggest no.
+1 to remove for me

== hibernate.id.new_generator_mappings
its not a matter of better. its a question of upgradeability since the new ones have very different interpretation of the "stored values"). That said, its been long enough imo, so +1 to drop.

== Misc

• I saw a few comments along the lines of "remove them and enable these settings by default". Well, if we remove the settings "by default" is the only option - you'd have no way to adjust them. Completely obvious yes, but let's make sure that is an acceptable situation. Its more than just what will be the default.

== default {}

Aside from the specific ones above, +1 for dropping the rest

[Sanne]

Totally agree on the principle as well - but happy to follow the project lead's advice about such details.

In those few cases in which @sebersole doesn't feel strongly, I would suggest to drop flags aggressively. IMO it's totally fine to have the 6.0 release to have a stronger accent of "this is the future"; we can always reintroduce some selection in later minor versions, should there be evidence of real and strong need from feedback.

Regarding the JPA compatibility flags, I also agree with Steve: AFAIR these represent particular situations in which we believe the Hibernate semantics are "better" than JPA but also in direct inconsistency with JPA requirements. We should keep offering the "better" option, assuming we still believe that's the case, while some people might need consistency with JPA; so these might be an unavoidable pain point that we'll need to keep around. I suppose ideally we should review each of these to see if the JPA spec isn't too bad, after all - but even in this case we might need a deprecation process so perhaps it's best to face such topics a different time.

[sebersole]

Regarding the JPA compatibility flags, I also agree with Steve: AFAIR these represent particular situations in which we believe the Hibernate semantics are "better" than JPA but also in direct inconsistency with JPA requirements. We should keep offering the "better" option, assuming we still believe that's the case, while some people might need consistency with JPA; so these might be an unavoidable pain point that we'll need to keep around. I suppose ideally we should review each of these to see if the JPA spec isn't too bad, after all - but even in this case we might need a deprecation process so perhaps it's best to face such topics a different time.

Generally this comes down to balancing between (1) better behavior, (2) spec compliance (tck) and (3) legacy behavior

isJpaListComplianceEnabled

The TCK often checks for things it defines as illegal query syntax though many times it is something Hibernate supports (implicit SELECT clause, etc). We need this to continue to support those better constructs while still having the ability to pass the TCK.

isJpaTransactionComplianceEnabled

The spec says that Transaction/EntityTransaction can only be accessed in a non-JTA environment (local transactions) - the intention being one would instead use JTA access in JTA environments. I think our behavior is better, but again we need this to pass the TCK while continuing to support.

isJpaListComplianceEnabled

Ok, this one is not so straight-forward. In my opinion we made a poor choice here originally in terms of interpreting a mapping like below as a bag mapping:

@OneToMany(...)
List<?> someObjects;

In a clean-room, I'd change this so that we only chose bag when either:

1. Collection is used instead of List
2. Mapping uses a (not yet existing) @Bag annotation

But now we have a legacy consideration here. Changing this has the potential to mess up existing applications in potentially subtle ways.

So this one is a good candidate to consider removing.

isJpaClosedComplianceEnabled

This is one we can consider getting rid of as I think the "legacy" support is not that compelling compared to the JPA requirement to justify a setting. This mainly comes down to how to handle calls to EM#close and EMF#close when those objects are already closed. Historically Hibernate allowed idempotence for these calls, whereas JPA says it is an error.

I can be convinced to drop this one if everyone feels it is not worth it

isJpaProxyComplianceEnabled

This boils down to calling #getId on a proxy. JPA says this should load the entity and the TCK checks this. Unless we always want getting the identifier from a proxy to trigger initialization of the proxy, we need to keep this one.

isJpaCacheComplianceEnabled

To be honest, I forget the exact reason I added this one. But for sure, this was a case where the TCK asserts something not explicitly stated in the spec, but we "lost the challenge". We can certainly drop it and see what (if anything) breaks.

isGlobalGeneratorScopeEnabled

We've discussed this one already. Depends whether you think the JPA behavior is logically illogical :D Personally I find the JPA behavior confusing.

isJpaOrderByMappingComplianceEnabled

According to the spec, the only valid items here are attribute references. Hibernate supports many types of references (column, SQL expressions, etc).

isLoadByIdComplianceEnabled

I think this one is not relevant. It was something I was working on for 6, but I believe I rolled back that feature and just missed removing this

[gavinking]

The TCK often checks for things it defines as illegal query syntax

Tsk, that's kinda garbage and stupid. What a pain.

Changing this has the potential to mess up existing applications in potentially subtle ways.

Yeah, changing this sounds like it would break way too many things.

I can be convinced to drop this one if everyone feels it is not worth it

I'm fine either way. It's probably not worth having.

JPA says this should load the entity and the TCK checks this. Unless we always want getting the identifier from a proxy to trigger initialization of the proxy

Ugh. I hate that so we should definitely keep it.

[gavinking]

Alternative suggestion: why not just roll all these things into one property setting that we only turn on when we run the TCK?

[sebersole]

That is actually the plan. To allow hibernate.jpa.compliance itself as a config setting. Conceptually it sets the default for the others, but you would just be able to set that and affect the others

[gavinking]

Folks, this issue is getting urgent.

There are actually quite some properties in AvailableSettings which are now unused or AFAICT completely useless.

[gavinking]

Like, hibernate.cache.provider_configuration_file_resource_path, for example, appears to be unused, and has incorrect Javadoc. Can this be removed?

[gavinking]

OK, I see that actually a few of the settings we talked about removing have actually been removed.

But some are still hanging around.

[gavinking]

Here's a rough list of some of the ones that look like maybe we don't need them anymore:

• hibernate.jpa.*—do we really need to diverge from the JPA spec in all these things?

It seems all these are staying for now.

• hibernate.ejb.*—EJB!? If these are still useful they should at least be renamed.

These are all gone.

• hibernate.jmx.*—does anyone still use JMX?

It's gone, by the looks

• hibernate.connection.acquisition_mode—this is already deprecated.
• hibernate.connection.release_mode—this is already deprecated.

Both gone.

• hibernate.jdbc.lob_noncontextual_creation—should be a Dialect attribute, it seems to me.

Gone.

• hibernate.jdbc.use_scrollable_resultset—should be a Dialect attribute, it seems to me.
• hibernate.jdbc.use_get_generated_keys—should be a Dialect attribute, it seems to me.
• hibernate.jdbc.use_streams_for_binary—should be a Dialect attribute, it seems to me.

All still there. Are we really keeping these?

• hibernate.jdbc.wrap_result_sets—should be a Dialect attribute, it seems to me.

Gone.

• hibernate.dialect.oracle.prefer_long_raw—I have a philosophical objection to this one: settings don't belong to dialects!

Still there, sadly.

• hibernate.id.new_generator_mappings—if the new ones are better, let's just use them.

Gone.

• hibernate.jpaql_strict_compliance

Seems to be gone.

• hibernate.query.substitutions—we don't just pass through tokens anymore in the "new" HQL, so I don't think this really makes much sense anymore.

Gone!

• hibernate.query.validate_parameters
• hibernate.query.conventional_java_constants

Both still there. I think they should be killed off.

• hibernate.legacy_limit_handler—we don't need legacy things.

Gone!

• hibernate.transaction.factory_class—this is already deprecated.

Not gone. Exists in some sort of strange sort of purgatorial state between life and death. Just kill it off?

• hibernate.native_exception_handling_51_compliance—ugh, the name says it all.

Dead.

[gavinking]

So my hit list is now down to:

• hibernate.query.validate_parameters
• hibernate.query.conventional_java_constants

And these three, which, even if we don't completely remove them internally, we could, IMO, at least undocument:

• hibernate.jdbc.use_scrollable_resultset
• hibernate.jdbc.use_get_generated_keys
• hibernate.jdbc.use_streams_for_binary

[gavinking]

OK, hibernate.cache.provider_configuration_file_resource_path, hibernate.query.conventional_java_constants, and hibernate.query.validate_parameters are on their way to the chopping block.

[gavinking]

This one is gone too: hibernate.jdbc.use_streams_for_binary, as is hibernate.bytecode.enforce_legacy_proxy_classnames, as soon as I merge my latest PR.