[Django] #27335: Avoid object save during QuerySet.update_or_create when there were no changes
Django
changes
----------------------------------------------+--------------------
Reporter: tonnzor | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Right now when you use QuerySet.update_or_create -- it always saves into
DB even if there were no changes.
It would be great that if nothing is changed -- nothing is saved into DB.
I found myself often reusing the same pattern in our projects.
Use case:
{{{#!python
>>> from django.contrib.auth.models import User
>>> User.objects.create(is_superuser=True, username='admin',
is_active=True) # generate a use in DB to work with
>>> # later in our code
>>> user, created = User.objects.update_or_create(defaults={'is_active':
True}, username='admin')
# here an extra DB write though no data were changed
}}}
If you check the current code in master
(https://github.com/django/django/blob/master/django/db/models/query.py#L477):
{{{#!python
try:
obj = self.select_for_update().get(**lookup)
except self.model.DoesNotExist:
obj, created = self._create_object_from_params(lookup,
params)
if created:
return obj, created
for k, v in six.iteritems(defaults):
setattr(obj, k, v() if callable(v) else v)
obj.save(using=self.db) # HERE IT IS => always save
}}}
If object exists in DB, it would apply all attributes from `default` and
save it even if there were no changes in values.
We could do something like that:
{{{#!python
changed = False
for k, v in six.iteritems(defaults):
v = v() if callable(v) else v
if v != getattr(obj, k):
modified = True
setattr(obj, k, v)
if changed:
obj.save(using=self.db) # save only if there were changes
}}}
If you think it is too complex for all use cases, we may put a flag for
enabling / disabling this feature or have another method for this logic.
--
Ticket URL: <https://code.djangoproject.com/ticket/27335>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
changes
-------------------------------------+-------------------------------------
Reporter: tonnzor | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/27335#comment:1>
Django
changes
-------------------------------------+-------------------------------------
Reporter: tonnzor | Owner: Andrew
Type: | Nester
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Andrew Nester):
* owner: nobody => Andrew Nester
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/27335#comment:2>
Django
changes
-------------------------------------+-------------------------------------
Reporter: tonnzor | Owner: Andrew
Type: | Nester
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Andrew Nester):
Hi @tonnzorr
As discussed in pull request linked to this ticket implementing this
optimization might cause some issues not covered here previously. Some of
them:
1. Handling foreign key changes
2. Regression of auto_now feature
3. Updating models backed by tables with triggers and so on.
My point here is that implementing this feature requires a lot of time
comparing with profit we can gain from it, so it looks like we are trying
to be too smart here and it's a little bit dangerous because this
optimization looks like source of potential regression.
I am just curious to know your opinion about it because my point here is
that it's better to leave behavior of update_or_create as it is now.
--
Ticket URL: <https://code.djangoproject.com/ticket/27335#comment:3>
Django
changes
-------------------------------------+-------------------------------------
Reporter: tonnzor | Owner: Andrew
Type: | Nester
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: assigned => closed
* resolution: => wontfix
Comment:
Closing in absence of further discussion.
--
Ticket URL: <https://code.djangoproject.com/ticket/27335#comment:4>
Django
changes
-------------------------------------+-------------------------------------
Reporter: Artem Skoretskiy | Owner: Andrew
Type: | Nester
Cleanup/optimization | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Rich Rauenzahn):
I'd like to see some kind of solution (or workaround) for this --
particularly because there isn't a workaround that I can see -- except
maybe putting this logic in my models update(), but then it's always
active. I guess a temporary proxy model might work as well.
I have a use case where I am synchronizing two databases with different
schemas as part of a migration. The sync takes a long time when updating
the data even when the data is the same because it's forcing an update for
data that is already synced.
I'm for making the behavior optional, though ...
--
Ticket URL: <https://code.djangoproject.com/ticket/27335#comment:5>