#27001 closed Bug (fixed)
Regression in query counts using RadioSelect with ModelChoiceField
Reported by: | Alex Hill | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | 1.10 |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Before 1.9, I had on my list of things to look at the fact that a standard ModelChoiceField with a RadioSelect takes two queries. Now looking at this in 1.10, the number of queries has ballooned (to 11 queries in my simple test case).
I've not got far in looking for a solution, but this test passes in 1.9 and fails in 1.10 and master with AssertionError: 11 != 2 : 11 queries executed, 2 expected
:
def test_radioselect_num_queries(self): class CategoriesForm(forms.Form): categories = forms.ModelChoiceField( queryset=Category.objects.all(), widget=forms.RadioSelect ) template = Template('{% for w in form.categories %}{{ w }}{% endfor %}') with self.assertNumQueries(2): template.render(Context({'form': CategoriesForm()}))
Change History (8)
comment:1 by , 8 years ago
Component: | Uncategorized → Forms |
---|
comment:2 by , 8 years ago
comment:3 by , 8 years ago
Created a pull request with a fix.
ChoiceFieldRenderer
doesn't have __iter__
defined, so Python iterates over it by calling __getitem__
with an increasing index until an exception is raised. ChoiceFieldRenderer.__getitem__
calls list
on itself, which turns iteration into an n2 operation. When the choices are backed by a queryset as in ModelChoiceField, that means lots of redundant database queries.
Fixed by adding an __iter__
method to ChoiceFieldRenderer
and changing __getitem__
to use it, so that indexing still works.
comment:4 by , 8 years ago
Has patch: | set |
---|---|
Needs tests: | set |
Type: | Uncategorized → Bug |
comment:6 by , 8 years ago
Needs tests: | unset |
---|---|
Severity: | Normal → Release blocker |
As this is a regression in 1.10 I suppose this should be backported.
Branch with failing test here: https://github.com/django/django/compare/master...AlexHill:ticket_27001