Opened 6 years ago

Closed 3 years ago

#14396 closed Cleanup/optimization (duplicate)

Admin generates a query burst on ForeignKey with null = True

Reported by: piro Owned by: Daniel Roseman
Component: contrib.admin Version: 1.2
Severity: Normal Keywords: admin database performance
Cc: semenov@…, Brillgen Developers, erik.wognsen@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When the The admin loads a list of objects to display the change list, it is supposed to use select_related() to fetch ForeignKey fields in the display list.

Because it uses the form select_related() without the filed name, the records are not fetched if the property has null=True. This generates a burst of up of 100 query to display the admin page for such a model.

Attached there is a preliminary patch to fix the issue. It is probably to be improved as it doesn't kick in if the model is configured with list_select_related=True or if select_related() was already called on the query.

Attachments (4)

django-admin-query-burst.patch (565 bytes) - added by piro 6 years ago.
14396_list_select_related.diff (7.6 KB) - added by Daniel Roseman 6 years ago.
14396_list_select_related.patch (7.6 KB) - added by Daniel Roseman 6 years ago.
14396_list_select_related2.patch (6.7 KB) - added by Daniel Roseman 5 years ago.

Download all attachments as: .zip

Change History (14)

Changed 6 years ago by piro

comment:1 Changed 6 years ago by piro

Keywords: admin added; qdmin removed
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

Changed 6 years ago by Daniel Roseman

Changed 6 years ago by Daniel Roseman

comment:2 Changed 6 years ago by Daniel Roseman

Owner: changed from nobody to Daniel Roseman
Status: newassigned
Triage Stage: UnreviewedAccepted

I propose an extended syntax for list_select_related, to allow it to accept a list/tuple of field names, which is passed through as parameters to the query's select_related() call. This allows the developer to specify explicitly which relations to follow, as in a normal query.

Attached patch (which isn't displaying in Trac for some reason, but is a valid svn diff on download) adds this, plus tests and docs. Also fixes an additional issue with list_select_related in that it doesn't follow OneToOneField relations, which are supported in select_related calls.

comment:3 Changed 6 years ago by Ilya Semenov

Cc: semenov@… added

danielr's patch makes perfect sense.

comment:4 Changed 6 years ago by Brillgen Developers

Cc: Brillgen Developers added

comment:5 Changed 6 years ago by Brillgen Developers

An alternative syntax which allows to at least limit the depth of the select related. Simple change would be to make list_select_related=<number> where <number> is the depth parameter for select_related.
Danielr's suggestion is also greate and can be implemented as another field...suggestion: list_select_related_fields

comment:6 Changed 5 years ago by patchhammer

Easy pickings: unset
Patch needs improvement: set
Severity: Normal
Type: Uncategorized

14396_list_select_related.patch fails to apply cleanly on to trunk

Changed 5 years ago by Daniel Roseman

comment:7 Changed 5 years ago by Daniel Roseman

Patch needs improvement: unset

Updated patch which applies against trunk.

comment:8 Changed 5 years ago by Julien Phalip

Type: UncategorizedCleanup/optimization

comment:9 Changed 5 years ago by Erik Wognsen

Cc: erik.wognsen@… added
UI/UX: unset

What is the status of this ticket?

I think the idea of allowing lists/tuples for list_select_related is good and I would love to see this feature included.

comment:10 Changed 3 years ago by Tim Graham

Resolution: duplicate
Status: assignedclosed

Duplicate of #19080 which has been fixed.

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