[hibernate-dev] multiLoad support

[Sanne Grinovero]

On 25 July 2016 at 19:55, Steve Ebersole <steve at hibernate.org> wrote:
> See inline...
>
>
> On Mon, Jul 25, 2016 at 1:06 PM Sanne Grinovero <sanne at hibernate.org> wrote:
>>
>> some comments inline:
>>
>> On 25 July 2016 at 18:27, Steve Ebersole <steve at 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.