Model.save() refactoring incompatibilities

[Anssi Kääriäinen]

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.

[Anssi Kääriäinen]

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.

[Anssi Kääriäinen <akaariai@…>]

In 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

[Anssi Kääriäinen <akaariai@…>]

In 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

[Marti Raudsepp]

Thanks for the fix! :)