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/
SequenceHiLoGeneratorNoIncrementTest.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 <st...@hibernate.org> wrote:

> See inline...
>
> On Tue, Apr 18, 2017 at 8:36 PM Gail Badner <gbad...@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.
>
>
_______________________________________________
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Reply via email to