#20932: Issues with model Manager and inheritance.
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> I've found a couple of issues with the current handling of managers with
> inheritance.
>
> 1. Weird bug that I can't explain yet:
> https://github.com/loic/django/compare/manager_inheritance_bug
>
> `model_inheritance_regress.tests.ModelInheritanceTest.test_abstract_base_class_m2m_relation_inheritance`
> fails with `AttributeError: 'BachelorParty' object has no attribute
> 'bachelorparty_ptr'`.
>
> 2. Managers in MTI *are* inherited, contrary to what is said
> [https://docs.djangoproject.com/en/dev/topics/db/managers/#custom-
> managers-and-model-inheritance in the docs].
>
> Quoting the docs: "Managers defined on non-abstract base classes are not
> inherited by child classes. [...] Therefore, they aren’t passed onto
> child classes.". The problem is that, since we create the new class by
> calling `object.__new__()`, normal Python attribute resolution applies,
> and we gain access to the attributes of the base classes. This doesn't
> happen to fields because these are removed during the class creation
> process, but managers are left behind. It's always tempting to think we
> could just delete them, but you cannot delete something that is not in
> your own `__dict__`. The problem is not so much that we inherit those
> managers, but that they don't return the right model type as demonstrated
> in https://github.com/loic/django/compare/manager_inheritance_bug2.
>
> Note: I know we usually should have one ticket for each bug. This time
> though, we only have a couple of lines of codes that have multiple
> consequences, so I'd like to keep the discussion centralized. We can
> always open a ticket for each bug when the time comes to actually
> implement what we decide.
New description:
I've found a couple of issues with the current handling of managers with
inheritance.
1. Given:
{{{#!python
class AbstractEvent(models.Model):
events = models.Manager()
class Meta:
abstract = True
class BachelorParty(AbstractEvent):
objects = models.Manager()
class MessyBachelorParty(BachelorParty):
pass
}}}
If `AbstractEvent` has a manager called anything but `objects` (e.g.
`events`):
`MessyBachelorParty.objects.model == <class
'model_inheritance_regress.models.BachelorParty'>`
Which causes the following failure:
{{{
model_inheritance_regress.tests.ModelInheritanceTest.test_abstract_base_class_m2m_relation_inheritance`
fails with `AttributeError: 'BachelorParty' object has no attribute
'bachelorparty_ptr'.
}}}
If `AbstractEvent` doesn't have an explicit manager, or has one called
`objects`:
`MessyBachelorParty.objects.model == <class
'model_inheritance_regress.models.MessyBachelorParty'>` which is the
expected result.
2. Managers in MTI *are* inherited, contrary to what is said
[https://docs.djangoproject.com/en/dev/topics/db/managers/#custom-
managers-and-model-inheritance in the docs] (now tracked specifically in
#25897):
"Managers defined on non-abstract base classes are not inherited by
child classes. [...] Therefore, they aren’t passed onto child classes.".
The problem is that, since we create the new class by calling
`object.__new__()`, normal Python attribute resolution applies, and we
gain access to the attributes of the base classes. This doesn't happen to
fields because these are removed during the class creation process, but
managers are left behind. It's always tempting to think we could just
delete them, but you cannot delete something that is not in your own
`__dict__`. The problem is not so much that we inherit those managers, but
that they don't return the right model type as demonstrated in
https://github.com/loic/django/compare/manager_inheritance_bug2.
--
Comment (by loic):
Investigated the first issue a bit more and updated the ticket description
with the findings. I don't think it's addressed by PR
https://github.com/django/django/pull/5797.
Currently `MessyBachelorParty.objects` is the wrongly inherited manager
from `BachelorParty`.
With the fix from the PR (after the deprecation period),
`MessyBachelorParty.objects` would raise an `AttributeError`, but the
expected behaviour would be that `MessyBachelorParty` gets an implicit
`objects = models.Manager()` created automatically.
--
Ticket URL: <https://code.djangoproject.com/ticket/20932#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/064.d860d381e1a51751e81c995ad6ab9de6%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
