[Django] #29260: Django makes an extra UPDATE query when custom PK is evaluating before save.
Django
save.
-------------------------------------+-------------------------------------
Reporter: user0007 | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 2.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Using 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 model's manager method:
{{{
>> Account.objects.create(title="abc")
1. INSERT INTO "app_account" ("title", "id") VALUES (\'abc\', \'3d8c1b3c-
214a-4798-a0fa-d4c22c2b877f\'::uuid)
}}}
Related issue? https://code.djangoproject.com/ticket/29129
--
Ticket URL: <https://code.djangoproject.com/ticket/29260>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
save.
-------------------------------------+-------------------------------------
Reporter: user0007 | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> Using 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 model's manager method:
>
> {{{
> >> Account.objects.create(title="abc")
>
> 1. INSERT INTO "app_account" ("title", "id") VALUES (\'abc\', \'3d8c1b3c-
> 214a-4798-a0fa-d4c22c2b877f\'::uuid)
> }}}
>
> Related issue? https://code.djangoproject.com/ticket/29129
New description:
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
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:1>
Django
save.
-------------------------------------+-------------------------------------
Reporter: user0007 | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (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
[https://docs.djangoproject.com/en/stable/ref/models/instances/#how-
django-knows-to-update-vs-insert as documented] an `UPDATE` is tried
([https://github.com/django/django/blob/4554f9a783665d64b59c35b53ae71178e56ac783/django/db/models/base.py#L804-L805
code]). A fix might try to detect if the primary key came from a `default`
and if so, skip the update.
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:2>
Django
save.
-------------------------------------+-------------------------------------
Reporter: user0007 | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (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.
{{{#!python
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:3>
Django
has a default
-------------------------------------+-------------------------------------
Reporter: user0007 | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:4>
Django
that has a default
-------------------------------------+-------------------------------------
Reporter: user0007 | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:5>
Django
that has a default
Reporter: user0007 | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Windson yang):
@Tim Graham, should we still work on it?
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:6>
Django
that has a default
Reporter: user0007 | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
Simon's proposal seems fine to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:7>
Django
that has a default
-------------------------------------+-------------------------------------
Reporter: user0007 | Owner:
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => candypoplatte
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:8>
Django
that has a default
-------------------------------------+-------------------------------------
Reporter: user0007 | Owner:
| candypoplatte
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:9>
Django
that has a default
-------------------------------------+-------------------------------------
| Ramezani
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: HyunTae Hwang => Hasan Ramezani
* has_patch: 0 => 1
Comment:
PR[https://github.com/django/django/pull/11680]
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:10>
Django
that has a default
-------------------------------------+-------------------------------------
Reporter: user0007 | Owner: Hasan
| Ramezani
Component: Database layer | Version: 2.0
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"85458e94e38c20e57939947ee515a1a53689659f" 85458e9]:
{{{
#!CommitTicketReference repository=""
revision="85458e94e38c20e57939947ee515a1a53689659f"
Fixed #29260 -- Skipped an UPDATE when adding a model instance with
primary key that has a default.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:11>
Django
that has a default
-------------------------------------+-------------------------------------
Reporter: user0007 | Owner: Hasan
| Ramezani
Type: Bug | Status: closed
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* version: 2.0 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:12>
Django
that has a default
-------------------------------------+-------------------------------------
Reporter: user0007 | Owner: Hasan
| Ramezani
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"9e14bc2135cb806b66374bac791c79344fff4ded" 9e14bc2]:
{{{
#!CommitTicketReference repository=""
revision="9e14bc2135cb806b66374bac791c79344fff4ded"
Refs #29260 -- Doc'd Model.save() behavior change in Django 3.0.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:13>
Django
that has a default
-------------------------------------+-------------------------------------
Reporter: user0007 | Owner: Hasan
| Ramezani
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"a04e6fb3557de79826d946c34bde6d049fd45c55" a04e6fb3]:
{{{
#!CommitTicketReference repository=""
revision="a04e6fb3557de79826d946c34bde6d049fd45c55"
[3.0.x] Refs #29260 -- Doc'd Model.save() behavior change in Django 3.0.
Backport of 9e14bc2135cb806b66374bac791c79344fff4ded from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29260#comment:14>