Avoid N+1 queries in PrimaryKeyRelatedField(many=True) validation by adelkhayata76 · Pull Request #9984 · encode/django-rest-framework

adelkhayata76 · 2026-06-17T12:55:56Z

ManyRelatedField.to_internal_value resolved each related object with its own to_internal_value() call, so validating a list of N primary keys ran N SELECT queries. As @sevdog noted on the issue, the many-related path delegates per-item and does no DB-level batching.

Change

Override PrimaryKeyRelatedField.many_init to return a private PrimaryKeyManyRelatedField that resolves every pk with a single in_bulk() query.
SlugRelatedField, HyperlinkedRelatedField, and custom relations keep the default RelatedField.many_init → ManyRelatedField path (no new extension point on RelatedField).
Collect every invalid item into an index-keyed ValidationError, matching ListField.run_child_validation and the ListSerializer(many=True) dict format from Change errors for list serializers (many=True) to dict format #9837.

Errors: all invalid indexes, one query

incorrect_type, does_not_exist, and pk_field failures are reported together as {index: [ErrorDetail, ...]} instead of failing on the first item. Input ordering, duplicate handling, the queryset filter, and pk_field transforms are preserved. A queryset that cannot use in_bulk() (e.g. sliced) falls back to a collecting per-item loop.

input pks	before	after
10	10 SELECT	1 SELECT

Tests

Adds regression tests in tests/test_relations_pk.py, including an assertNumQueries(1) guard, parity tests for ordering/duplicates/queryset filtering/pk_field, mixed does_not_exist/incorrect_type, collected pk_field validation errors, and that many=True builds PrimaryKeyManyRelatedField.

Follow-up

ListSerializer.create has the same per-item shape (also flagged on the issue); left out here to keep this change surgical. Read-side to_representation batching (values_list('pk')) is tracked separately — see discussion on this PR.

ManyRelatedField.to_internal_value resolved each related object with a separate child_relation.to_internal_value() call, so validating a list of N primary keys issued N SELECT queries (encode#9607). Add an opt-in to_internal_value_bulk() hook on RelatedField (defaulting to the existing per-item loop, so SlugRelatedField, HyperlinkedRelatedField and custom relations are unchanged) and override it on PrimaryKeyRelatedField to resolve every pk with a single in_bulk() query. Per-item error semantics (incorrect_type / does_not_exist), input ordering, duplicate handling, the queryset filter and pk_field transforms are all preserved; a type the backend cannot compare falls back to the per-item path so the offending item still raises the same error.

Handle string primary keys (e.g. from HTML form input): in_bulk() keys its result by the database pk type, so a string "1" must be coerced via the pk field's get_prep_value() before the membership check, exactly as queryset.get(pk=...) does. Without this, string pks raised a spurious does_not_exist error. Also make the regression tests rely on the pks actually created in setUp rather than hard-coding 1..5, which is not guaranteed across backends/test ordering, and add an explicit string-pk test.

Copilot

Pull request overview

This PR introduces an opt-in bulk validation hook for relational fields to eliminate N+1 queries during PrimaryKeyRelatedField(many=True) validation, resolving related instances via a single in_bulk() query and adding regression coverage for query count and semantic parity.

Changes:

Added RelatedField.to_internal_value_bulk() as a bulk-conversion hook used by ManyRelatedField.
Implemented a batched to_internal_value_bulk() on PrimaryKeyRelatedField using queryset.in_bulk(...).
Added regression tests to ensure PrimaryKeyRelatedField(many=True) validates with one query and preserves ordering/duplicates/errors/pk_field behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`rest_framework/relations.py`	Adds the bulk validation hook and switches `ManyRelatedField` to use it; implements PK bulk resolution via `in_bulk()`.
`tests/test_relations_pk.py`	Adds regression tests asserting single-query validation and parity behaviors for PK many validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

-            self.child_relation.to_internal_value(item)
-            for item in data
-        ]
+        return self.child_relation.to_internal_value_bulk(data)


