#22841: ModelChoiceField does not make it easy to reuse querysets
---------------------------------------+------------------------
Reporter: mjtamlyn | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
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 |
---------------------------------------+------------------------
`ModelChoiceField` is designed to aggressively ensure that the queryset is
always copied and executed anew each time. This is generally a good idea,
but it has performance implications, especially where formsets of
modelforms are concerned. The situation is not restricted to formsets
however, there are other use cases where you may already have the executed
queryset you need for the form within that request/response cycle.
Here's a simple example of a view and form with the API I might like to
see.
{{{
# views.py
def book_create(request):
categories = Category.objects.all()
if request.method == 'POST':
form = BookForm(data=request.POST, categories=categories)
if form.is_valid():
form.save()
return HttpResponseRedirect(reverse('book_list'))
else:
form = BookForm(categories=categories)
context = {
'categories': categories,
'form': form,
}
return render('book_form.html', context)
# forms.py
class BookForm(forms.ModelForm):
class Meta:
model = Book
fields = ['name', 'category']
def __init__(self, categories, **kwargs):
super(BookForm, self).__init__(**kwargs)
self.fields['category'].evaluated_queryset = categories
}}}
So we have a view to create a book, but that view has the list of
categories in the context as it also includes a by-category navigation in
a sidebar. As a result, in order to render the view we currently have to
execute `Category.objects.all()` twice - once to render the navigation and
once for the form.
I have introduced a new proposed API to the `ModelChoiceField`
(`form.fields['category']` in the example), currently called
`evaluated_queryset`. This will be used by the `ModelChoiceIterator`
*without* calling `.all()` on it, allowing the same queryset cache to be
used twice within the view.
----
The current "best approach" for doing this that I've found looks as
follows:
{{{
class BookForm(forms.ModelForm):
# ...
def __init__(self, categories, **kwargs):
super(BookForm, self).__init__(**kwargs)
iterator = ModelChoiceIterator(self.fields['category'])
choices = [iterator.choice(obj) for obj in categories]
self.fields['category'].choices = choices
}}}
Whilst this is functional, it is not a particularly nice API. If we are
happy with it as the correct pattern, we should document it, but at
present `ModelChoiceIterator` is not documented, and it probably shouldn't
be.
----
Possible more general improvements which become possible with a feature
like this:
- Automatic sharing of querysets between identical forms in a formset
- Similarly, if the queryset has been executed then we can check inside it
instead of doing the additional `.get()` query on data validation. This
has a small performance gain on write in certain circumstances - in
particular where you have a formset with 10+ fields, loading the full
queryset once will be more efficient than doing 10 `.get()` queries.
- Inlines and list editable in the admin could use this feature for
significant performance improvements
--
Ticket URL: <https://code.djangoproject.com/ticket/22841>
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/051.d8afc6c5d291bd517efe40b0c98662e0%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
