Before doing this, I'd like to get some feedback about these warnings cropping up, just in case there is a valid use case that we're missing.
On Fri, Oct 5, 2018 at 2:46 AM andrea boriero <and...@hibernate.org> wrote: > 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 <gbad...@redhat.com> 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 <gbad...@redhat.com> 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 <gbad...@redhat.com> 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/c1934b72edb4f781520937618b3b750bebb84576#diff-2c7912a47063b9ea81323bf9c6628895L651 >> >> >> > >> _______________________________________________ >> 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
