On Tue, Jan 5, 2016 at 12:42 AM Gail Badner <gbad...@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...
_______________________________________________
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