+        pks = []
+        for item in data:
+            value = item
+            if self.pk_field is not None:
+                value = self.pk_field.to_internal_value(value)
+            try:
+                if isinstance(value, bool):
+                    raise TypeError
+                # Coerce to the pk's Python type (e.g. "1" -> 1) so the lookup
+                # below matches the keys returned by `in_bulk()`, exactly as
+                # `queryset.get(pk=value)` would have.
+                value = model_pk.get_prep_value(value)
+            except (TypeError, ValueError):
+                self.fail('incorrect_type', data_type=type(item).__name__)
+            pks.append(value)
+        try:
+            objects = queryset.in_bulk(pks)
+        except (TypeError, ValueError):
+            # queryset doesn't support in_bulk (e.g. distinct/sliced); fall
+            # back to the per-item path so behavior is unchanged.
+            return [self.to_internal_value(item) for item in data]
+        result = []
+        for pk in pks:
+            if pk not in objects:
+                self.fail('does_not_exist', pk_value=pk)


- Report `incorrect_type` / `does_not_exist` details using the post-`pk_field` value (matching the per-item `to_internal_value` path) instead of the raw input or the pk-coerced lookup key. With a type-changing `pk_field` (e.g. BooleanField) the bulk path previously reported a different `data_type`. - `ManyRelatedField.to_internal_value` now falls back to the per-item loop when the child field has no `to_internal_value_bulk`, so wrapping a non-RelatedField child no longer raises AttributeError. Adds regression tests for both.

adelkhayata76 · 2026-06-17T14:39:58Z

Thanks for the review. Addressed both points in b0a437d0:

1. AttributeError when the child isn't a RelatedField — good catch. ManyRelatedField.to_internal_value now falls back to the per-item loop when the child field has no to_internal_value_bulk, so the optimization only applies to relational children and any other child field type keeps working as before:

bulk = getattr(self.child_relation, 'to_internal_value_bulk', None)
if bulk is not None:
    return bulk(data)
return [self.child_relation.to_internal_value(item) for item in data]

2. Error-detail divergence with a custom pk_field — you're right, and it was observable. With pk_field=BooleanField() and input "true", the per-item path reported received bool while the bulk path reported received str. The bulk method now tracks (lookup_key, value) pairs and uses the post-pk_field value for both incorrect_type (type(value)) and does_not_exist (pk_value=value) details — matching to_internal_value exactly — while the pk-coerced lookup_key is used only to match in_bulk() results.

Added regression tests for both: one asserting the bulk error detail matches the per-item path under a type-changing pk_field, and one asserting a ManyRelatedField wrapping a non-relational child still validates.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

rest_framework/relations.py:293

PrimaryKeyRelatedField.to_internal_value_bulk() currently raises incorrect_type during the pre-processing loop, before it knows whether an earlier item would have raised does_not_exist. This can change which error is reported for mixed inputs (e.g. [missing_pk, object()] would raise incorrect_type in the bulk path, but the per-item path raises does_not_exist for the first element). To preserve per-item left-to-right error precedence, defer raising incorrect_type until after the bulk query and then iterate in order, raising the first error that would occur per-item (missing vs incorrect type).

                # below matches the keys returned by `in_bulk()`, exactly as
                # `queryset.get(pk=value)` would have.
                lookup_key = model_pk.get_prep_value(value)
            except (TypeError, ValueError):
                self.fail('incorrect_type', data_type=type(value).__name__)

rest_framework/relations.py:574

ManyRelatedField.to_internal_value() calls to_internal_value_bulk whenever the attribute exists, but it doesn't verify it's callable. A non-callable attribute with that name (accidental or otherwise) would raise a confusing TypeError. Safer to gate on callable() and fall back to the per-item loop otherwise.

        # `to_internal_value_bulk` is defined on `RelatedField`; fall back to
        # the per-item loop for any other child field type.
        bulk = getattr(self.child_relation, 'to_internal_value_bulk', None)
        if bulk is not None:
            return bulk(data)

+        with pytest.raises(serializers.ValidationError) as exc_info:
+            field.run_validation(['not-a-pk'])
+        assert exc_info.value.detail[0].code == 'incorrect_type'
+


JPDSousa

Have you explored the alternative of using a class PrimaryKeyManyRelatedField(ManyRelatedField) which implements .to_internal_value, rather than adding one more extension point to RelatedField?

