Fixed #20988 -- Added model meta option select_on_save #1523

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Member

akaariai commented Aug 28, 2013

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.

Refs #16649

@akaariai akaariai 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.

Refs #16649
f65c814

@timgraham timgraham commented on an outdated diff Aug 28, 2013

docs/ref/models/instances.txt
The one gotcha here is that you should be careful not to specify a primary-key
value explicitly when saving new objects, if you cannot guarantee the
primary-key value is unused. For more on this nuance, see `Explicitly specifying
auto-primary-key values`_ above and `Forcing an INSERT or UPDATE`_ below.
+.. versionchanged:: 1.6
+
+ Previously Django used to do a ``SELECT`` when the primary key
@timgraham

timgraham Aug 28, 2013

Owner

"Previously" and "used to" is redundant. "Previously Django did a SELECT..."

@timgraham timgraham commented on an outdated diff Aug 28, 2013

docs/ref/models/instances.txt
The one gotcha here is that you should be careful not to specify a primary-key
value explicitly when saving new objects, if you cannot guarantee the
primary-key value is unused. For more on this nuance, see `Explicitly specifying
auto-primary-key values`_ above and `Forcing an INSERT or UPDATE`_ below.
+.. versionchanged:: 1.6
+
+ Previously Django used to do a ``SELECT`` when the primary key
+ attribute was set. If the ``SELECT`` found a row, then Django did an
+ ``UPDATE``, otherwise it did an ``INSERT``. The old algorithm resulted
+ in one more query in ``UPDATE`` case. There are some rare cases where the
@timgraham

timgraham Aug 28, 2013

Owner

the UPDATE case.

@timgraham timgraham commented on an outdated diff Aug 28, 2013

docs/ref/models/instances.txt
The one gotcha here is that you should be careful not to specify a primary-key
value explicitly when saving new objects, if you cannot guarantee the
primary-key value is unused. For more on this nuance, see `Explicitly specifying
auto-primary-key values`_ above and `Forcing an INSERT or UPDATE`_ below.
+.. versionchanged:: 1.6
+
+ Previously Django used to do a ``SELECT`` when the primary key
+ attribute was set. If the ``SELECT`` found a row, then Django did an
+ ``UPDATE``, otherwise it did an ``INSERT``. The old algorithm resulted
+ in one more query in ``UPDATE`` case. There are some rare cases where the
+ database doesn't report that a row was ``UPDATEd`` even if the database
@timgraham

timgraham Aug 28, 2013

Owner

not sure if the mixed case in UPDATEd is a typo or intentional. In any case, I would probably just change it to "updated" without backticks.

@timgraham

timgraham Aug 28, 2013

Owner

elaborating of when this might happen would be helpful. is it specific to particular DBs or something?

@timgraham timgraham commented on an outdated diff Aug 28, 2013

docs/ref/models/instances.txt
The one gotcha here is that you should be careful not to specify a primary-key
value explicitly when saving new objects, if you cannot guarantee the
primary-key value is unused. For more on this nuance, see `Explicitly specifying
auto-primary-key values`_ above and `Forcing an INSERT or UPDATE`_ below.
+.. versionchanged:: 1.6
+
+ Previously Django used to do a ``SELECT`` when the primary key
+ attribute was set. If the ``SELECT`` found a row, then Django did an
+ ``UPDATE``, otherwise it did an ``INSERT``. The old algorithm resulted
+ in one more query in ``UPDATE`` case. There are some rare cases where the
+ database doesn't report that a row was ``UPDATEd`` even if the database
+ contains a row for the object's primary key value. In such cases it
+ is possible to revert to the old algorithm by using
@timgraham

timgraham Aug 28, 2013

Owner

using the

@timgraham timgraham commented on an outdated diff Aug 28, 2013

docs/ref/models/options.txt
@@ -317,3 +317,24 @@ Django quotes column and table names behind the scenes.
verbose_name_plural = "stories"
If this isn't given, Django will use :attr:`~Options.verbose_name` + ``"s"``.
+
+``select_on_save``
@timgraham

timgraham Aug 28, 2013

Owner

the options are sort of alphabetized except when there are related options so I would put this below Options.proxy

@timgraham timgraham commented on an outdated diff Aug 28, 2013

docs/ref/models/options.txt
@@ -317,3 +317,24 @@ Django quotes column and table names behind the scenes.
verbose_name_plural = "stories"
If this isn't given, Django will use :attr:`~Options.verbose_name` + ``"s"``.
+
+``select_on_save``
+------------------
+
+.. attribute:: Options.select_on_save
+
+ .. versionadded:: 1.6
+
+ Determines if Django will use the pre-1.6
+ :meth:`django.db.models.Model.save()` algorithm. The old algorithm
+ used ``SELECT`` to determine if there is an existing row to be updated.
@timgraham

