Since I haven't heard anything back, I've gone ahead and updated my PR [1] to ignore queued operations when merging a detached collection. This restores the functionality that was in place prior to HHH-5855 (which caused HHH-11209). In addition I've added warnings for the following situations: * a detached collection is being merged that has queued operations; * a collection with queued operations is attached to a session; * a collection with queued operations is detached from a session. I've also added a test case that shows that Session#merge and Session#saveOrUpdate have different results when called on an entity with a collection with queued operations. When Session#merge is called, the queued operations are ignored. When Session#saveOrUpdate is called, the queued operations are executed on flush. I don't like this inconsistency, but I'm not sure what to do about it. I don't think it's a good idea to have collections floating around with queued operations. As far as the application knows, it is just an uninitialized collection. I can see that the entity could find its way into a new session, with surprising results. IIUC, an application probably shouldn't detach an entity with a collection in this state (having queued operations), so I think it makes sense that a warning be logged when this happens. On the other hand, the warning will also be logged if a transaction involving updates to a collection with queued operations fails. I would really appreciate some feedback on this, since HHH-11209 is a bad regression that really should be fixed. Thanks, Gail [1] https://github.com/hibernate/hibernate-orm/pull/2460 On Fri, Sep 28, 2018 at 6:27 PM Gail Badner <gbadner(a)redhat.com&gt; wrote: > HHH-11209 involves a bug merging a detached entity with an uninitialized > collection that has queued operations. > > After looking into this issue I found a bug > where AbstractPersistentCollection#operationQueue was not being cleared > after a commit, if there was no cascade mapped for the collection. > > I've fixed that in a PR. [1] > > After that fix, the detached entity in the test case attached to > HHH-11209 has an uninitialized entity without any queued operations after > commit, which can be merged successfully in a new session/transaction. > > There is still a problem when Hibernate tries to merge a detached entity > has an uninitialized collection with queued operations though. My PR throws > UnsupportedOperationException in this case, and there is a test case that > reproduces it. > > I've been working on a fix to add support for this, but I have my doubts > that it really should be supported. I see that prior to fixing HHH-5855 > (which caused HHH-11209), Hibernate simply ignored the queued operations in > the detached collection. [2]. > > The fix for HHH-5855 properly dealt with merging a managed collection > that was uninitialized with queued operations. It introduced the bug where > a NullPointerException would get thrown if that collection was detached. > > If we want to support merging the queued operations, then I would > consider that an improvement. Would this be a worthwhile improvement? I'm > guessing not, but I wanted to get some opinions. > > For now, I see a couple of ways to deal with this so that HHH-11209 can > be wrapped up: > 1) ignore queued operations in a detached collection when merging, as was > done before HHH-5855 was fixed. > 2) clear the queued operations when the collection is detached. > > Comments? > > Thanks, > Gail > > [1] https://github.com/hibernate/hibernate-orm/pull/2460 > [2] > https://github.com/hibernate/hibernate-orm/commit/c1934b72edb4f7815209376... >

I would say to log a warning when the entity is detached and then clear the queued operations. On Fri, 5 Oct 2018 at 07:25, Gail Badner <gbadner(a)redhat.com&gt; wrote: > Guillaume has reviewed my PR and I've pushed the fix to master. > > I still think the inconsistency should be resolved. I have no plans to do > any more on this without any feedback. > > Regards, > Gail > > On Tue, Oct 2, 2018 at 6:16 PM Gail Badner <gbadner(a)redhat.com&gt; wrote: > > > Since I haven't heard anything back, I've gone ahead and updated my PR > [1] > > to ignore queued operations when merging a detached collection. This > restores > > the functionality that was in place prior to HHH-5855 (which caused > > HHH-11209). > > > > In addition I've added warnings for the following situations: > > * a detached collection is being merged that has queued operations; > > * a collection with queued operations is attached to a session; > > * a collection with queued operations is detached from a session. > > > > I've also added a test case that shows that Session#merge and > > Session#saveOrUpdate have different results when called on an entity > with a > > collection with queued operations. When Session#merge is called, the > queued > > operations are ignored. When Session#saveOrUpdate is called, the queued > > operations are executed on flush. > > > > I don't like this inconsistency, but I'm not sure what to do about it. I > > don't think it's a good idea to have collections floating around with > > queued operations. As far as the application knows, it is just an > > uninitialized collection. I can see that the entity could find its way > into > > a new session, with surprising results. > > > > IIUC, an application probably shouldn't detach an entity with a > collection > > in this state (having queued operations), so I think it makes sense > that a > > warning be logged when this happens. On the other hand, the warning will > > also be logged if a transaction involving updates to a collection with > > queued operations fails. > > > > I would really appreciate some feedback on this, since HHH-11209 is a > bad > > regression that really should be fixed. > > > > Thanks, > > Gail > > > > [1] https://github.com/hibernate/hibernate-orm/pull/2460 > > > > On Fri, Sep 28, 2018 at 6:27 PM Gail Badner <gbadner(a)redhat.com&gt; wrote: > > > >> HHH-11209 involves a bug merging a detached entity with an > uninitialized > >> collection that has queued operations. > >> > >> After looking into this issue I found a bug > >> where AbstractPersistentCollection#operationQueue was not being cleared > >> after a commit, if there was no cascade mapped for the collection. > >> > >> I've fixed that in a PR. [1] > >> > >> After that fix, the detached entity in the test case attached to > >> HHH-11209 has an uninitialized entity without any queued operations > after > >> commit, which can be merged successfully in a new session/transaction. > >> > >> There is still a problem when Hibernate tries to merge a detached > entity > >> has an uninitialized collection with queued operations though. My PR > throws > >> UnsupportedOperationException in this case, and there is a test case > that > >> reproduces it. > >> > >> I've been working on a fix to add support for this, but I have my > doubts > >> that it really should be supported. I see that prior to fixing HHH-5855 > >> (which caused HHH-11209), Hibernate simply ignored the queued > operations in > >> the detached collection. [2]. > >> > >> The fix for HHH-5855 properly dealt with merging a managed collection > >> that was uninitialized with queued operations. It introduced the bug > where > >> a NullPointerException would get thrown if that collection was > detached. > >> > >> If we want to support merging the queued operations, then I would > >> consider that an improvement. Would this be a worthwhile improvement? > I'm > >> guessing not, but I wanted to get some opinions. > >> > >> For now, I see a couple of ways to deal with this so that HHH-11209 can > >> be wrapped up: > >> 1) ignore queued operations in a detached collection when merging, as > was > >> done before HHH-5855 was fixed. > >> 2) clear the queued operations when the collection is detached. > >> > >> Comments? > >> > >> Thanks, > >> Gail > >> > >> [1] https://github.com/hibernate/hibernate-orm/pull/2460 > >> [2] > >> > https://github.com/hibernate/hibernate-orm/commit/c1934b72edb4f7815209376... > >> > > > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev >