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 Version 2
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 (last modified by )
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 and save_base in django/db/models/base.py: _save_parents return if something was inserted (default to fAlse if there is no parent), and force_insert if the parent was inserted in both methods (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 (without irrelevant code, materialized as [...]):
def _save_parents([...]): [...] inserted = False for parent, field in meta.parents.items(): [...] 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 [...] return inserted def save_base([...]): [...] with transaction.atomic(using=using, savepoint=False): parent_inserted = False if not raw: parent_inserted = self._save_parents(cls, using, update_fields) updated = self._save_table(raw, cls, force_insert or parent_inserted, force_update, using, update_fields) [...]
Change History (2)
comment:1 by , 6 years ago
Description: | modified (diff) |
---|
comment:2 by , 6 years ago
Description: | modified (diff) |
---|