Thanks Gail, I'll try it on my fork after I push the migrated User Guide.
Vlad On Thu, Jan 7, 2016 at 11:36 AM, Gail Badner <gbad...@redhat.com> wrote: > I've created a pull request for HHH-5855. [1] > > I ran into several bugs related to delayed operations, so this is a > work-in-progess. I wanted to create a pull request of what I have so far. I > will revisit it when I return to work 1/14/16. I plan to address the other > bugs separately. > > Regards, > Gail > > [1] https://github.com/hibernate/hibernate-orm/pull/1212 > > On Wed, Dec 23, 2015 at 12:05 PM, Vlad Mihalcea <mihalcea.v...@gmail.com> > wrote: > >> Hi Gail, >> >> I'm glad there is a development plan on this one . I've been following >> this issue for a couple of years and seen some recent comments which >> reminded me of it. >> Someone asked me on my blog if we can get it fixed, as it's causing >> problems when people are trying to merge back detached entities. >> >> If I can help you with anything, just let me know. Now that I've been >> also digging into it, I can at least assist you and test your PR on my fork >> too. >> >> Vlad >> >> On Wed, Dec 23, 2015 at 9:25 PM, Gail Badner <gbad...@redhat.com> wrote: >> >>> We really don't want to initialize a List when merging. Instead, we want >>> to do the same sort of replace on the values stored in the DelayedOperation >>> objects. That way, the collection will be initialized only when necessary. >>> The DelayedOperations are executed on flush. I'll should get a pull request >>> out for this today or tomorrow. >>> >>> Vlad, If you are interested in digging into an issue that is assigned to >>> me, I'm happy to tell you my status and share what I know about it. I would >>> certainly welcome your help. >>> >>> Thanks, >>> Gail >>> >>> On Wed, Dec 23, 2015 at 10:51 AM, Gail Badner <gbad...@redhat.com> >>> wrote: >>> >>>> Hi Vlad, >>>> >>>> I've spend quite a bit of time on this one and already have a fix. I >>>> just have some tests to add to confirm. I will look into what you suggest, >>>> but please check with me first if you see that an issue is assigned to me. >>>> >>>> Thanks, >>>> Gail >>>> >>>> On Wed, Dec 23, 2015 at 4:13 AM, Vlad Mihalcea <mihalcea.v...@gmail.com >>>> > wrote: >>>> >>>>> Hi guys >>>>> >>>>> I decided to spend some time to investigate the infamous HHH-5855 ( >>>>> https://hibernate.atlassian.net/browse/HHH-5855 ) bug and this is my >>>>> report. >>>>> >>>>> One of the first thing that I noticed is that Sets are fine, while >>>>> this bug >>>>> only replicates with bidirectional Bags. >>>>> >>>>> After some hours of debugging and logging (since debugging triggers >>>>> collection initialization), I found the culprit. >>>>> >>>>> In the org.hibernate.type.TypeHelper.replace(Object[] original, >>>>> Object[] >>>>> target, Type[] types, SessionImplementor session, Object owner, Map >>>>> copyCache) method, when copying the cached entity state (which >>>>> contains the >>>>> newly added child entity along with its identifier) onto the original >>>>> collection: >>>>> >>>>> copied[i] = types[i].replace( original[i], target[i], session, owner, >>>>> copyCache ); >>>>> >>>>> inside the org.hibernate.type.CollectionType.replace(Object[] original, >>>>> Object[] target, Type[] types, SessionImplementor session, Object >>>>> owner, >>>>> Map copyCache) method there is this check: >>>>> >>>>> if ( !Hibernate.isInitialized( original ) ) { >>>>> return target; >>>>> } >>>>> >>>>> >>>>> For Sets, the collection is always initialized because of this line >>>>> inside >>>>> the add() method of the >>>>> org.hibernate.collection.internal.PersistentSet: >>>>> >>>>> final Boolean exists = isOperationQueueEnabled() ? >>>>> readElementExistence( value ) : null; >>>>> >>>>> Because of the the readElementExistence( value ) call the Set is always >>>>> initialized and upon triggering the flush, the newly added Entity being >>>>> already managed it will be left alone. >>>>> >>>>> For PersistentList the aforementioned check is false and the replace >>>>> never >>>>> occurs, hence the transient entity lingers in the persistence context >>>>> and >>>>> the flush will trigger another insert, so we get duplicates. >>>>> >>>>> To make the Bag behave like the Set, all we need to do is to change >>>>> the add >>>>> method like this: >>>>> >>>>> public boolean add(Object object) { >>>>> initialize(false); >>>>> if ( !isOperationQueueEnabled() ) { >>>>> write(); >>>>> return bag.add( object ); >>>>> } >>>>> else { >>>>> queueOperation( new SimpleAdd( object ) ); >>>>> return true; >>>>> } >>>>> } >>>>> >>>>> but then four tests will fail: >>>>> >>>>> org.hibernate.test.legacy.MasterDetailTest > testQueuedBagAdds FAILED >>>>> java.lang.AssertionError at MasterDetailTest.java:1068 >>>>> >>>>> org.hibernate.test.unionsubclass.UnionSubclassTest > testUnionSubclass >>>>> FAILED >>>>> org.hibernate.ObjectDeletedException at UnionSubclassTest.java:364 >>>>> >>>>> org.hibernate.test.version.VersionTest > testCollectionNoVersion FAILED >>>>> java.lang.AssertionError at VersionTest.java:118 >>>>> >>>>> org.hibernate.test.version.VersionTest > testCollectionVersion FAILED >>>>> java.lang.AssertionError at VersionTest.java:79 >>>>> >>>>> 3 of them fail because we expect the List not to be initialized and >>>>> the UnionSubclassTest fails >>>>> for a doubtful reason (we attempt to delete an entity which is still >>>>> referenced). >>>>> >>>>> Basically, such a change will finally fix this issue and the Sets and >>>>> Lists >>>>> will behave consistently. Since you know the reasons behind the >>>>> difference >>>>> in how Sets and Lists are initialized, we need to discuss if this >>>>> change is >>>>> appropriate or we should address this issue differently. >>>>> >>>>> I have a branch on my fork with a test that replicates this issue and >>>>> that >>>>> the proposed change manages to fix it: >>>>> >>>>> https://github.com/vladmihalcea/hibernate-orm/tree/feature/hhh5855 >>>>> >>>>> Let me know what you think and let's discuss it further. >>>>> >>>>> Vlad >>>>> _______________________________________________ >>>>> hibernate-dev mailing list >>>>> hibernate-dev@lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>>> >>>> >>>> >>> >> > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
