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
