Opened 6 years ago
Last modified 6 years ago
#29568 closed Cleanup/optimization
Avoid trying to UPDATE a parent model when its child just got INSERT — at Initial Version
Reported by: | François Dupayrat | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Hello,
While sorting through queries, I noticed something unusual: let's say you have non-abstract models inheritance:
class Parent(models.Model): parent_field = models.BooleanField() class DirectChild(Parent): direct_child_field = models.BooleanField() class IndirectChild(DirectChild): indirect_child_field = models.BooleanField()
When trying to create IndirectChild, I would expect 3 queries to be done:
- INSERT Parent
- INSERT DirectChild
- INSERT IndirectChild
Instead, since they all share the same PK, 5 queries are done:
- INSERT Parent
- UPDATE DirectChild (since it has a PK)
- INSERT DirectChild (because previous UPDATE failed)
- UPDATE IndirectChild (since it has a PK)
- INSERT IndirectChild (because previous UPDATE failed)
I found a fix that worked for me by modifying _save_parents in django/db/models/base.py to return if something was inserted (default to fAlse if there is no parent), and force_insert if the parent was inserted (because if there parent was inserted, the child can't possibly exist).
Being a beginner, I'm really wary of breaking something. Could someone if it's something wanted by Django and check if there is obvious mistake?
Here is the modified method:
def _save_parents(self, cls, using, update_fields): """Save all the parents of cls using values from self.""" meta = cls._meta inserted = False 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: inserted = True # Set the parent's PK value to self. if field: setattr(self, field.attname, self._get_pk_val(parent._meta)) # Since we didn't have an instance of the parent handy set # attname directly, bypassing the descriptor. Invalidate # the related object cache, in case it's been accidentally # populated. A fresh instance will be re-built from the # database if necessary. if field.is_cached(self): field.delete_cached_value(self) return inserted