#11604 closed (wontfix)
ChangeList should pass depth=1 to .select_related()
Reported by: | 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 |
Description
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)
Change History (11)
by , 15 years ago
Attachment: | 11604-changelist-select-related-depth.diff added |
---|
comment:1 by , 15 years ago
comment:2 by , 15 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:3 by , 15 years ago
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.
http://groups.google.com/group/django-developers/browse_thread/thread/ad87d824d4910284
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 by , 15 years ago
Component: | Uncategorized → django.contrib.admin |
---|
comment:5 by , 15 years ago
Has patch: | set |
---|
comment:6 by , 14 years ago
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 by , 14 years ago
milestone: | → 1.3 |
---|
comment:8 by , 14 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
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 by , 14 years ago
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.
See also #10348, #10742, #9554 and some other related tickets for previous activity in this front.