I have no strong position towards any option, but raising this here, as other may have.

JPDSousa · 2026-09-02T09:27:02Z

+                # Coerce to the pk's Python type (e.g. "1" -> 1) so the lookup
+                # below matches the keys returned by `in_bulk()`, exactly as
+                # `queryset.get(pk=value)` would have.
+                lookup_key = model_pk.get_prep_value(value)


Isn't this redundant with what queryset will do internally?

Django does coerce values for SQL, but in_bulk() returns a dict keyed by the instance pk (1), not the input ("1"). Without get_prep_value, HTML string pks miss the dict and raise does_not_exist. The bool guard is also required (bool is a subclass of int).

JPDSousa · 2026-09-02T09:27:07Z

+        result = []
+        for lookup_key, value in entries:
+            if lookup_key not in objects:
+                self.fail('does_not_exist', pk_value=value)


You should report errors in bulk as well, rather than just failing on the first invalid item. That is consistent with error handling in multi-item fields.

Same change as above — does_not_exist is collected with the rest of the list instead of aborting on the first missing pk.

JPDSousa · 2026-09-02T09:29:59Z

+                # `queryset.get(pk=value)` would have.
+                lookup_key = model_pk.get_prep_value(value)
+            except (TypeError, ValueError):
+                self.fail('incorrect_type', data_type=type(value).__name__)


Per-item errors should be reported in bulk as well. This is the common behavior for ListSerializer and ListField

