Opened 6 years ago
Closed 5 years ago
#29998 closed Bug (fixed)
pk setup for MTI to parent get confused by multiple OneToOne references.
Reported by: | Mārtiņš Šulcs | Owned by: | Mariusz Felisiak |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Can Sarıgöl | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
class Document(models.Model): pass class Picking(Document): document_ptr = models.OneToOneField(Document, on_delete=models.CASCADE, parent_link=True, related_name='+') origin = models.OneToOneField(Document, related_name='picking', on_delete=models.PROTECT)
produces django.core.exceptions.ImproperlyConfigured: Add parent_link=True to appname.Picking.origin.
class Picking(Document): origin = models.OneToOneField(Document, related_name='picking', on_delete=models.PROTECT) document_ptr = models.OneToOneField(Document, on_delete=models.CASCADE, parent_link=True, related_name='+')
Works
First issue is that order seems to matter?
Even if ordering is required "by design"(It shouldn't be we have explicit parent_link marker) shouldn't it look from top to bottom like it does with managers and other things?
Change History (12)
follow-up: 2 comment:1 by , 6 years ago
Triage Stage: | Unreviewed → Accepted |
---|
follow-up: 3 comment:2 by , 6 years ago
Replying to Simon Charette:
It does go away primary_key.
Why is parent_link even necessary in that case? Having pk OneToOne with to MTI child should imply it's parent link.
comment:3 by , 6 years ago
Replying to Mārtiņš Šulcs:
Replying to Simon Charette:
It does go away primary_key.
Why is parent_link even necessary in that case? Having pk OneToOne with to MTI child should imply it's parent link.
I have an update. The warning goes away with primary_key, but model itself is still broken(complains about document_ptr_id not populated) unless field order is correct.
comment:4 by , 6 years ago
Been able to replicate this bug with a simpler case:
class Document(models.Model): pass class Picking(Document): some_unrelated_document = models.OneToOneField(Document, related_name='something', on_delete=models.PROTECT)
Produces the same error against some_unrelated_document.
comment:5 by , 6 years ago
Cc: | added |
---|
Hi, Could this approach be appropriate? parent_links
's params can be base and related class instance therefore just last sample columns are added into parent_links.
We can extend key logic with related_name, e.g ('app', 'document', 'picking').
Or using this method, we can always ensure that the parent_link=True
field is guaranteed into self.parents
.
+++ b/django/db/models/base.py @@ -196,10 +196,11 @@ class ModelBase(type): if base != new_class and not base._meta.abstract: continue # Locate OneToOneField instances. - for field in base._meta.local_fields: - if isinstance(field, OneToOneField): - related = resolve_relation(new_class, field.remote_field.model) - parent_links[make_model_tuple(related)] = field + fields = [field for field in base._meta.local_fields if isinstance(field, OneToOneField)] + for field in sorted(fields, key=lambda x: x.remote_field.parent_link, reverse=True): + related_key = make_model_tuple(resolve_relation(new_class, field.remote_field.model)) + if related_key not in parent_links: + parent_links[related_key] = field # Track fields inherited from base models. inherited_attributes = set()
comment:6 by , 5 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:11 by , 5 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
That seems to be a bug, managed to reproduce.
Does the error go away if you add
primary_key=True
todocument_ptr
like I assume you wanted to do? This makes me realized that the MTI documentation is not completely correcy, the automatically added `place_ptr` field end up with `primary_key=True`.Not sure why we're not checking
and field.remote_field.parent_link
on parent links connection.