timgraham Aug 28, 2013

Owner

uses SELECT

@timgraham timgraham commented on an outdated diff Aug 28, 2013

docs/ref/models/instances.txt
The one gotcha here is that you should be careful not to specify a primary-key
value explicitly when saving new objects, if you cannot guarantee the
primary-key value is unused. For more on this nuance, see `Explicitly specifying
auto-primary-key values`_ above and `Forcing an INSERT or UPDATE`_ below.
+.. versionchanged:: 1.6
+
+ Previously Django used to do a ``SELECT`` when the primary key
+ attribute was set. If the ``SELECT`` found a row, then Django did an
+ ``UPDATE``, otherwise it did an ``INSERT``. The old algorithm resulted
@timgraham

timgraham Aug 28, 2013

Owner

results in one more query (present tense since the old algorithm still exists)

@timgraham timgraham commented on an outdated diff Aug 28, 2013

docs/ref/models/options.txt
@@ -317,3 +317,24 @@ Django quotes column and table names behind the scenes.
verbose_name_plural = "stories"
If this isn't given, Django will use :attr:`~Options.verbose_name` + ``"s"``.
+
+``select_on_save``
+------------------
+
+.. attribute:: Options.select_on_save
+
+ .. versionadded:: 1.6
+
+ Determines if Django will use the pre-1.6
+ :meth:`django.db.models.Model.save()` algorithm. The old algorithm
+ used ``SELECT`` to determine if there is an existing row to be updated.
+ The new algorith tries an ``UPDATE`` directly. In some rare cases
+ ``UPDATE`` of an existing row isn't visible to Django. In such cases
@timgraham

timgraham Aug 28, 2013

Owner

the UPDATE

@timgraham timgraham commented on the diff Aug 28, 2013

docs/releases/1.6.txt
@@ -138,6 +138,21 @@ A :djadmin:`check` management command was added, enabling you to verify if your
current configuration (currently oriented at settings) is compatible with the
current version of Django.
+:meth:`Model.save() <django.db.models.Model.save()>` algorithm changed
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@timgraham

timgraham Aug 28, 2013

Owner

newline after ~~~~~~ row

@timgraham timgraham commented on an outdated diff Aug 28, 2013

docs/releases/1.6.txt
@@ -138,6 +138,21 @@ A :djadmin:`check` management command was added, enabling you to verify if your
current configuration (currently oriented at settings) is compatible with the
current version of Django.
+:meth:`Model.save() <django.db.models.Model.save()>` algorithm changed
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The :meth:`Model.save() <django.db.models.Model.save()>` method now
+tries to directly ``UPDATE`` the database if the object has a primary
+key value. Previously ``SELECT`` was performed to determine if ``UPDATE``
+or ``INSERT`` was needed. The new algorithm needs only one query for
@timgraham

timgraham Aug 28, 2013

Owner

was -> were

@timgraham timgraham commented on an outdated diff Aug 28, 2013

docs/releases/1.6.txt
@@ -138,6 +138,21 @@ A :djadmin:`check` management command was added, enabling you to verify if your
current configuration (currently oriented at settings) is compatible with the
current version of Django.
+:meth:`Model.save() <django.db.models.Model.save()>` algorithm changed
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The :meth:`Model.save() <django.db.models.Model.save()>` method now
+tries to directly ``UPDATE`` the database if the object has a primary
+key value. Previously ``SELECT`` was performed to determine if ``UPDATE``
+or ``INSERT`` was needed. The new algorithm needs only one query for
+updating an existing row while the old algorithm needed two. See
+:meth:`Model.save() <django.db.models.Model.save()>` for more details.
+
+In some rare cases the database doesn't report that a matching row was
+found when doing an ``UPDATE``. An example is PostgreSQL ON UPDATE
@timgraham

timgraham Aug 28, 2013

Owner

add `` for ON UPDATE? include this example in the docs as well

@timgraham timgraham commented on an outdated diff Aug 28, 2013

docs/releases/1.6.txt
@@ -138,6 +138,21 @@ A :djadmin:`check` management command was added, enabling you to verify if your
current configuration (currently oriented at settings) is compatible with the
current version of Django.
+:meth:`Model.save() <django.db.models.Model.save()>` algorithm changed
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The :meth:`Model.save() <django.db.models.Model.save()>` method now
+tries to directly ``UPDATE`` the database if the object has a primary
+key value. Previously ``SELECT`` was performed to determine if ``UPDATE``
+or ``INSERT`` was needed. The new algorithm needs only one query for
+updating an existing row while the old algorithm needed two. See
+:meth:`Model.save() <django.db.models.Model.save()>` for more details.
+
+In some rare cases the database doesn't report that a matching row was
+found when doing an ``UPDATE``. An example is PostgreSQL ON UPDATE
+trigger which returns NULL. In such cases it is possible to set
@timgraham

