Opened 6 years ago

Closed 4 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 xx396)

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 xx396, 6 years ago

Description: modified (diff)

comment:2 by xx396, 6 years ago

Description: modified (diff)

comment:3 by Zeeshan Khan, 6 years ago

Owner: changed from nobody to Zeeshan Khan
Status: newassigned

comment:4 by Tim Graham, 6 years ago

Easy pickings: unset
Summary: Serializer handle_m2m_field should honour any previous prefetchSerializer handle_m2m_field() should honor any previous prefetch
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

I guess it would be nice to fix that but removing iterator() negates the memory savings on the non-prefetch case, doesn't it?

comment:5 by Claude Paroz, 4 years ago

Has patch: set

I have two suggestion commits for possible fixes:

I can turn one of them into a PR if an ORM informed person could validate either approach.

comment:6 by Simon Charette, 4 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.

Last edited 4 years ago by Simon Charette (previous) (diff)

comment:7 by Claude Paroz, 4 years ago

Thanks, PR.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In adf58311:

Fixed #29078 -- Made serializers respect prefetch_related() for m2m fields.

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