[Django] #20988: Model.save() refactoring incompatibilities
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Database | Keywords:
layer (models, ORM) | Has patch: 0
Severity: Release | Needs tests: 0
blocker | Easy pickings: 0
Triage Stage: Accepted |
Needs documentation: 0 |
Patch needs improvement: 0 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
Ticket #16649 / commit 6b4834952dcce0db5cbc1534635c00ff8573a6d8 introduced
a new algorithm for saving.
Previously when saving an object with primary key set, Django first did a
SELECT to check if an row existed in the database. If it existed, an
UPDATE was performed, otherwise an INSERT was performed. In Django 1.6
this is change so that a direct UPDATE is done. The database should return
how many rows were found for the UPDATE. If no rows were found, then
Django knows to perform an INSERT. If rows were found, then the UPDATE is
already done. This way reduces one query in the common case where there is
a matching row in the database.
However, there are some rare cases where the database doesn't return "row
found" for UPDATE even if there is a matching row in the DB. One case is
PostgreSQL and ON UPDATE trigger that returns NULL (returning NULL from ON
UPDATE trigger skips the UPDATE operation). There are likely some other
cases, at least when considering all the 3rd party backends.
When the database doesn't return "row found" for UPDATE, the result is
that Django tries to insert and an IntegrityError follows. Thus, those
users who are hit by the above condition simply cannot update existing
objects. Django itself shouldn't ever generate schemas that have this
problem, but again, who knows about third party backends.
It would be really nice to offer some fallback way for users who have the
above problem. One possibility is introduced in
https://github.com/akaariai/django/commit/1ca398b0352b11868ec236f92cf17b6ce82ff88c,
that is Model.Meta option select_on_save that switches the model back to
the old behaviour.
I think at least adding the "forced_update" argument to Model._do_update()
before release candidate would be good. If it turns out later on that we
want "select_on_save" flag, then there is no need to change the
_do_update() signature mid stable release. Even if it is private API that
would be nice for users. In addition, this allows those who have this
problem to override _do_update(), and avoid the problem (private API
override, but still better than "use raw SQL").
In addition release notes should be updated, the current note about this
is too small, and isn't easy to understand if you don't already know what
it means (same thing goes for .save() documentation). This can be done
post RC1.
It is worth considering if adding the "select_on_save" flag should be
done. A possibility is to wait-and-see, and add in mid 1.6 if enough
complaints are risen.
I am marking this as release blocker, RC1 for the _do_update() signature
change, and 1.6 release for the release notes change.
--
Ticket URL: <https://code.djangoproject.com/ticket/20988>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
(models, ORM) | Triage Stage: Accepted
Severity: Release blocker | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by intgr):
* cc: marti@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20988#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
(models, ORM) | Triage Stage: Accepted
Severity: Release blocker | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by akaariai):
Currently adding "select_on_save" flag is the favoured approach. This is
discussed at https://groups.google.com/forum/#!topic/django-
developers/coQ19f1VhTk.
There are pull requests for 1.6 and master for this at
https://github.com/django/django/pull/1522 and
https://github.com/django/django/pull/1523. They should be pretty much
ready-for-checkin at the moment.
I will wait for more feedback for some time. Unless objections are risen I
will commit this one by the end of this week.
--
Ticket URL: <https://code.djangoproject.com/ticket/20988#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Cleanup/optimization | Version: master
Component: Database layer | Resolution: fixed
(models, ORM) | Triage Stage: Accepted
Severity: Release blocker | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"e973ee6a9879969b8ae05bb7ff681172cc5386a5"]:
{{{
#!CommitTicketReference repository=""
revision="e973ee6a9879969b8ae05bb7ff681172cc5386a5"
Fixed #20988 -- Added model meta option select_on_save
The option can be used to force pre 1.6 style SELECT on save behaviour.
This is needed in case the database returns zero updated rows even if
there is a matching row in the DB. One such case is PostgreSQL update
trigger that returns NULL.
Reviewed by Tim Graham.
Refs #16649
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20988#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Cleanup/optimization | Version: master
Component: Database layer | Resolution: fixed
(models, ORM) | Triage Stage: Accepted
Severity: Release blocker | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Anssi Kääriäinen <akaariai@…>):
In [changeset:"76e38a21777243fec58c640d617bb71a251c5ba1"]:
{{{
#!CommitTicketReference repository=""
revision="76e38a21777243fec58c640d617bb71a251c5ba1"
[1.6.x] Fixed #20988 -- Added model meta option select_on_save
The option can be used to force pre 1.6 style SELECT on save behaviour.
This is needed in case the database returns zero updated rows even if
there is a matching row in the DB. One such case is PostgreSQL update
trigger that returns NULL.
Reviewed by Tim Graham.
Refs #16649
Backport of e973ee6a9879969b8ae05bb7ff681172cc5386a5 from master
Conflicts:
django/db/models/options.py
tests/basic/tests.py
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20988#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Cleanup/optimization | Version: master
Component: Database layer | Resolution: fixed
(models, ORM) | Triage Stage: Accepted
Severity: Release blocker | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by intgr):
Thanks for the fix! :)
--
Ticket URL: <https://code.djangoproject.com/ticket/20988#comment:5>