timgraham Aug 28, 2013

Owner

`` for NULL?

@timgraham timgraham commented on the diff Aug 29, 2013

docs/ref/models/instances.txt
The one gotcha here is that you should be careful not to specify a primary-key
value explicitly when saving new objects, if you cannot guarantee the
primary-key value is unused. For more on this nuance, see `Explicitly specifying
auto-primary-key values`_ above and `Forcing an INSERT or UPDATE`_ below.
+.. versionchanged:: 1.6
+
+ Previously Django did a ``SELECT`` when the primary key attribute was set.
+ If the ``SELECT`` found a row, then Django did an ``UPDATE``, otherwise it
+ did an ``INSERT``. The old algorithm results in one more query in the
+ ``UPDATE`` case. There are some rare cases where the database doesn't
+ report that a row was updated even if the database contains a row for the
+ object's primary key value. An example is PostgreSQL ``ON UPDATE`` trigger
@timgraham

timgraham Aug 29, 2013

Owner

the PostgreSQL

@timgraham timgraham commented on the diff Aug 29, 2013

docs/ref/models/instances.txt
The one gotcha here is that you should be careful not to specify a primary-key
value explicitly when saving new objects, if you cannot guarantee the
primary-key value is unused. For more on this nuance, see `Explicitly specifying
auto-primary-key values`_ above and `Forcing an INSERT or UPDATE`_ below.
+.. versionchanged:: 1.6
+
+ Previously Django did a ``SELECT`` when the primary key attribute was set.
+ If the ``SELECT`` found a row, then Django did an ``UPDATE``, otherwise it
+ did an ``INSERT``. The old algorithm results in one more query in the
+ ``UPDATE`` case. There are some rare cases where the database doesn't
+ report that a row was updated even if the database contains a row for the
+ object's primary key value. An example is PostgreSQL ``ON UPDATE`` trigger
+ which returns ``NULL``. In such cases it is possible to revert to the
+ old algorithm by using the :attr:`~django.db.models.Options.select_on_save`
@timgraham

timgraham Aug 29, 2013

Owner

by setting the select_on_save option to True

@timgraham timgraham commented on the diff Aug 29, 2013

docs/ref/models/options.txt
@@ -256,6 +256,27 @@ Django quotes column and table names behind the scenes.
If ``proxy = True``, a model which subclasses another model will be treated as
a :ref:`proxy model <proxy-models>`.
+``select_on_save``
+------------------
+
+.. attribute:: Options.select_on_save
+
+ .. versionadded:: 1.6
+
+ Determines if Django will use the pre-1.6
+ :meth:`django.db.models.Model.save()` algorithm. The old algorithm
+ uses ``SELECT`` to determine if there is an existing row to be updated.
+ The new algorith tries an ``UPDATE`` directly. In some rare cases the
+ ``UPDATE`` of an existing row isn't visible to Django. In such cases
@timgraham

timgraham Aug 29, 2013

Owner

include trigger example here

@timgraham timgraham commented on the diff Aug 29, 2013

docs/releases/1.6.txt
@@ -138,6 +138,22 @@ A :djadmin:`check` management command was added, enabling you to verify if your
current configuration (currently oriented at settings) is compatible with the
current version of Django.
+:meth:`Model.save() <django.db.models.Model.save()>` algorithm changed
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The :meth:`Model.save() <django.db.models.Model.save()>` method now
+tries to directly ``UPDATE`` the database if the instance has a primary
+key value. Previously ``SELECT`` was performed to determine if ``UPDATE``
+or ``INSERT`` were needed. The new algorithm needs only one query for
+updating an existing row while the old algorithm needed two. See
+:meth:`Model.save() <django.db.models.Model.save()>` for more details.
+
+In some rare cases the database doesn't report that a matching row was
+found when doing an ``UPDATE``. An example is PostgreSQL ``ON UPDATE``
@timgraham

timgraham Aug 29, 2013

Owner

the PostgreSQL

Owner

timgraham commented Aug 29, 2013

LGTM besides those minor comments.

Member

mjtamlyn commented Aug 30, 2013

Looks like this got merged in e973ee6

mjtamlyn closed this Aug 30, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment