Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#29260 closed Bug (fixed)

Remove UPDATE query when saving a new model instance with a primary key that has a default

Reported by: user0007 Owned by: Hasan Ramezani
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 user0007)

Using a model's instance:

class Account(models.Model):
    id = models.UUIDField(
        primary_key=True,
        default=uuid.uuid4,
        editable=False
    )
    title = models.TextField()

>> account = Account()
>> account.title = "abc"
>> account.save()

1. UPDATE "app_account" SET "title" = \'\', WHERE "app_account"."id" = \'67c9327d-150e-419f-b493-0c2c59a045c3\'::uuid',
2. INSERT INTO "app_account" ("title", "id") VALUES (\'abc\', \'3d8c1b3c-214a-4798-a0fa-d4c22c2b877f\'::uuid)

Using a model's manager method:

>> Account.objects.create(title="abc")

1. INSERT INTO "app_account" ("title", "id") VALUES (\'abc\', \'3d8c1b3c-214a-4798-a0fa-d4c22c2b877f\'::uuid)

Using a model's instance with force_insert argument:

>> account = Account()
>> account.title = "abc"
>> account.save(force_insert=true)

1. INSERT INTO "app_account" ("title", "id") VALUES (\'abc\', \'3d8c1b3c-214a-4798-a0fa-d4c22c2b877f\'::uuid)

Related issue? https://code.djangoproject.com/ticket/29129

Change History (14)

comment:1 Changed 4 years ago by user0007

Description: modified (diff)

comment:2 Changed 4 years ago by Tim Graham

I'm not sure if the issue can or should be fixed. For the model you gave, an instance will have an id from default=uuid.uuid4, so as documented an UPDATE is tried (code). A fix might try to detect if the primary key came from a default and if so, skip the update.

comment:3 Changed 4 years ago by Simon Charette

A fix might try to detect if the primary key came from a default and if so, skip the update.

I think we could use some kind of self._state.adding and self._meta.pk.default heuristics to automatically set force_insert=True on the last table/leaf child. That would break the following scenario though.

a = Account(pk='known-uuid-pk')
a.title = new_title
a.save()  # expects an UPDATE here.

But I would argue that force_update should be passed in this case.

That wouldn't solve the MTI issue described in #29129 but that would do for this case.

comment:4 Changed 4 years ago by Tim Graham

Summary: Django makes an extra UPDATE query when custom PK is evaluating before save.Remove UPDATE query when saving a new model instance with a primar key that has a default
Triage Stage: UnreviewedAccepted

comment:5 Changed 4 years ago by Tim Graham

Summary: Remove UPDATE query when saving a new model instance with a primar key that has a defaultRemove UPDATE query when saving a new model instance with a primary key that has a default

comment:6 Changed 4 years ago by Windson yang

@Tim Graham, should we still work on it?

comment:7 Changed 4 years ago by Tim Graham

Simon's proposal seems fine to me.

comment:8 Changed 4 years ago by HyunTae Hwang

Owner: changed from nobody to HyunTae Hwang
Status: newassigned

comment:9 Changed 4 years ago by Simon Charette

Closed #29129 as a duplicate because the root of the issue is really the primary key with a default and not MTI.

comment:10 Changed 3 years ago by Hasan Ramezani

Has patch: set
Owner: changed from HyunTae Hwang to Hasan Ramezani

comment:11 Changed 3 years ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 85458e9:

Fixed #29260 -- Skipped an UPDATE when adding a model instance with primary key that has a default.

comment:12 Changed 3 years ago by Mariusz Felisiak

Version: 2.0master

comment:13 Changed 3 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In 9e14bc2:

Refs #29260 -- Doc'd Model.save() behavior change in Django 3.0.

comment:14 Changed 3 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In a04e6fb3:

[3.0.x] Refs #29260 -- Doc'd Model.save() behavior change in Django 3.0.

Backport of 9e14bc2135cb806b66374bac791c79344fff4ded from master

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