Re: [hibernate-dev] Oracle12cDialect identity support

> Native is a Hibernate-only concept and so we can really choose whatever > we want for native generators. Also native is an outdated concept, > replaced by AUTO and SequenceStyleGenerator. IMO we should never be > choosing IDENTITY for identifier generation unless a user explicitly asks > for it. > Just to make sure I understand, are you saying we should not be choosing IDENTITY by default for *new* dialects only?

> So for the first piece, Oracle12cDialect should > override getNativeIdentifierGeneratorClass to > return SequenceStyleGenerator. The Dialect implementation > of getNativeIdentifierGeneratorClass really assumes databases which support > IDENTITY but not SEQUENCE. > Thanks for clarifying that. I was wondering about that. > Oracle12cDialect is the first case we have had where a database that > historically supported SEQUENCES has added support for IDENTITY and we just > did not account for that properly. In fact, the "proper" solution would be > to go into the base Oracle Dialect(s) and override > getNativeIdentifierGeneratorClass to just always > return SequenceStyleGenerator. But doing that in SequenceStyleGenerator > only is viable too. > Did you mean, "doing that in Oracle12cDialect only is viable too"?

> > As for how to get IDENTITY generation to work consistently with Oracle, > that I cannot say. I asked Oscar (the reporter of HHH-9983) for find out > the way to get IDENTITY generation to work with Oracle 12c *via JDBC*. > "Via JDBC" is the operative part. All of the Oracle documentation and > google hits at that time only discussed using IDENTITY via command told > (SQLPlus, etc). According to his findings we seem to have to correct. > I believe Oracle12cDialect identity support is working properly in master now. IIUC you are saying that the problem is porting that from 5.1 (master) to > 5.0? Due to an SPI change? What exactly is the SPI that changed? We did > make lots of changes to "IdentityColumnSupport" (adding that as a > first-class contract), but that is really just cosmetic grouping of stuff > already available on Dialect. So what change specifically stops you from > porting those changes to 5.0? > I've looked over the changes again and I think the main problem is that the refactoring done for HHH-10084 [1] will break custom dialects that override the Dialect methods that are deprecated for 5.1. The deprecated Dialect methods are no longer used; Hibernate obtains the identity support information from Dialect#getIdentityColumnSupport only.

I've created a test to illustrate: [2]. Custom dialects will need to be modified to override getIdentityColumnSupport() to work. Would this type of breaking change be OK for 5.1 (I'm guessing no...). IMO, it is not OK to introduce this breaking change in 5.0.

HHH-10084 ([1]) changed the deprecated Dialect methods to delegate to the IdentityColumnSupport. It also extracted the overridden methods from Dialect subclasses into dialect-specific IdentityColumnSupport classes. If I backport HHH-9983 to 5.0, I think I would only backport part of HHH-10084. I would change IdentityColumnSupportImpl methods to delegate to the deprecated Dialect methods; I would not extract the overridden methods into dialect-specific IdentityColumnSupport classes. That way, custom dialects would not be broken.