Opened 12 years ago

Closed 7 years ago

#18729 closed Bug (fixed)

Admin changelist view defaults to `DISTINCT`, unusable on reasonably sized databases

Reported by: Henrique C. Alves Owned by: AlexMalek
Component: contrib.admin Version: dev
Severity: Normal Keywords: admin sql distinct slow performance
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The default behavior on the admin is not usable for reasonably sized tables (a couple thousand tuples is enough for >1s queries).

Right now, it always includes a DISTINCT clause if you add a M2M field on list_filter, even if you're not applying any filters. Use of DISTINCT is known to be painfully slow, and even more so without a WHERE clause, because it forces SQL databases into a full table scan. The behavior is not easily overridden because the logic is scattered on a couple ChangeList methods (https://github.com/django/django/blob/master/django/contrib/admin/views/main.py#L299).

The only solution right now is not including a M2M field on list_filter and limiting functionality. It would be better if that behavior could be overridden (a use_distinct parameter on get_query_set?) or, optimally, moving this logic somewhere else. Ticket #15819 already includes a patch for that.

Change History (9)

comment:1 by Travis Swicegood, 12 years ago

Resolution: duplicate
Status: newclosed
Type: UncategorizedBug

Resolving this as a dupe #15819. Looks like that issue has all of the same points here and fixes this issue.

Feel free to re-open if I missed something in this ticket that's different than #15819.

comment:2 by AlexMalek, 10 years ago

Resolution: duplicate
Status: closednew

I think it is a different issue.
It is possible to alter the check that examines list_filter to determine if distinct() is needed based on if the field appears in the query/request but this does not address the deeper issues in #15819
For example in Django 1.5.2 I think the following changes solves the issue:

django/contrib/admin/util.py:
20c20
< def lookup_needs_distinct(opts, lookup_path, query_dict=None):
---
> def lookup_needs_distinct(opts, lookup_path):
29,31c29
<          not field.field.unique) and
<        (query_dict is None or (field_name in ",".join(query_dict.keys())))
<       ):
---
>          not field.field.unique)):

django/contrib/admin/views/main.py:
128c128
<                                                           field_path, lookup_params))
---
>                                                           field_path))
Last edited 9 years ago by Tim Graham (previous) (diff)

comment:3 by AlexMalek, 10 years ago

Owner: changed from nobody to AlexMalek
Status: newassigned

comment:4 by AlexMalek, 10 years ago

Has patch: set

comment:5 by Tim Graham, 10 years ago

Triage Stage: UnreviewedAccepted

comment:6 by Nick Sandford, 10 years ago

Triage Stage: AcceptedReady for checkin

comment:7 by Tim Graham, 10 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

I left comments for improvement on the PR. Please uncheck "Patch needs improvement" when you update it, thanks.

comment:8 by Tim Graham, 7 years ago

Patch needs improvement: unset

comment:9 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In 8f76673:

Fixed #18729 -- Made admin changelist not use distinct() if a list_filter doesn't require it.

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