[Django] #33208: Allow globally defining custom (i.e. with select_related) querysets for ModelChoiceFields/ForeignKeys
Django
ModelChoiceFields/ForeignKeys
---------------------------------------------+------------------------
Reporter: Matthijs Kooijman | Owner: nobody
Type: New feature | Status: new
Component: Uncategorized | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
TL;DR: In cases where you have models with a foreignkey, a
ModelChoiceField (e.g. using ModelAdmin), and a `__str__` method that uses
a related field, a lot of queries might be duplicated. There does not
currently seem to be a clean way to add select_related in the right place
to fix this cleanly.
e.g. consider something like this:
{{{
from django.db import models
from django.contrib import admin
class Author(models.Model):
name = models.CharField(max_length=255)
class Book(models.Model):
name = models.CharField(max_length=255)
author = models.ForeignKey(Author, on_delete=models.CASCADE)
def __str__(self):
return "{} by {}".format(self.name, self.author.name)
class BookReview(models.Model):
book = models.ForeignKey(Book, on_delete=models.CASCADE)
rating = models.IntegerField()
@admin.register(BookReview)
class BookReviewAdmin(admin.ModelAdmin):
pass
}}}
When showing the admin change view for a Review, I get a ton of queries
when the ModelChoiceField tries to render all options for the `book`
field. For each Book in the database, it calls `__str__`, which does a
single query to retrieve `self.author.name`.
The obvious fix for this would be to ensure that the ModelChoiceField
queryset uses `select_related` to prefetch all data. I've found that this
is possible by defining `formfield_for_foreignkey` on (in this case)
`BookReviewAdmin`, e.g. something like:
{{{
def formfield_for_foreignkey(self, db_field, request, **kwargs):
if db_field.name == "book":
kwargs['queryset'] =
Book.objects.all().select_related('author')
return super().formfield_for_foreignkey(db_field, request,
**kwargs)
}}}
This particular implementation has the downside that it bypasses the
default queryset processing of ModelAdmin (currently only applying the
ordering for the ModelAdmin registered for Book, if any). It would be
nicer to override `ModelAdmin.get_field_queryset`, but that does not seem
to be a public API.
In any case, the bigger problem with this approach, is that it has to be
applied for *every* ModelAdmin that has a ForeignKey to Book, which leads
to duplication. Ideally, and that is the point of this issue, it would be
possible to define this select_related business in the Book model or in
BookAdmin, so it is used automatically.
I'm not quite sure *how* this would would work and where to put it,
though. Conceptually, the select_related is needed because of how
`__str__` is defined on the model, so defining this in the model would
make sense. There is already the concept of different managers that can
prepare querysets for different usecases, but I'm not sure if this case
would warrant a new manager name. If it would, then when would this
manager be used?
Conceptually I guess it's not even linked to the
ForeignKey/ModelChoiceField itself, but it would be the "manager to use
when you want to call __str__". Since using ModelChoiceField implies
calling `__str__`, using this custom manager for ModelChoiceField (and/or
from ForeignKey.dbfield) would also solve the problem, but might apply the
select_related also in cases where it is not really needed (expanding this
further, you could of course just add the select_related to the default
manager, which would work, but end up selecting too much in a lot of
cases).
One pragmatic alternative could be to try and fix this at the admin level
instead, where in this case BookAdmin could define something like
`get_queryset_for_references()` or so, and have the
`BookReviewAdmin.get_field_queryset()` use that (similar to how it already
applies the ordering from the related field admin). Again, not sure how to
exactly define this in an elegant way...
--
Ticket URL: <https://code.djangoproject.com/ticket/33208>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
ModelChoiceFields/ForeignKeys
Reporter: Matthijs Kooijman | Owner: nobody
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: new => closed
* resolution: => wontfix
* component: Uncategorized => contrib.admin
Comment:
Thanks for the report, however I don't think a new hook is necessary you
can use
[https://docs.djangoproject.com/en/stable/ref/contrib/admin/#django.contrib.admin.ModelAdmin.autocomplete_fields
ModelAdmin.autocomplete_fields] to avoid selecting all related instances
to display in the dropdown, as documented:
> ''"By default, the admin uses a select-box interface (<select>) for
those fields. Sometimes you don’t want to incur the overhead of
**selecting all the related instances** to display in the dropdown. The
Select2 input looks similar to the default input but comes with a search
feature that loads the options asynchronously. This is faster and more
user-friendly if the related model has many instances."''
For example:
{{{#!python
@admin.register(Book)
class BookAdmin(admin.ModelAdmin):
search_fields = ['name', 'author__name']
# Register your models here.
@admin.register(BookReview)
class BookReviewAdmin(admin.ModelAdmin):
autocomplete_fields = ['book']
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33208#comment:1>
Django
ModelChoiceFields/ForeignKeys
Reporter: Matthijs Kooijman | Owner: nobody
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution: wontfix
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Also, there are both [https://www.youtube.com/watch?v=e52S1SjuUeM
established ways of optimising this] and
[https://docs.djangoproject.com/en/3.2/ref/forms/fields/#iterating-
relationship-choices customising choice generation] (but these are topics
for [https://forum.djangoproject.com/c/users/6/l/latest the Using category
on the Forum] rather than here 🙂)
--
Ticket URL: <https://code.djangoproject.com/ticket/33208#comment:2>
Django
ModelChoiceFields/ForeignKeys
Reporter: Matthijs Kooijman | Owner: nobody
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution: wontfix
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Matthijs Kooijman):
Thanks for your responses.
> Thanks for the report, however I don't think a new hook is necessary
you can use ModelAdmin.autocomplete_fields to avoid selecting all related
instances to display in the dropdown, as documented:
I would not really consider that a solution of this particular problem. It
significantly changes UI (which might not be what I want here) which
prevents this prefetching from being needed, but IMHO does not really
solve the problem itself. But yeah, it does prevent the problem from being
a problem, that's true :-)
> Also, there are both established ways of optimising this and
customising choice generation (but these are topics for the Using
category on the Forum rather than here 🙂)
AFAICS those all work on the form end of things, which would be similar to
(maybe even more verbose than) the `formfield_for_foreignkey()`-based
approach I suggested above. However, the point of this ticket was to talk
about solving this more centrally on the original model side of things.
Having said that, I can see that fixing this is not easy and can be
considered out of scope, so closing as wontfix is perfectly acceptable.
Also, thanks for the links, I've learned a few new things from them :-)
--
Ticket URL: <https://code.djangoproject.com/ticket/33208#comment:3>