Opened 8 years ago
Closed 5 years ago
#29078 closed Cleanup/optimization (fixed)
Serializer handle_m2m_field() should honor any previous prefetch
| Reported by: | xx396 | Owned by: | Zeeshan Khan |
|---|---|---|---|
| Component: | Core (Serialization) | Version: | 2.0 |
| Severity: | Normal | 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 (last modified by )
Example:
from django.contrib.auth.models import Group
from django.core import serializers
groups = Group.objects.prefetch_related('permissions').all()
serializers.serialize('json', groups)
This will N+1 query the permissions as handle_m2m_field uses iterator() which bypasses any cache. Suggest serializers/python.py line 78 replaces iterator() with all()
Change History (8)
comment:1 by , 8 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 8 years ago
| Description: | modified (diff) |
|---|
comment:3 by , 8 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:4 by , 8 years ago
| Easy pickings: | unset |
|---|---|
| Summary: | Serializer handle_m2m_field should honour any previous prefetch → Serializer handle_m2m_field() should honor any previous prefetch |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Bug → Cleanup/optimization |
comment:5 by , 5 years ago
| Has patch: | set |
|---|
I have two suggestion commits for possible fixes:
- https://github.com/claudep/django/commit/7173c7de
- https://github.com/claudep/django/commit/68bbb8ff
I can turn one of them into a PR if an ORM informed person could validate either approach.
comment:6 by , 5 years ago
Claude, I think https://github.com/claudep/django/commit/7173c7de makes more sense as the API to retrieve cached many-to-many results should likely be added to RelatedManager (e.g. expose a get_prefetched_objects method) instead of QuerySet.iterator.
I guess it would be nice to fix that but removing
iterator()negates the memory savings on the non-prefetch case, doesn't it?