Opened 11 years ago

Closed 9 years ago

Last modified 9 years ago

#11604 closed (wontfix)

ChangeList should pass depth=1 to .select_related()

Reported by: rlaager@… Owned by: nobody
Component: contrib.admin Version: 1.0
Severity: Keywords: efficient-admin
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Currently, if a ModelAdmin has a ForeignKey field in list_display, ChangeList will call .select_related() on the QuerySet (with some exceptions). As it does not specify a depth, this leads to joining against up to 5 levels (the arbitrary default limit). If it passed depth=1, this would be avoided. In the case where the related model needs another related model to calculate its unicode(), lazy loading will happen. In the event that matters for performance, there's already a mechanism (list_select_related on the ModelAdmin) to specify that you want a full (well, 5 levels deep) .select_related().

Attachments (1)

11604-changelist-select-related-depth.diff (542 bytes) - added by rlaager@… 11 years ago.

Download all attachments as: .zip

Change History (11)

Changed 11 years ago by rlaager@…

comment:1 Changed 11 years ago by Ramiro Morales

See also #10348, #10742, #9554 and some other related tickets for previous activity in this front.

comment:2 Changed 11 years ago by Alex Gaynor

Triage Stage: UnreviewedDesign decision needed

comment:3 Changed 11 years ago by nick@…

Despite what it says at the start of this thread, select_related making very complicated queries is what is causing mysql to get stuck in state statistics.

This is certainly a mysql bug, but it is triggered by the admin's uncontrolled use of select_related.

Applying the patch above for select_related(depth=1) fixes the mysql lockups

So I'd vote for this patch as it is non-intrusive and fixes our rather difficult to track down mysql lockups.

comment:4 Changed 11 years ago by Thejaswi Puthraya

Component: Uncategorizeddjango.contrib.admin

comment:5 Changed 11 years ago by rlaager@…

Has patch: set

comment:6 Changed 10 years ago by ishirav

Yes! Please please please apply this patch. We've seen ridiculous queries joining 20 or more tables needlessly, and MySQL hanging while trying to run these queries.

As a temporary workaround, here's a way to override the select_related depth in the admin queryset:

class FoobarAdmin(admin.ModelAdmin):
    def queryset(self, request):
        qs = super(FoobarAdmin, self).queryset(request)
        return qs.select_related(depth=1)

comment:7 Changed 10 years ago by anonymous

milestone: 1.3

comment:8 Changed 9 years ago by Julien Phalip

Resolution: wontfix
Status: newclosed

It is quite common to require access to the related models, for example to use as list filters, so it makes sense that Django uses select_related(). However, by arbitrarily using a specific depth of 1 (or whatever number), Django would be taking a guess about what performance trick would work for the majority of sites. A depth of 1 may work well with your use case, but pretty poorly in others. For what it's worth, in 4+ years using Django in many projects I've rarely witnessed such performance issues in the admin, even with MySQL. I'm not saying that your use case doesn't exist, but it doesn't seem like it is common enough to justify changing Django's default behaviour. What's more is that ModelAdmin already offers some hooks to get exactly what you want, as illustrated in ishirav's code snippet above.

comment:9 Changed 9 years ago by Luke Plant

Just to add to what Julian said:

The alternatives (not using selected_related or limiting to 1 etc) could just as easily cause 'silly' performance problems where hundreds of queries are executed instead of one. This would make other people just as unhappy (or the same people unhappy in different ways). What is more, this change would be a regression for lots of people. Whether or not 5 is the best default doesn't matter - it is the number we have picked and you can override the behaviour. See Jacob's comment on #9554.

comment:10 Changed 9 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top