[hibernate-dev] Oracle12cDialect identity support

[Gail Badner]

I was mistaken when I said that an IDENTITY ID will be created when using
@GeneratedValue or @GeneratedValue(strategy= GenerationType.AUTO). With
hibernate.id.new_generator_mappings set to true by default in 5.0 and 5.1,
a SequenceStyleGenerator will be used instead.

Currently, the default for "native" ID generator mapped in hbm.xml uses an
IDENTITY if the dialect supports an IDENTITY. This will be overridden in
Oracle12cDialect as we've been discussing.

On Tue, Jan 5, 2016 at 9:46 AM, Steve Ebersole <steve at hibernate.org> wrote:

>
>
> On Tue, Jan 5, 2016 at 12:42 AM Gail Badner <gbadner at redhat.com> wrote:
>
>>
>>
>>>  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?
>>
>
> Well in an ideal world where we can time-travel and retroactively apply
> our gained 20/20 hindsight I would change that everywhere.  However we
> don't have that luxury :)  So, yes, I would do that just for new Dialects.
>
> So moving forward, any new Dialect should never chose {"native" ->
> IDENTITY} and should never chose {AUTO -> IDENTITY}.
>
>
>
>>
>>
>>> 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"?
>>
>
> Yes.  Considering I typed that on my phone I am shocked that was my only
> typo :)
>
>
>>
>>>
>>> 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.
>>
>
> Then the deprecated Dialect methods ought to be removed.  Leaving them
> only causes confusion
>
>
>>
>> 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.
>>
>
> I am not sure what you are asking when you say "Would this type of
> breaking change be OK for 5.1"
>
>
>
>>
>> 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.
>>
>
> This kind of multi-dispatch makes my head hurt.  And I bet users don't
> find it any less confusing...
>
>