[Django] #25496: ModelChoiceField generates extra queries by not respecting prefetch_related
Django
-------------------------------+--------------------
Reporter: mdentremont | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version:
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
''Note'': could be worked around if this were resolved:
https://code.djangoproject.com/ticket/22841
When a `ModelChoiceField`'s choices are generated, the queryset's
`prefetch_related` calls are not evaluated, which can lead to 1 query per
choice if the model's `unicode` accesses a related field.
Example models:
{{{#!python
from django.db import models
class RelatedObj(models.Model):
name = models.CharField(max_length=255)
class ObjWithRelated(models.Model):
name = models.CharField(max_length=255)
related = models.ManyToManyField(RelatedObj)
def __unicode__(self):
return '{}: {}'.format(self.name, self.related.count())
}}}
With many models, we can see the following results:
{{{#!python
from django.db import connection
from django.forms import ModelChoiceField
from django.test.utils import CaptureQueriesContext
queryset = ObjWithRelated.objects.prefetch_related('related')
field = ModelChoiceField(queryset)
with CaptureQueriesContext(db.connection) as queries:
list(field.choices)
print 'ModelChoiceField query count: {}'.format(len(queries))
with CaptureQueriesContext(db.connection) as queries:
[str(obj) for obj in queryset]
print 'Regular query count: {}'.format(len(queries))
}}}
This will have the output of:
{{{#!comment
ModelChoiceField query count: <at least 1 query to evaluate the queryset +
1 extra query per ObjWithRelatedModel>
Regular query count: 2
}}}
There are ways to work around this, but ideally there would be a solution
which wouldn't require a workaround. We're using
[https://github.com/edoburu/django-parler django-parler] to translate
models, without the prefetching we get 1 query per dropdown choice, per
dropdown when a translated model is displayed in the django admin.
--
Ticket URL: <https://code.djangoproject.com/ticket/25496>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: mdentremont | Owner: nobody
Type: Uncategorized | Status: closed
Component: Forms | Version:
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* status: new => closed
* needs_better_patch: => 0
* resolution: => invalid
* needs_tests: => 0
* needs_docs: => 0
Comment:
Hi mdentremont,
I think you misunderstood how `prefetch_related` and `count()` interact.
The `count()` method never takes prefetched data into consideration and
always result in a new query.
If you want to avoid an extra query I suggest you `annotate()` your
queryset with its count of related objects and adapt your models
`__unicode__` to lookup the annotated attribute first and fallback to
calling `self.related.count()`:
{{{#!python
from django.db import models
class RelatedObj(models.Model):
name = models.CharField(max_length=255)
class ObjWithRelated(models.Model):
related = models.ManyToManyField(RelatedObj)
def __unicode__(self):
related_count = getattr(self, 'related_count', None)
if related_count is None:
related_count = self.related.count()
return '{}: {}'.format(self.name, related_count)
queryset =
ObjWithRelated.objects.annotate(related_count=models.Count('related'))
field = ModelChoiceField(queryset)
}}}
Please use support channels before filling a ticket
wiki:TicketClosingReasons/UseSupportChannels
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:1>
Django
Reporter: mdentremont | Owner: nobody
Type: Uncategorized | Status: closed
Component: Forms | Version:
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mdentremont):
Hello charettes,
Sorry, I used `.count()` in the example to try and simplify things a
little. In actuality we are accessing a field on a related object in our
`unicode`, in which case we definitely would want to use
`prefetch_related`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:2>
Django
Reporter: mdentremont | Owner: nobody
Type: Uncategorized | Status: closed
Component: Forms | Version:
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* resolution: invalid => duplicate
Comment:
Then I guess this is simply a duplicate of #22841?
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:3>
Django
Reporter: mdentremont | Owner: nobody
Type: Uncategorized | Status: closed
Component: Forms | Version:
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mdentremont):
Potentially?
My thinking was that #22841 is for passing in a queryset which will not be
revaluated, where I want the re-evaluation to a avoid stale content, I
just want the prefetch to be respected to avoid too many queries.
Also it took us a ton of investigation to determine why the prefetch
wasn't performed, it is not obvious at all. If I pass a queryset along, I
expect that it will be used entirely, not just certain pieces of it.
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:4>
Django
Reporter: mdentremont | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* status: closed => new
* type: Uncategorized => Bug
* resolution: duplicate =>
Comment:
Hmm it looks like it might be a regression in `1.8` by the fix for #23623
(fa534b92dda0771661a98a1ca302ced264d0a6da) which introduced the use of
`iterator()` to reduce memory usage.
From the `prefetch_related`
[https://docs.djangoproject.com/en/1.8/ref/models/querysets/#prefetch-
related docs]
> Note that if you use `iterator()` to run the query, `prefetch_related()`
calls will be ignored since these two optimizations do not make sense
together.
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:5>
Django
Reporter: mdentremont | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
* version: => 1.8
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
[https://github.com/django/django/pull/5397 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:6>
Django
Reporter: mdentremont | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.8
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:7>
Django
Reporter: mdentremont | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"6afa6818fcf25665bbf61f0921c8c8c6fa8f223e" 6afa6818]:
{{{
#!CommitTicketReference repository=""
revision="6afa6818fcf25665bbf61f0921c8c8c6fa8f223e"
Fixed #25496 -- Made ModelChoiceField respect prefetch_related().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:8>
Django
Reporter: mdentremont | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"6bc8bdf55ae71482ee3cd6c1ab157404c2e4ec5c" 6bc8bdf]:
{{{
#!CommitTicketReference repository=""
revision="6bc8bdf55ae71482ee3cd6c1ab157404c2e4ec5c"
[1.9.x] Fixed #25496 -- Made ModelChoiceField respect prefetch_related().
Backport of 6afa6818fcf25665bbf61f0921c8c8c6fa8f223e from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:9>
Django
Reporter: mdentremont | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"de570d4da9447ebef253519c2473d3b4f6fb2bea" de570d4]:
{{{
#!CommitTicketReference repository=""
revision="de570d4da9447ebef253519c2473d3b4f6fb2bea"
[1.8.x] Fixed #25496 -- Made ModelChoiceField respect prefetch_related().
Backport of 6afa6818fcf25665bbf61f0921c8c8c6fa8f223e from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:10>
Django
Reporter: mdentremont | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by intgr):
* cc: marti@… (added)
Comment:
It appears that this fix caused a regression. I have submitted pull
requests with fixes, see ticket #25683.
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:11>
Django
Reporter: mdentremont | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"1155843a41af589a856efe8e671a796866430049" 1155843a]:
{{{
#!CommitTicketReference repository=""
revision="1155843a41af589a856efe8e671a796866430049"
Fixed #25683 -- Allowed ModelChoiceField(queryset=...) to accept Managers.
This fixes a regression from refs #25496.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:12>
Django
Reporter: mdentremont | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"8db5122d6937bfcaf9a5894ba8f329d2a4424ca5" 8db5122d]:
{{{
#!CommitTicketReference repository=""
revision="8db5122d6937bfcaf9a5894ba8f329d2a4424ca5"
[1.9.x] Fixed #25683 -- Allowed ModelChoiceField(queryset=...) to accept
Managers.
This fixes a regression from refs #25496.
Backport of 1155843a41af589a856efe8e671a796866430049 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:13>
Django
Reporter: mdentremont | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"3144785ebf9fdc19639ef3d35267283f5c2f321a" 3144785]:
{{{
#!CommitTicketReference repository=""
revision="3144785ebf9fdc19639ef3d35267283f5c2f321a"
[1.8.x] Fixed #25683 -- Allowed ModelChoiceField(queryset=...) to accept
Managers.
This fixes a regression from refs #25496.
Backport of 1155843a41af589a856efe8e671a796866430049 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25496#comment:14>