#33208: Allow globally defining custom (i.e. with select_related) querysets for
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.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/059.8b388334a01cadf08affb2db408dfc2b%40djangoproject.com.

Reply via email to