[hibernate-dev] Negative sequence numbers

[Gail Badner]

I see that there are some OptimizerUnitTest tests failing. Please hold off
on looking at the PR until I figure out what's going on.

On Thu, Apr 27, 2017 at 5:46 PM, Gail Badner <gbadner at redhat.com> wrote:

> Hi Steve,
>
> I'm finally getting back to this.
>
> The assertion you suggest fails for SequenceHiLoGeneratorNoIncrementTest
> because the test specifically sets increment_size=0. [1] I'm assuming the
> legacy behavior should be preserved.
>
> Another strange thing -- in NoopOptimer#generate, when incrementSize > 0
> and sequence values are < 1, Hibernate will cycle through sequence values
> until 1 is returned.
>
> Both of these are covered by HHH-5933.
>
> Please take a look at the PR. [3]
>
> Thanks,
> Gail
>
> [1] https://github.com/hibernate/hibernate-orm/blob/master/
> hibernate-core/src/test/java/org/hibernate/id/SequenceHiLoG
> eneratorNoIncrementTest.java#L67
> [2] https://github.com/hibernate/hibernate-orm/blob/master/
> hibernate-core/src/main/java/org/hibernate/id/enhanced/
> NoopOptimizer.java#L41-L45
> [3] https://github.com/hibernate/hibernate-orm/pull/1893
>
> On Wed, Apr 19, 2017 at 6:33 AM, Steve Ebersole <steve at hibernate.org>
> wrote:
>
>> See inline...
>>
>> On Tue, Apr 18, 2017 at 8:36 PM Gail Badner <gbadner at redhat.com> wrote:
>>
>>> Should Hibernate support negative sequence values?
>>>
>>
>> Absolutely.  Hibernate has historically supported decrementing and/or
>> negative values - so yes that should continue to work.
>>
>>
>> If so, is my proposed fix OK?
>>>
>>
>> Define "OK" :)
>>
>> As you said, your testing confirms that your proposed change "works".  So
>> from that simple perspective, yes it is "OK".  Although I think it can, and
>> maybe should, be slightly different - namely we really only need to encode
>> the increment value into the SEQUENCE definition when that increment is
>> something other than 1 (one) since that is the default increment for
>> SEQUENCEs.  So something like:
>>
>> NoopOptimizer {
>>     ...
>>     public boolean applyIncrementSizeToSourceValues() {
>>             assert getIncrementSize() != 0;
>>             return getIncrementSize() != 1;
>>         }
>> }
>>
>> Can increment for no-op be anything other than 1 or -1?  Zero is not
>> valid.  1 and -1 are both valid.  But what about 2 or -2?  I'm not sure
>> that makes sense.  The `!= 1` check handles that case.
>>
>> However, I do have a concern about the design here and moving forward.  I
>> think there is some amount of technical debt here, and I think there was a
>> lot of confusion present even in the original code I wrote here.
>>
>>
>