#36183 closed Bug (invalid)
Model o2o inheritance with abstract models does not "evaluate" a lazy relationship
Reported by: | BeryCZ | Owned by: | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 5.1 |
Severity: | Normal | Keywords: | |
Cc: | BeryCZ | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Pull Requests: | How to create a pull request | ||
Description ¶
When I define models like this
class AbstractPage(models.Model): mtime = models.DateTimeField( verbose_name="Last modification at", auto_now=True, ) class Meta: abstract = True class AbstractPageType(models.Model): page = models.OneToOneField( "Page", verbose_name="ID", primary_key=True, on_delete=models.CASCADE, related_name="%(class)s", parent_link=True, ) title = models.CharField( verbose_name="Title", max_length=255, null=False, blank=False, ) class Meta: abstract = True class Page(AbstractPage): pass class ArticlePage(AbstractPageType, Page): pass
And then I do
ArticlePage.objects.order_by("-mtime").all()
I get
File "django/db/models/manager.py", line 87, in manager_method return getattr(self.get_queryset(), name)(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "django/db/models/query.py", line 1701, in order_by obj.query.add_ordering(*field_names) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "django/db/models/sql/query.py", line 2249, in add_ordering self.names_to_path(item.split(LOOKUP_SEP), self.model._meta) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "django/db/models/sql/query.py", line 1777, in names_to_path path_to_parent = opts.get_path_to_parent(model) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "django/db/models/options.py", line 755, in get_path_to_parent targets = (final_field.remote_field.get_related_field(),) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "django/db/models/fields/reverse_related.py", line 311, in get_related_field field = self.model._meta.get_field(self.field_name) ^^^^^^^^^^^^^^^^ Exception Type: AttributeError Exception Value: 'str' object has no attribute '_meta'
It works when I do
class ArticlePage(AbstractPageType, Page): page = models.OneToOneField( "Page", verbose_name="ID", primary_key=True, on_delete=models.CASCADE, related_name="%(class)s", parent_link=True, )
but I would prefer not to have to define page field in every PageType.
Change History (5)
comment:1 by , 38 hours ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 35 hours ago
Hello Natalia, thanks for your response, but I think you either misunderstood or I'm missing something that's not clear from your answer.
Based on the docs https://docs.djangoproject.com/en/5.1/topics/db/models/#multi-table-inheritance I can override the primary key on ArticlePage
so instead of page_ptr
I can have just page
. So that's what I'm doing, it works when I put it in ArticlePage
model, but not when I put it the AbstractPageType
.
I've just asked on discord and klove said it should be ok to name the pk page
.
So I even tried to remove my custom pk field and checked ArticlePage.page
and article_page_object.page
and both raise AttributeError, so by default page
is not there.
My use case is - I want to have the "parent" model Page
from which I can MTI "submodels" like Article, Category, Product, Reference, whatever. The Page
model will then be used for references in other models (galeries, urls, tree-maps, ...).
So, am I missing something, or is this a bug? I'm not saying it's an easy-to-fix bug :) but I honestly think it is a bug.
I'm not even sure where the lazy load of to
relationship is, or why it doesn't work in abstract models, otherwise I would try to fix it and PR.
Thank you,
Bery
comment:3 by , 27 hours ago
I believe your AbstractPageType
definition is invalid as it should not allowed to define a parent_link
on a model that doesn't directly inherit from the referenced model even when abstract.
The correct way to do this is to have the abstract model defining the parent link subclass the concrete model (abstract MTI). This means you should be able to achieve what you're after by re-organizing your model definition to make sure AbstractPageType
does inherit from Page
class AbstractPage(models.Model): mtime = models.DateTimeField( verbose_name="Last modification at", auto_now=True, ) class Meta: abstract = True class Page(AbstractPage): pass class AbstractPageType(Page): page = models.OneToOneField( "Page", verbose_name="ID", primary_key=True, on_delete=models.CASCADE, related_name="%(class)s", parent_link=True, ) class Meta: abstract = True class ArticlePage(AbstractPageType): pass class OtherPage(AbstractPageType): pass
This is not something Django could be easily adjusted to warn you about as no registry of abstract models is kept and thus system checks cannot be run against them.
comment:4 by , 27 hours ago
So I tried adapting BaseModel.__new__
to confirm this was effectively never supported with
-
TabularUnified django/db/models/base.py
diff --git a/django/db/models/base.py b/django/db/models/base.py index 575365e11c..5a33f78a98 100644
a b def __new__(cls, name, bases, attrs, **kwargs): 243 243 for field in base._meta.local_fields: 244 244 if isinstance(field, OneToOneField) and field.remote_field.parent_link: 245 245 related = resolve_relation(new_class, field.remote_field.model) 246 parent_links[make_model_tuple(related)] = field 246 related_model_tuple = make_model_tuple(related) 247 if abstract and not any( 248 make_model_tuple(parent) == related_model_tuple 249 for parent in parents 250 if hasattr(parent, "_meta") 251 ): 252 raise TypeError( 253 "Abstract models must directly inherit from parent " 254 "links they refer to." 255 ) 256 parent_links[related_model_tuple] = field 247 257 248 258 # Track fields inherited from base models. 249 259 inherited_attributes = set()
And the suited failed on these models which git-blame
led me to #20883 (163a34ce4bc1086b346a52c7271f48d2c207f710) which makes me believe this either never fully worked (notice the tests are very minimal) or that the latest changes to abstract model inheritance field attribution in #24305 (85ef98dc6ec565b1add417bd76808664e7318026).
Given we have models in the suite I think we should at least bisect to determine if this was ever properly supported but so minimally tested that #24305 silently broke it. If it never truly worked then I'd suggest we add this TypeError
otherwise if it was broken by #24305 we should consider it like a long standing regression.
comment:5 by , 23 hours ago
Hello Simon, thank you for your response.
The re-organization of the models isnt really a solution, because in my case the abstracts are in a reusable app and then models are project specific. I dont like to have concrete models in a reusable app and never be able to rewrite them, but I need the AbstractPageType
in the reusable app already.
I would really like if it was possible to define the parent_link field's name in the abstract, so I don't have to define it on all PageType models, because page
is much more comfortable to work with than page_ptr
.
Hello BeryCZ, thank you for this report. I've analyzed your models and I fear that the issue is the complex inheritance that you have proposed. Specifically, you cannot have a model (
ArticlePage
) that both inherits fromPage
and has aOneToOneField
toPage
. Without understanding your use case, it feels like a possible solution would be for your "pages" to have a FK to "page types" (instead of the other way around with the 1-1 field).Your model definitions create an inheritance conflict:
AbstractPageType
expects a separatePage
instance (this means every model that inherits fromAbstractPageType
must have a separatePage
instance linked via thepage
field).ArticlePage
is also inheritingPage
, which means Django expectsArticlePage
itself to be aPage
instance.Django sees two competing ways to resolve
ArticlePage
:ArticlePage
is aPage
(due to model inheritance), orArticlePage
has a separatePage
instance (due to the pageOneToOneField
).The error is caused because Django tries to retrieve the
mtime
field, which exists onPage
(inherited viaAbstractPage
). But becauseArticlePage
also has aOneToOneField
calledpage
, Django gets confused about how to resolvemtime
. Instead of treatingpage
as a relationship to anotherPage
instance, Django interprets it as a string.Solutions are either to remove the direct inheritance to
Page
or remove thepage
field fromAbstractPageType
.If you have further questions, there are several user support channels available. Please refer to TicketClosingReasons/UseSupportChannels for ways to get help.