Skip to content

Conversation

@diesieben07
Copy link

Problem

Currently, InheritanceManager does not support prefetch_related very well. This is mainly because Django's prefetch_related_objects expects the list it is given to be homogeneous, but for InheritanceIterable it is not.
This makes it so a prefetch_related('child__relation') will not work correctly in various ways, because

  • prefetch_related only looks at the first object in the list to make its decisions
  • Even if that happens to be correct, the prefetch was written from the point of view of the parent model, but now we have a child model instance.

Additionally, a prefetch child__relation will not be accessible in a grandchild, because from Django's point of view that would be child__grandchild__relation.

Solution

The solution has two parts:

  1. InheritanceQuerySetMixin now overrides _prefetch_related_objects and instead of using self._result_cache (which has the subclasses generated by InheritanceIterable) it first reconstructs the list of base objects. It then uses the base objects for prefetch_related_objects. This makes prefetch_related('child__relation') work.
  2. Copy the prefetch cache (which really has two parts, one for foreign keys and one for many to many) from parents into their children. This ensures that the field descriptors on the child instances can find the cache. Django does make some attempt to do this already, but only for foreign keys and only for one level of ancestry (a child will only look in its direct parent).

In the process I have also fixed the use of raw SQL in instance_of and simplified _get_ancestors_path by using the API Django already provides. If needed I can split these changes off into their own PR.

Commandments

  • Write PEP8 compliant code.
  • Cover it with tests.
  • Update CHANGES.rst file to describe the changes, and quote according issue with GH-<issue_number>.
  • Pay attention to backward compatibility, or if it breaks it, explain why.
  • Update documentation (if relevant).

@diesieben07
Copy link
Author

I have opened an issue for Django so that in the future hopefully the ugly copying of the caches can be removed:
https://code.djangoproject.com/ticket/36282

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant