On Fri, Jun 1, 2012 at 7:08 PM, Anssi Kääriäinen
<[email protected]> wrote:
> On Jun 1, 1:43 pm, Russell Keith-Magee <[email protected]>
> wrote:
>> > Just as a bike-shedding thought: Would it be possible to have
>> > frank.events.confirmed.all() as the syntax? I see this a tiny bit
>> > cleaner. On the other hand it isn't explicit you can only use
>> > the .confirmed only directly after the events, and there is the
>> > possibility of clashing with queryset methods. The clash issue can be
>> > solved by just using frank.events('clashing_name').all() instead of
>> > frank.events.clashing_name.all() in the rare cases this is needed.
>>
>> True - but providing an API that avoids the clash in the first place
>> is preferable, IMHO.
>>
>> That said, I'm not *strongly* opposed to .events.confirmed -- it just
>> tastes a bit funny. I'd rather not eat it, but if if everyone else
>> says I should…. :-)
>
> As said, just bike-shedding. Lets go with the call-notation for now.
> It is easy enough to bike-shed this more later. The call syntax is
> clear enough, and it is explicit and should be easy to make that work.
>
>> > How would this work with prefetch_related? Maybe
>> > prefetch_related('events__confirmed')? IMHO this is an important use
>> > case, as there isn't currently any possibility to use different
>> > filtering, ordering etc than the one given by the default related
>> > manager.
>>
>> That's a good point -- I hadn't thought of the consequences on
>> prefetch_related. I'm not wild about overloading the __ notation to
>> add the 'manager selector" interpretation. There are already
>> complications differentiating between aggregates, filter clauses,
>> related fields… I don't think we need to add another to this mix.
>>
>> How about a kwarg to prefetch_related? For example:
>>
>> person.prefetch_related('events', managers={'events': 'confirmed'})
>>
>> or maybe just use kwargs directly:
>>
>> person.prefetch_related(events='confirmed')
>>
>> If you start getting into multiple joins, it's going to get more
>> complex; we might have to require a list of managers:
>>
>> person.prefetch_related(events__things=['confirmed', 'objects'])
>
> This API has one problem: what if you want confirmed events in one
> list, and then non-confirmed events in another?
>
> I created a ticket where you could use R-objects for prefetch related.
> The idea is that you can use any query you like, and fetch the related
> objects to any attribute you like.
>
> In short, the API is this:
> SomeModel.objects.prefetch_related(
> R('events', to_attr='confirmed_events',
> qs=RelModel.confirmed.order_by('foo')),
> )
>
> You can chain those calls:
> Person.objects.prefetch_related(
> R('events', to_attr='confirmed_events',
> qs=Event.confirmed.order_by('foo')),
> R('confirmed_events__locations', to_attr='locations',
> qs=Location.objects.order_by('name')),
> )
> Every Person object fetched fill have a list "confirmed_events", and
> each confirmed event will have an attribute "locations".
I might be missing something in the details here, but are you
proposing here that even though there's only one FK reverse relation
from Event to Person, there would be 2 attributes (events and
confirmed_events) containing lists? That doesn't strike me as a good
idea in itself -- or, at least, something that needs to be followed
through elsewhere in the API. Introducing attributes to a model as a
result of a prefetch_related call isn't something that has an analog
anywhere else in the ORM, AFAICT.
If you drop the 'multiple attributes' thing, then the general problem
you describe could be syntactically covered by:
Person.objects.prefetch_related('events',
events__locations=['confirmed','objects'])
i.e., the events attribute is prefetch-populated with the default
manager, but only those matching the confirmed manager would have a
location pre-poulated. However, this would be a can of worms all of
its own as far as implementation is concerned (probably not
impossible, but sufficiently complex to implement and maintain that I
would be completely comfortable saying "no, you can't do that").
And that's something I'm willing to do more broadly, too.
Prefetch_related is an optimisation. If we can find an elegant way to
integrate non-default manager selection, then sure -- lets do it. But
if we can only introduce non-default manager selection at the cost of
a hideously complex API, then I'm inclined to just call it a
limitation (albeit an unfortunate one) of the feature.
Yours,
Russ Magee %-)
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
