Avoid N+1 queries in PrimaryKeyRelatedField(many=True) validation

[adelkhayata76]

Fixes #9607.

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.

[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.

[adelkhayata76]

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)

[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]

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.

[adelkhayata76]

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

[adelkhayata76]

@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]

@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.

[adelkhayata76]

@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]

@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]

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

[auvipy]

lets wait for another round of review

[Copilot]

🟡 Changes recommended

Custom relation overrides and custom primary-key conversions can break, while the sliced-queryset fallback rejects valid values.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

rest_framework/relations.py:639

• get_prep_value() produces a database lookup value, not necessarily the Python value stored in obj.pk. Passing that prepared value to in_bulk() causes Django to prepare it again, and the returned mapping is keyed by the model's Python PK; custom primary-key fields with non-idempotent preparation can therefore query the wrong key or report an existing object as missing. Use the field's Python conversion for comparison keys and let the ORM perform database preparation once.

                # 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)

rest_framework/relations.py:655

• The stated sliced-queryset fallback does not work: after in_bulk() rejects a sliced queryset, child.to_internal_value() calls queryset.get(pk=...), which also attempts to filter the sliced queryset and raises TypeError. Because the child catches that exception as incorrect_type, even valid PKs are rejected. The new test only mocks in_bulk() on an unsliced queryset, so it misses this behavior; handle sliced querysets without calling get() on the slice and test with an actual slice.

        except (TypeError, ValueError):
            # queryset doesn't support in_bulk (e.g. distinct/sliced); fall
            # back to a collecting per-item loop so mixed lists still report
            # every invalid item.

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Sliced querysets still fail in fallback, and strict Python key matching can reject database-valid primary keys.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Balanced

[auvipy]

@adelkhayata76 please cross check and fix the open suggestions

[adelkhayata76]

@auvipy Cross-checked the open Copilot suggestions and addressed them in 06e32a74:

1. Subclass many_init — already covered by your 79ea3de0 guard (cls is not PrimaryKeyRelatedField → super().many_init). Added a regression test that a subclass with an overridden to_internal_value stays on plain ManyRelatedField when many=True, and that the override is actually called.

2. Sliced queryset fallback — real bug. in_bulk() and get() both fail on sliced querysets; the old test only patched in_bulk on an unsliced QS. The fallback now materializes the slice once ({obj.pk: obj for obj in queryset}) and matches in Python, so the allowed set stays the slice. Replaced the mock with a real sliced-queryset test.

3. in_bulk key matching vs DB-accepted PKs — on Python key misses after in_bulk, we run one filter(pk__in=...) probe first. If empty → all does_not_exist (no per-miss N+1). If non-empty (CI collation / prep mismatch) → per-value get(pk=value) with ObjectDoesNotExist / type errors collected. All-valid inputs still use a single query; mixed true-miss cases use two.