django-rest-framework Reduce queries for supported ManyRelatedField

Description

From the discussion here https://github.com/encode/django-rest-framework/discussions/8919, a related field with a many=True will only execute one query for all items instead of one for every item.

It currently works for:

PrimaryKeyRelatedField
SlugRelatedField

There are currently four issues regarding this pull request:

if someone is inheriting a field with to_many_internal_value and only override to_internal_value, the overridden value won't be used (because to_many_internal_value will be use instead).
- We could check if the to_internal_value property was overwritten and raise an error / use the old behaviour in this case but I am not sure this is a good idea
For PrimaryKeyRelatedField, when the type is wrong, the type of the first item is raised in the error as it is unclear how to retrieve the problematic item. Some solutions to keep the old behaviour if this is an issue:
- Check that all items have the same type before querying the database and raise an error if some of them have different type
- In the exception handling, run the query for every item to check which one has the issue
For SlugRelatedField, I add to annotate the related field to detect if an item does not exist and which one. I don't know if this is acceptable or not.
The pre-commit check fails for a reason I don't understand

Jan 05 '24 12:01 clement-escolano

tests seems to be failing after new push, can you recheck please?

Jan 05 '24 12:01 auvipy

tests seems to be failing after new push, can you recheck please?

It should be good now. There is also some general issues to talk about in the PR description :-)

Jan 05 '24 15:01 clement-escolano

@auvipy What do you think about the pull request? Is there some ways I can improve it?

Jan 15 '24 13:01 clement-escolano

It currently works for:

* PrimaryKeyRelatedField

* SlugRelatedField

There are currently four issues regarding this pull request:

* if someone is inheriting a field with `to_many_internal_value` and only override `to_internal_value`, the overridden value won't be used (because `to_many_internal_value` will be use instead).
  
  * We could check if the `to_internal_value` property was overwritten and raise an error / use the old behaviour in this case but I am not sure this is a good idea

* For `PrimaryKeyRelatedField`, when the type is wrong, the type of the first item is raised in the error as it is unclear how to retrieve the problematic item. Some solutions to keep the old behaviour if this is an issue:
  
  * Check that all items have the same type before querying the database and raise an error if some of them have different type
  * In the exception handling, run the query for every item to check which one has the issue

* For `SlugRelatedField`, I add to annotate the related field to detect if an item does not exist and which one. I don't know if this is acceptable or not.

what is the current standing of the issues now? is this backward compatible? and do you think all other related issues can be resolved in this PR?

Jan 15 '24 16:01 auvipy

I think all issues should be fixed in this pull request but I am asking for some advice on the issues because I am not sure what is the best way of solving them. Basically:

for the inheritance, do you think checking if the method was overridden is a good idea?
for the wrong type, which solutions I am suggesting look the best for you?
for the annotate, is this even an issue at all?

Jan 15 '24 16:01 clement-escolano