So what I ended up doing is: 1. Add new MultiIdentifierLoadAccess#enableReturnOfDeletedEntities option (default == false) 2. Add new MultiIdentifierLoadAccess#enableOrderedReturn option (default == true) 3. Added Javadoc clarification to MultiIdentifierLoadAccess#multiLoad to see the Javadocs for the above 2 options.
I am not super stoked about the way the ordering happens, at the moment. For now I essentially: 1. Perform X number of batch loads 2. After all the batch loads, come back and build the result List based on the incoming id positions. I did it this way to also handle de-duplication which is a related, reported bug. On Mon, Jul 25, 2016 at 3:01 PM Sanne Grinovero <sa...@hibernate.org> wrote: > On 25 July 2016 at 19:55, Steve Ebersole <st...@hibernate.org> wrote: > > See inline... > > > > > > On Mon, Jul 25, 2016 at 1:06 PM Sanne Grinovero <sa...@hibernate.org> > wrote: > >> > >> some comments inline: > >> > >> On 25 July 2016 at 18:27, Steve Ebersole <st...@hibernate.org> wrote: > >> > I wanted to start a consolidated discussion about multi-load support. > >> > This > >> > relates to a few Jiras, questioning a few different aspects of its > >> > current > >> > behavior: > >> > > >> > https://hibernate.atlassian.net/browse/HHH-10984 > >> > https://hibernate.atlassian.net/browse/HHH-10617 > >> > > >> > Basically this comes down to the following questions/requests... > >> > > >> > > >> > ## Handling locally deleted entities > >> > > >> > First (from HHH-10984) is the idea that locally deleted entities are > >> > currently returned from multi-load calls. The background here is that > >> > multi-load support was designed based on > >> > IdentifierLoadAccess#getReference > >> > (Session#load). So its outcome follows what is done for #getReference > >> > in > >> > terms of behavior and results. Now one of the behaviors of > >> > #getReference > >> > that differs from IdentifierLoadAccess#load (Session#get) is how > locally > >> > deleted entities are treated: #getReference will return them, whereas > >> > #load > >> > will not. > >> > > >> > So as I see it we have 3 options: > >> > > >> > 1. Continue to expose just the one form on > MultiIdentifierLoadAccess > >> > and > >> > either have this filter out the locally deleted objects, or add a > new > >> > option to specify whether locally deleted objects ought to be > >> > returned in > >> > the results. > >> > 2. Expose 2 distinct forms on MultiIdentifierLoadAccess: > >> > 1. Plural form of #getReference (current behavior, more or less) > >> > 2. Plural form of #load > >> > 3. Expose 3 distinct forms on MultiIdentifierLoadAccess: > >> > 1. Plural form of #getReference > >> > 2. Plural form of #load > >> > 3. "Hybrid" form > >> > >> Regarding locally deleted entities, I understand the underlying logic > >> but as a user I think I'd be surprised for it to return them. > >> Is there a good use case to return these? > > > > > > I think its just more efficient in many cases. Meaning, in some cases > > filtering out the deleted ones would mean a performance overhead mainly > > because we'd have to ascertain whether they are deleted or not. Which > means > > going to check the related EntityEntry, which has an overhead going the > > reference to via creation of an EntityKey and then the > "EntityEntryContext > > lookup". So I think in the case of #getReference (allow proxy creation) > it > > might be more of a significant overhead . > > > > If we go with the proposed "2 distinct forms" approach, the current > behavior > > would align with "plural form of #getReference (current behavior, more or > > less)". We'd just offer another solution that aligns with "plural form > of > > #load". However, see the caveat wrt nulls as discussed below... > > > > > >> > Much of the discussion on HHH-10984 revolves around the (poorly based, > >> > imo) > >> > assumption that because a List/array of ids is passed in and because a > >> > List > >> > is returned that we ought to return the results in the order indicated > >> > by > >> > the incoming List/array of ids. > >> > >> As discussed on chat, in lack of any explicit documentation I also > >> would have expected the order to be preserved, but stating otherwise > >> in javadoc seems good enough to me. > >> > >> +1 to change the parameter type from List to Collection as you > >> suggested; the return should also be changed from List so to not > >> suggest any ordering guarantee but since that's breaking we'll do it > >> later. > >> > >> > While I do not agree with the assumption there, I can see that that > >> > behavior might sometimes be beneficial. Is this something we want to > >> > support? There is certainly a performance overhead, and so I think we > >> > definitely do not want to support it by default. But do we want to > >> > expose > >> > an option to allow users to request this? > >> > >> Yes I think there would be performance benefits to allow the consumer > >> to push the sorting guarantee to ORM: > >> in worst case, ORM will do it in the same inefficient way that the > >> consumer would need to, but there might be cases in which ORM can do > >> something smarter. > >> > >> In Hibernate Search we have two different situations in which we'd > >> load by "byMultipleIds"; > >> in one case we'll need strict sorting, in the other we actually don't > >> care at all for the order.. so yes we'd benefit from an option. > >> > >> Considering that the current API takes a List as parameter, and > >> returns a List, and that we seem to agree that there's value in > >> maintaining ordering.. maybe we should keep this API as is and make it > >> always ordered? > >> One could add the variation based on Collections as the alternative > >> API to use when ordering is not needed, by adding it as a new API we'd > >> not be breaking anything and still providing both options. > > > > > > I'd instead opt then to keep the current API taking and returning Lists, > but > > with a sorting option (#enabledOrderedResults) that controls whether we > sort > > or not. > > > > Keep in mind too that this Jira also then asserts (and I'd have to agree > > with, if we accept ordering) that nulls should be pushed into the > resulting > > List at the correct spot. So the List consumer would have to handle for > > null elements, which would be a change in behavior as well. Sanne - > could > > Search, as the List consumer, deal with null elements? > > Yes, not a problem. > > > > > That is why I would allow to control this via the switch. I could agree > > either way about the default here. > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev