Opened 15 years ago

Closed 8 years ago

#10506 closed Bug (fixed)

Automatically use correct auto-manager class for inherited models

Reported by: Malcolm Tredinnick Owned by: Malcolm Tredinnick
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: use_for_related_fields
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When creating a subclass of a model with a default manager that has use_for_related_fields = True, and if the subclass has no explicit manager set, we should use the class of the parent's manager. This is consistent with every other time we create an automatic manager (in fact, the docs added in r10057 hint that we already do this, but it's not quite true for subclasses yet).

For multiple base classes, we walk them all and choose the most derived manager satisfying this constraint. It should subclass all the other managers that also do. If there is no such manager (e.g. two different managers subclassing django.db.models.Manager), we raise an error and the user has to specify the default manager themselves. This is because there would be no way to ensure that an appropriate get_query_set() method id used

Using MRO to determine the class type is wrong, since a model that inherits from both an ordinary model and a gis model, in that order, needs a GeoManager (which already subclasses Manager, so we're fine).

When this is done, there's a "TODO" in the gis tests that can be removed, too (in a child model).

I'll get to this before 1.1, but I had to fix the first parts of this as part of some other yak-shaving and don't want to get distracted on this yet.

Change History (9)

comment:1 Changed 15 years ago by Malcolm Tredinnick

Summary: Understand correct auto-manager class for inherited modelsAutomatically use correct auto-manager class for inherited models
Triage Stage: UnreviewedAccepted

comment:2 Changed 15 years ago by Malcolm Tredinnick

milestone: 1.1

Given the work to do before 1.1, pushing this one to later.

comment:3 Changed 13 years ago by Chris Beaven

Severity: Normal
Type: Bug

comment:4 Changed 12 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:5 Changed 12 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:6 Changed 8 years ago by Alex Poleha

Replying to mtredinnick:

When creating a subclass of a model with a default manager that has use_for_related_fields = True, and if the subclass has no explicit manager set, we should use the class of the parent's manager.

I don't think that child class should inherit parent class's manager since documentation says it shouldn't: https://docs.djangoproject.com/en/dev/topics/db/managers/#custom-managers-and-model-inheritance

comment:7 Changed 8 years ago by Tim Graham

Keywords: use_for_related_fields added

comment:8 Changed 8 years ago by Loic Bistuer

PR6175 revamps manager inheritance and allows choosing the default manager explicitly through the base_manager_name model option.

Last edited 8 years ago by Loic Bistuer (previous) (diff)

comment:9 Changed 8 years ago by Loïc Bistuer <loic.bistuer@…>

Resolution: fixed
Status: newclosed

In ed0ff91:

Fixed #10506, #13793, #14891, #25201 -- Introduced new APIs to specify models' default and base managers.

This deprecates use_for_related_fields.

Old API:

class CustomManager(models.Model):

use_for_related_fields = True

class Model(models.Model):

custom_manager = CustomManager()

New API:

class Model(models.Model):

custom_manager = CustomManager()

class Meta:

base_manager_name = 'custom_manager'

Refs #20932, #25897.

Thanks Carl Meyer for the guidance throughout this work.
Thanks Tim Graham for writing the docs.

Note: See TracTickets for help on using tickets.
Back to Top