ModelChoiceField and ModelMultipleChoiceField with RadioSelect and CheckboxSelectMultiple widgets produce unnecessary db queries while rendering form (particularily in contrib.admin)

[Althalus]

So we have some admin change form (I was testing it only with my custom admin forms but it obviously can be reproduced anywhere) with ModelChoiceField(widget=RadioSelect) and/or ModelMultipleChoiceField(widget=CheckboxSelectMultiple).

Form fields are rendered in ​admin/includes/fieldset.html template. And calls of {{ field.field }} perform extra queries (coz field reevaluates its queryset). So we get 4 queries instead of 1.
According to django-debug-toolbar we have get 2 extra queries on 7th line in fieldset.html (I suppose in {% if field.field.name %} and {% if field.field.is_hidden %}) and 1 in {% if field.field.help_text %} (except the main query in {{ field.field }} that is only one actually needed).

Here is my stacktrace of what is really happens here.

lib/python3.4/site-packages/django/template/base.py in _resolve_lookup(764)
  current = current[bit]
  
lib/python3.4/site-packages/django/forms/forms.py in __getitem__(526)
  return list(self.__iter__())[idx]
  
lib/python3.4/site-packages/django/forms/forms.py in __iter__(519)
  for subwidget in self.field.widget.subwidgets(self.html_name, self.value(), attrs):
  
lib/python3.4/site-packages/django/forms/widgets.py in subwidgets(727)
  for widget in self.get_renderer(name, value, attrs, choices):
  
lib/python3.4/site-packages/django/forms/widgets.py in get_renderer(735)
  choices = list(chain(self.choices, choices))
  
lib/python3.4/site-packages/django/forms/models.py in __iter__(1074)
  for obj in self.queryset.all():
  
lib/python3.4/site-packages/django/db/models/query.py in __iter__(141)
  self._fetch_all()

Consider relevant forms.py part.

class BoundField(object):
    ...

    def __iter__(self):
        """
        Yields rendered strings that comprise all widgets in this BoundField.

        This really is only useful for RadioSelect widgets, so that you can
        iterate over individual radio buttons in a template.
        """
        id_ = self.field.widget.attrs.get('id') or self.auto_id
        attrs = {'id': id_} if id_ else {}
        for subwidget in self.field.widget.subwidgets(self.html_name, self.value(), attrs):
            yield subwidget

    def __len__(self):
        return len(list(self.__iter__()))

    def __getitem__(self, idx):
        return list(self.__iter__())[idx]

As we know django template system do . lookups in the following order:

• Dictionary lookup
• Attribute lookup
• Method call
• List-index lookup

So when we try to access name, help_text, is_hidden etc. we evaluate list(self.__iter__()) coz __getitem__ method is called first on our BoundField.

I've made a simple patch which must not clash with anything while prevents reevaluation:
​https://github.com/Vincent-Vega/django/commit/88478ef9d93af08e78de5e2755f8da036ab54b6f

One remaining issue is accessing BoundField object by indexes will still hit db but I'm not sure if it important. Iterating is needed and I suppose is used by many people.
But accessing individual radios or checkboxes by index? Anyway we have getitem so somebody could have used it in temapltes. I mean removing this method could solve the problem too but it would be incompatible change.

[Althalus]

Also I think that this change is fully compatible because both in python code and in templates new code raises the same exception as before (TypeError for non-integers). It is just doing it earlier. My github commit is not described properly yet but I'm ready to do it and make pull request is this ticket will be accepted.

[Baptiste Mispelon]

Hi,

While I'm not too convinced by your approach of using isinstance, I agree that it would be nice to be able to fix this.

In any case, a proper patch will require some tests and documentation (a mention in the release notes for 1.8 at this point).

Thanks.

[Althalus]

Sorry for the delay. I've made a new properly described commit in separate branch and corresponding pull request:
​https://github.com/django/django/pull/2816

I suppose further discussion on this issue moves there?

[Althalus]

This patch fixes obviously wrong behaviour which can be faced when using ModelChoiceFields in a common way (including admin).
But still accessing certain widget values from templates by index will trigger qs every time. May be we can use some cache here?

[merb]

made a small review and looks good to me.

[Tim Graham <timograham@…>]

In 5e06fa1469180909c51c07151692412269e51ea3:

Fixed #22745 -- Prevented reevaluation of ModelChoiceField's queryset when accesssing BoundField's attrs.

Thanks Christian Schmitt for review.