Done. to_internal_value_bulk now collects every incorrect_type, does_not_exist, and pk_field error into an index-keyed dict — the same shape as ListField.run_child_validation (and the ListSerializer(many=True) dict format from #9837). One in_bulk() query is still used whenever any items are type-valid.

JPDSousa · 2026-09-02T09:32:11Z

        except (TypeError, ValueError):
            self.fail('incorrect_type', data_type=type(data).__name__)

+    def to_internal_value_bulk(self, data):


I know that .prefetch_related on the queryset removes the N+1 problem on read operations, but shouldn't we also implement a consistent optimization for .to_representation? Reasons being:

Developers don't need to remember about .prefetch_related

We can use .values_list('pk', flat=True) to return just the primary keys this field needs.

Agreed that a matching read-side optimization would be useful so callers don't have to remember prefetch_related. I'd rather keep this PR on write-side validation and take to_representation / values_list('pk') as a follow-up — happy to open an issue if you want that tracked.

Yeah, please do open an issue 👍

Update: GitHub blocks issue creation on encode/django-rest-framework for non-collaborators (see the “limited to collaborators” banner). I left the full issue text in a PR comment for you or a maintainer to file: #9984 (comment)

Report every incorrect_type, does_not_exist, and pk_field error in one index-keyed ValidationError, matching ListField, instead of failing on the first item. Co-authored-by: Cursor <cursoragent@cursor.com>

adelkhayata76 · 2026-09-02T10:18:59Z

@JPDSousa on the PrimaryKeyManyRelatedField alternative: I did look at it. RelatedField.many_init already documents overriding the many class, so that path is available. I kept to_internal_value_bulk() on RelatedField so SlugRelatedField, HyperlinkedRelatedField, and custom relations stay on the existing per-item loop without teaching many_init a new type. Happy to switch if maintainers prefer the subclass.

JPDSousa · 2026-09-02T13:38:09Z

@JPDSousa on the PrimaryKeyManyRelatedField alternative: I did look at it. RelatedField.many_init already documents overriding the many class, so that path is available. I kept to_internal_value_bulk() on RelatedField so SlugRelatedField, HyperlinkedRelatedField, and custom relations stay on the existing per-item loop without teaching many_init a new type. Happy to switch if maintainers prefer the subclass.

@adelkhayata76

I meant that you could override PrimeryKeyRelatedField.many_init, which preserves the behavior for SlugRelatedField and HyperlinkedRelatedField.

Override PrimaryKeyRelatedField.many_init so many=True builds a dedicated ManyRelatedField subclass with in_bulk validation. Removes to_internal_value_bulk from RelatedField and leaves Slug/Hyperlinked on the default path. Co-authored-by: Cursor <cursoragent@cursor.com>

adelkhayata76 · 2026-09-02T13:48:08Z

@JPDSousa thanks for the clarification — that makes sense.

Switched in 13563f3d: PrimaryKeyRelatedField.many_init now returns a private PrimaryKeyManyRelatedField that does the in_bulk() + collect-all error path. Removed to_internal_value_bulk from RelatedField / PrimaryKeyRelatedField, and restored plain ManyRelatedField to the per-item loop. SlugRelatedField / HyperlinkedRelatedField stay on the default many_init.

adelkhayata76 · 2026-09-02T13:51:28Z

@JPDSousa re: opening the to_representation follow-up issue — I tried, but issue creation on encode/django-rest-framework is restricted to collaborators (same via API). Could you or a maintainer open it from the text below? Happy to co-author / adjust.

Title: Optimize PrimaryKeyRelatedField(many=True).to_representation to avoid N+1 reads

Description

Follow-up from #9984 / #9607.

PrimaryKeyRelatedField(many=True) validation is being optimized to resolve pks with a single in_bulk() query. On the read side, ManyRelatedField.to_representation still iterates and calls child_relation.to_representation per related object. That can N+1 when the relation was not prefetched.

Proposal

For the primary-key many case, batch representation so callers do not need to remember prefetch_related only to serialize pks, e.g. use something like:

queryset.values_list('pk', flat=True)

(or an equivalent that still honors pk_field transforms and ordering).

Why

Developers should not need prefetch_related just to emit a list of primary keys.
Matches the write-side goal of Avoid N+1 queries in PrimaryKeyRelatedField(many=True) validation #9984: fewer queries for the common PK many path.

Scope notes

Keep this separate from Avoid N+1 queries in PrimaryKeyRelatedField(many=True) validation #9984 (write-side validation only).
Other relation types (SlugRelatedField, HyperlinkedRelatedField) can stay out of the first cut unless an obvious shared hook appears.

Related discussion: #9984 (comment)

JPDSousa · 2026-09-03T07:42:40Z

@auvipy What are the next steps to get this merged?

adelkhayata76 added 3 commits June 17, 2026 16:52

Use US spelling in comment (codespell)

9c03f88

auvipy requested review from auvipy and Copilot June 17, 2026 14:19

Copilot started reviewing on behalf of auvipy June 17, 2026 14:19 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

browniebroke requested a review from Copilot July 25, 2026 19:14

Copilot started reviewing on behalf of browniebroke July 25, 2026 19:15 View session

Copilot AI reviewed Jul 25, 2026

View reviewed changes

Comment thread tests/test_relations_pk.py

with pytest.raises(serializers.ValidationError) as exc_info:

field.run_validation(['not-a-pk'])

assert exc_info.value.detail[0].code == 'incorrect_type'

Merge branch 'main' into fix/9607-pk-related-n-plus-one

fda2a90

stumpylog mentioned this pull request Aug 31, 2026

Performance: batch-resolve tag ids in TagsField instead of one query per id paperless-ngx/paperless-ngx#13805

Draft

12 tasks

JPDSousa reviewed Sep 2, 2026

View reviewed changes

Collect all invalid pks in PrimaryKeyRelatedField bulk validation.

8b73670

Report every incorrect_type, does_not_exist, and pk_field error in one index-keyed ValidationError, matching ListField, instead of failing on the first item. Co-authored-by: Cursor <cursoragent@cursor.com>

Merge branch 'main' into fix/9607-pk-related-n-plus-one

dbcc4ab

Merge branch 'main' into fix/9607-pk-related-n-plus-one

e089278

Uh oh!

Uh oh!

Conversation

adelkhayata76 commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change

Errors: all invalid indexes, one query

Tests

Follow-up

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

adelkhayata76 commented Jun 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

JPDSousa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adelkhayata76 commented Sep 2, 2026

Uh oh!

JPDSousa commented Sep 2, 2026

Uh oh!

adelkhayata76 commented Sep 2, 2026

Uh oh!

adelkhayata76 commented Sep 2, 2026

Uh oh!

JPDSousa commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

adelkhayata76 commented Jun 17, 2026 •

edited

Loading