#33414: Diamond inheritance causes duplicated PK error when creating an object,
if
the primary key field has a default.
-------------------------------------+-------------------------------------
Reporter: Yu Li | Owner: Yu Li
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
After a quick investigation it seems like we simply don't have any tests
covering diamond inheritance model saves.
The only instance of diamond inheritance I could find in the test suite
was
[https://github.com/django/django/blob/652c68ffeebd510a6f59e1b56b3e007d07683ad8/tests/model_meta/models.py#L144-L158
in model_meta tests] but this hierarchy of models exists solely for
`Option` testing purposes.
Here's what a tentative patch could look like for this issue, it seems to
be passing the full suite on SQLite at least.
{{{#!diff
diff --git a/django/db/models/base.py b/django/db/models/base.py
index 37f6a3dd58..d363a1ddeb 100644
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -823,21 +823,29 @@ def save_base(self, raw=False, force_insert=False,
save_base.alters_data = True
- def _save_parents(self, cls, using, update_fields):
+ def _save_parents(self, cls, using, update_fields,
saved_parents=None):
"""Save all the parents of cls using values from self."""
meta = cls._meta
inserted = False
+ if saved_parents is None:
+ saved_parents = {}
for parent, field in meta.parents.items():
# Make sure the link fields are synced between parent and
self.
if (field and getattr(self, parent._meta.pk.attname) is None
and
getattr(self, field.attname) is not None):
setattr(self, parent._meta.pk.attname, getattr(self,
field.attname))
- parent_inserted = self._save_parents(cls=parent, using=using,
update_fields=update_fields)
- updated = self._save_table(
- cls=parent, using=using, update_fields=update_fields,
- force_insert=parent_inserted,
- )
- if not updated:
+ if (parent_updated := saved_parents.get(parent)) is None:
+ parent_inserted = self._save_parents(
+ cls=parent, using=using, update_fields=update_fields,
saved_parents=saved_parents,
+ )
+ updated = self._save_table(
+ cls=parent, using=using, update_fields=update_fields,
+ force_insert=parent_inserted,
+ )
+ if not updated:
+ inserted = True
+ saved_parents[parent] = updated
+ elif not parent_updated:
inserted = True
# Set the parent's PK value to self.
if field:
diff --git a/tests/model_inheritance/models.py
b/tests/model_inheritance/models.py
index fa37e121ea..99ce78a0f0 100644
--- a/tests/model_inheritance/models.py
+++ b/tests/model_inheritance/models.py
@@ -175,3 +175,20 @@ class Child(Parent):
class GrandChild(Child):
pass
+
+
+# Diamond inheritance
+class CommonAncestor(models.Model):
+ pass
+
+
+class FirstParent(CommonAncestor):
+ first_ancestor = models.OneToOneField(CommonAncestor, models.CASCADE,
primary_key=True, parent_link=True)
+
+
+class SecondParent(CommonAncestor):
+ second_ancestor = models.OneToOneField(CommonAncestor,
models.CASCADE, primary_key=True, parent_link=True)
+
+
+class CommonChild(FirstParent, SecondParent):
+ pass
diff --git a/tests/model_inheritance/tests.py
b/tests/model_inheritance/tests.py
index 550a297fb3..97fb3e4a78 100644
--- a/tests/model_inheritance/tests.py
+++ b/tests/model_inheritance/tests.py
@@ -7,7 +7,7 @@
from django.test.utils import CaptureQueriesContext, isolate_apps
from .models import (
- Base, Chef, CommonInfo, GrandChild, GrandParent, ItalianRestaurant,
+ Base, Chef, CommonInfo, CommonChild, GrandChild, GrandParent,
ItalianRestaurant,
MixinModel, Parent, ParkingLot, Place, Post, Restaurant, Student,
SubBase,
Supplier, Title, Worker,
)
@@ -150,6 +150,12 @@ def b():
sql = query['sql']
self.assertIn('INSERT INTO', sql, sql)
+ def test_create_diamond_mti(self):
+ with self.assertNumQueries(4):
+ common_child = CommonChild.objects.create()
+ with self.assertNumQueries(4):
+ common_child.save()
+
def test_eq(self):
# Equality doesn't transfer in multitable inheritance.
self.assertNotEqual(Place(id=1), Restaurant(id=1))
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33414#comment:6>
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 view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.9d10edcd6bf61f79f26f6e1f6f2e2b5d%40djangoproject.com.
