Opened 16 months ago

Last modified 9 months ago

#30382 new Bug

force_insert flag is not passed when saving parents on inherited models.

Reported by: Phill Tornroth Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

We're using non-abstract model inheritance (table per model class) and issuing our own primary keys. When saving we pass force_insert=True to prevent the extra UPDATE statement that precedes the INSERT. The force_insert flag is respected on the child table but not on the parent. So given:

class ParentModel(models.Model):
    id = models.BigIntegerField(primary_key=True)

class ChildModel(ParentModel):
    pass

ChildModel(id=1).save(force_insert=True)

We'll see queries:

  1. UPDATE app_parentmodel (no rows affected)
  2. INSERT app_parentmodel
  3. INSERT app_childmodel

This is because Model.save_base doesn't pass force_insert along to Model._save_parents, and onto Model._save_table. Doing so would prevent the extra UPDATE and respect the spirit of the force_insert feature. This is a change I've made locally and seems to work / is pretty straightforward. I'm curious though if there's intent behind not carrying the force_insert functionality through for parents. I couldn't find any discussion on the topic.

For context about why this is particularly problematic in our case -- we're using MySQL w/ Innodb and Innodb will take out different exclusive locks for the UPDATE and INSERT in question -- so if you're creating new ChildModel instances in parallel you can get deadlocks when multiple threads issue query #1 and then need a lock with insert intention in order to get #2.

Change History (3)

comment:1 Changed 16 months ago by felixxm

Summary: force_insert not respected when saving parentsforce_insert flag is not passed when saving parents on inherited models.
Triage Stage: UnreviewedAccepted
Version: 2.2master

Sounds reasonable.

comment:2 Changed 16 months ago by Phill Tornroth

I started working on this. As is normal, it's note quite as easy as I hoped. The manager .create() methods assume force_insert and there's some previous behavior in tests (and so presumably in the wild) where code might do:

parent = ParentModel.objects.create()
child = ChildModel.objects.create(parent=parent)

The create method will pass force_insert, so the second line fails because it attempts to insert the parent again. A prior attempt from @akaariai suggested breaking this behavior (which I'm game for, if the team suggests). I can't think of a reasonable alternative that doesn't involve some other backwards-incompatible change (like making force_insert an explicit argument to create).

Here's the unmerged commit from @akaraaia: https://github.com/akaariai/django/commit/57384c7936fbd8d760a36c47a41ecef18e451308
From the (effectively duplicate) ticket: https://code.djangoproject.com/ticket/18305

comment:3 Changed 9 months ago by KESHAV KUMAR

I want to work on this ticket. Can I get some help about it ?

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