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

Change History (0)

Note: See TracTickets for help on using tickets.
Back to Top