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 <gbad...@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 <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
