Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#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 Changed 3 years ago by Alex Hill

Component: UncategorizedForms

comment:3 Changed 3 years ago by Alex Hill

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.

Last edited 3 years ago by Alex Hill (previous) (diff)

comment:4 Changed 3 years ago by Alex Hill

Has patch: set
Needs tests: set
Type: UncategorizedBug

comment:5 Changed 3 years ago by Simon Charette

Triage Stage: UnreviewedAccepted

comment:6 Changed 3 years ago by Simon Charette

Needs tests: unset
Severity: NormalRelease blocker

As this is a regression in 1.10 I suppose this should be backported.

comment:7 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In c5ebfda:

Fixed #27001 -- Fixed a query count regression in ModelChoiceField with RadioSelect.

comment:8 Changed 3 years ago by Tim Graham <timograham@…>

In 86ae2b2:

[1.10.x] Fixed #27001 -- Fixed a query count regression in ModelChoiceField with RadioSelect.

Backport of c5ebfda00226e3695cadbc13ea9ce4c5951d3ed0 from master

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