Skip to content

Fix #9607: Resolve N+1 queries with PrimaryKeyRelatedField many=True #9947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MuraveyApp wants to merge 1 commit into from
+25 −0
Closed

Fix #9607: Resolve N+1 queries with PrimaryKeyRelatedField many=True #9947

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions rest_framework/relations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,28 @@ def to_internal_value(self, data):
except (TypeError, ValueError):
self.fail('incorrect_type', data_type=type(data).__name__)

def many_to_internal_value(self, data):
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces a new optimized code path for many=True deserialization. Please add tests that (1) assert it does a single batched DB query (no N+1) and (2) covers pk normalization cases (e.g. UUID string casing / integer strings with leading zeros) to prevent regressions in the new mapping logic.

Copilot uses AI. Check for mistakes.
pks = []
for item in data:
if self.pk_field is not None:
item = self.pk_field.to_internal_value(item)
if isinstance(item, bool):
self.fail('incorrect_type', data_type=type(item).__name__)
pks.append(item)
queryset = self.get_queryset()
try:
objs = {str(obj.pk): obj for obj in queryset.filter(pk__in=pks)}
except (ValueError, TypeError):
# Fall back to per-item validation to surface correct error messages
return [self.to_internal_value(pk) for pk in pks]
Comment on lines +273 to +278
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The batched queryset.filter(pk__in=pks) path can raise django.core.exceptions.ValidationError for values that would previously only error if/when that specific item was validated (e.g. invalid UUID strings). Since the current except only catches (ValueError, TypeError), this bypasses the intended per-item fallback and can change which error is surfaced based on list contents/order. Consider also catching Django's ValidationError here and falling back to per-item validation to preserve previous error behavior.

Copilot uses AI. Check for mistakes.
result = []
for pk in pks:
obj = objs.get(str(pk))
Comment on lines +266 to +281
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Building the lookup map with string keys (str(obj.pk) / str(pk)) can incorrectly raise does_not_exist for inputs that Django will coerce successfully but whose string form differs from the canonical obj.pk string (e.g. UUIDs with uppercase hex, integer PKs with leading zeros or surrounding whitespace). Consider normalizing each incoming pk to the model PK type (e.g. via queryset.model._meta.pk.to_python(...) when pk_field is None) and keying the map by the normalized value (or using queryset.in_bulk(normalized_pks)), so lookups match Django’s coercion behavior.

Suggested change
pks = []
for item in data:
if self.pk_field is not None:
item = self.pk_field.to_internal_value(item)
if isinstance(item, bool):
self.fail('incorrect_type', data_type=type(item).__name__)
pks.append(item)
queryset = self.get_queryset()
try:
objs = {str(obj.pk): obj for obj in queryset.filter(pk__in=pks)}
except (ValueError, TypeError):
# Fall back to per-item validation to surface correct error messages
return [self.to_internal_value(pk) for pk in pks]
result = []
for pk in pks:
obj = objs.get(str(pk))
queryset = self.get_queryset()
pks = []
try:
for item in data:
if self.pk_field is not None:
item = self.pk_field.to_internal_value(item)
if isinstance(item, bool):
self.fail('incorrect_type', data_type=type(item).__name__)
if self.pk_field is None:
item = queryset.model._meta.pk.to_python(item)
pks.append(item)
objs = queryset.in_bulk(pks)
except (ValueError, TypeError):
# Fall back to per-item validation to surface correct error messages
return [self.to_internal_value(pk) for pk in data]
result = []
for pk in pks:
obj = objs.get(pk)

Copilot uses AI. Check for mistakes.
if obj is None:
self.fail('does_not_exist', pk_value=pk)
result.append(obj)
return result

def to_representation(self, value):
if self.pk_field is not None:
return self.pk_field.to_representation(value.pk)
Expand Down Expand Up @@ -524,6 +546,9 @@ def to_internal_value(self, data):
if not self.allow_empty and len(data) == 0:
self.fail('empty')

if hasattr(self.child_relation, 'many_to_internal_value'):
return self.child_relation.many_to_internal_value(data)

return [
self.child_relation.to_internal_value(item)
for item in data
Expand Down
Loading