select_related to work with backward relationships?

[Jari Pennanen]

Hello!

class City(models.Model):
# ...

class Person(models.Model):
# ...
hometown = models.ForeignKey(City, null=True, blank=True)

class Book(models.Model):
# ...
author = models.ForeignKey(Person)

# Can I cache Persons and Books too when getting City? After all there
is backwards relation City.person_set...
City.objects.select_related() # Doesn't work.
City.objects.select_related('person_set') # Doesn't work.

# Is there some reason this doesn't work when using backwards
relations?

[Jari Pennanen]

Found out that it doesn't work.

I think this should be documented that backwards relationships does
*not* work in select_related, since I see no reason why it couldn't
work, it might be tricky to implement, but I think it should be
doable.

If it were documented, someone might get idea to improve django and
make patch for it...

[Alex Gaynor]

There was a discussion of this on Django-users a few weeks ago, the general sentiment is "patches welcome" for now.  The most important thing is to make sure you're only returning N + K results, instead of N * K.

Alex

--
"I disapprove of what you say, but I will defend to the death your right to say it." --Voltaire
"The people's good is the highest law."--Cicero

[Malcolm Tredinnick]

On Mon, 2009-03-23 at 06:50 -0700, Jari Pennanen wrote:
> Found out that it doesn't work.
>
> I think this should be documented that backwards relationships does
> *not* work in select_related, since I see no reason why it couldn't
> work, it might be tricky to implement, but I think it should be
> doable.

Seriously, if we documented everything that wasn't possible with Django,
the documentation would be a couple of million words long. There's
nothing that says select_related() does work with reverse relations and
if people are going to make assumptions, that's their problem.

>
> If it were documented, someone might get idea to improve django and
> make patch for it...

It is documented in that respect. In a couple of different Trac tickets
(since there are multiple issues: select related for reverse one-to-one,
which only isn't in 1.1-beta because I ran out of time to fix the patch,
and select-related for multi-valued relations).

Regards,
Malcolm

[George Vilches]

On Mar 23, 2009, at 8:08 PM, Malcolm Tredinnick wrote:

> It is documented in that respect. In a couple of different Trac
> tickets
> (since there are multiple issues: select related for reverse one-to-
> one,
> which only isn't in 1.1-beta because I ran out of time to fix the
> patch,
> and select-related for multi-valued relations).

I'll happily bring the patch up to date on #7270 for 1.1 if it's just
a matter of you running out of time. You took over the ticket the day
I started looking back into it, so I let it be till I heard more from
you. :)

George

[Malcolm Tredinnick]

No, it's a bit more than that. I spent quite a bit of time looking at
this over the past week (I wanted to get it finished by yesterday
because I know of a bunch of high-volume places where it could be useful
for usre profiles). It seems to be a bit more intrusive than it needs to
be, but that's more of a gut feeling (which I trust) than anything
concrete yet. It also adds four new modules to the tests, when it seems
to only need to update one or two others (okay -- a rare case of a
complaint about "too much testing", but it's a little messy and just
placed a little strangely into the existing source).

Also, looking at it, the SQL it constructs isn't correct (I believe this
is also mentioned in a comment on the ticket somewhere). It isn't
promoting joins to outer joins all the time, which means results will be
omitted.

I do apologise, I realise I let you down here when you did so much work
on this initially. I have nothing to say to that. Will try to do better
next time. It got priorised lower than some things with greater impact.

Regards,
Malcolm

[Jari Pennanen]

On Mar 24, 3:08 am, Malcolm Tredinnick <malc...@pointy-stick.com>
wrote:

> Seriously, if we documented everything that wasn't possible with Django,
> the documentation would be a couple of million words long. There's
> nothing that says select_related() does work with reverse relations and
> if people are going to make assumptions, that's their problem.

My reasoning was: If function is named "select_related", it gets all
related stuff... backward relations are as good as any relation to my
knowledge, it should get them too. To not be confusing it should be
named select_forward_relations() or something such, and if backward
relations are implemented, then create select_backward_relations() and
select_related that combines those two.

I don't know whats common consensus what "select_related" term should
contain, but I just thought it means like all related stuff.