Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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
  • Loading branch information...
commit e973ee6a9879969b8ae05bb7ff681172cc5386a5 1 parent 13be3bf
Anssi Kääriäinen authored August 30, 2013
18  django/db/models/base.py
@@ -667,7 +667,9 @@ def _save_table(self, raw=False, cls=None, force_insert=False,
667 667
             base_qs = cls._base_manager.using(using)
668 668
             values = [(f, None, (getattr(self, f.attname) if raw else f.pre_save(self, False)))
669 669
                       for f in non_pks]
670  
-            updated = self._do_update(base_qs, using, pk_val, values, update_fields)
  670
+            forced_update = update_fields or force_update
  671
+            updated = self._do_update(base_qs, using, pk_val, values, update_fields,
  672
+                                      forced_update)
671 673
             if force_update and not updated:
672 674
                 raise DatabaseError("Forced update did not affect any rows.")
673 675
             if update_fields and not updated:
@@ -691,21 +693,27 @@ def _save_table(self, raw=False, cls=None, force_insert=False,
691 693
                 setattr(self, meta.pk.attname, result)
692 694
         return updated
693 695
 
694  
-    def _do_update(self, base_qs, using, pk_val, values, update_fields):
  696
+    def _do_update(self, base_qs, using, pk_val, values, update_fields, forced_update):
695 697
         """
696 698
         This method will try to update the model. If the model was updated (in
697 699
         the sense that an update query was done and a matching row was found
698 700
         from the DB) the method will return True.
699 701
         """
  702
+        filtered = base_qs.filter(pk=pk_val)
700 703
         if not values:
701 704
             # We can end up here when saving a model in inheritance chain where
702 705
             # update_fields doesn't target any field in current model. In that
703 706
             # case we just say the update succeeded. Another case ending up here
704 707
             # is a model with just PK - in that case check that the PK still
705 708
             # exists.
706  
-            return update_fields is not None or base_qs.filter(pk=pk_val).exists()
707  
-        else:
708  
-            return base_qs.filter(pk=pk_val)._update(values) > 0
  709
+            return update_fields is not None or filtered.exists()
  710
+        if self._meta.select_on_save and not forced_update:
  711
+            if filtered.exists():
  712
+                filtered._update(values)
  713
+                return True
  714
+            else:
  715
+                return False
  716
+        return filtered._update(values) > 0
709 717
 
710 718
     def _do_insert(self, manager, using, fields, update_pk, raw):
711 719
         """
4  django/db/models/options.py
@@ -22,7 +22,8 @@
22 22
                  'unique_together', 'permissions', 'get_latest_by',
23 23
                  'order_with_respect_to', 'app_label', 'db_tablespace',
24 24
                  'abstract', 'managed', 'proxy', 'swappable', 'auto_created',
25  
-                 'index_together', 'app_cache', 'default_permissions')
  25
+                 'index_together', 'app_cache', 'default_permissions',
  26
+                 'select_on_save')
26 27
 
27 28
 @python_2_unicode_compatible
28 29
 class Options(object):
@@ -35,6 +36,7 @@ def __init__(self, meta, app_label=None):
35 36
         self.ordering = []
36 37
         self.unique_together = []
37 38
         self.index_together = []
  39
+        self.select_on_save = False
38 40
         self.default_permissions = ('add', 'change', 'delete')
39 41
         self.permissions = []
40 42
         self.object_name, self.app_label = None, app_label
17  docs/ref/models/instances.txt
@@ -305,16 +305,23 @@ follows this algorithm:
305 305
 * If the object's primary key attribute is *not* set or if the ``UPDATE``
306 306
   didn't update anything, Django executes an ``INSERT``.
307 307
 
308  
-.. versionchanged:: 1.6
309  
-
310  
-    Previously Django used ``SELECT`` - if not found ``INSERT`` else ``UPDATE``
311  
-    algorithm. The old algorithm resulted in one more query in ``UPDATE`` case.
312  
-
313 308
 The one gotcha here is that you should be careful not to specify a primary-key
314 309
 value explicitly when saving new objects, if you cannot guarantee the
315 310
 primary-key value is unused. For more on this nuance, see `Explicitly specifying
316 311
 auto-primary-key values`_ above and `Forcing an INSERT or UPDATE`_ below.
317 312
 
  313
+.. versionchanged:: 1.6
  314
+
  315
+    Previously Django did a ``SELECT`` when the primary key attribute was set.
  316
+    If the ``SELECT`` found a row, then Django did an ``UPDATE``, otherwise it
  317
+    did an ``INSERT``. The old algorithm results in one more query in the
  318
+    ``UPDATE`` case. There are some rare cases where the database doesn't
  319
+    report that a row was updated even if the database contains a row for the
  320
+    object's primary key value. An example is the PostgreSQL ``ON UPDATE``
  321
+    trigger which returns ``NULL``. In such cases it is possible to revert to the
  322
+    old algorithm by setting the :attr:`~django.db.models.Options.select_on_save`
  323
+    option to ``True``.
  324
+
318 325
 .. _ref-models-force-insert:
319 326
 
320 327
 Forcing an INSERT or UPDATE
22  docs/ref/models/options.txt
@@ -256,6 +256,28 @@ Django quotes column and table names behind the scenes.
256 256
     If ``proxy = True``, a model which subclasses another model will be treated as
257 257
     a :ref:`proxy model <proxy-models>`.
258 258
 
  259
+``select_on_save``
  260
+------------------
  261
+
  262
+.. attribute:: Options.select_on_save
  263
+
  264
+    .. versionadded:: 1.6
  265
+
  266
+    Determines if Django will use the pre-1.6
  267
+    :meth:`django.db.models.Model.save()` algorithm. The old algorithm
  268
+    uses ``SELECT`` to determine if there is an existing row to be updated.
  269
+    The new algorith tries an ``UPDATE`` directly. In some rare cases the
  270
+    ``UPDATE`` of an existing row isn't visible to Django. An example is the
  271
+    PostgreSQL ``ON UPDATE`` trigger which returns ``NULL``. In such cases the
  272
+    new algorithm will end up doing an ``INSERT`` even when a row exists in
  273
+    the database.
  274
+
  275
+    Usually there is no need to set this attribute. The default is
  276
+    ``False``.
  277
+
  278
+    See :meth:`django.db.models.Model.save()` for more about the old and
  279
+    new saving algorithm.
  280
+
259 281
 ``unique_together``
260 282
 -------------------
261 283
 
20  docs/releases/1.6.txt
@@ -138,6 +138,22 @@ A :djadmin:`check` management command was added, enabling you to verify if your
138 138
 current configuration (currently oriented at settings) is compatible with the
139 139
 current version of Django.
140 140
 
  141
+:meth:`Model.save() <django.db.models.Model.save()>` algorithm changed
  142
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  143
+
  144
+The :meth:`Model.save() <django.db.models.Model.save()>` method now
  145
+tries to directly ``UPDATE`` the database if the instance has a primary
  146
+key value. Previously ``SELECT`` was performed to determine if ``UPDATE``
  147
+or ``INSERT`` were needed. The new algorithm needs only one query for
  148
+updating an existing row while the old algorithm needed two. See
  149
+:meth:`Model.save() <django.db.models.Model.save()>` for more details.
  150
+
  151
+In some rare cases the database doesn't report that a matching row was
  152
+found when doing an ``UPDATE``. An example is the PostgreSQL ``ON UPDATE``
  153
+trigger which returns ``NULL``. In such cases it is possible to set
  154
+:attr:`django.db.models.Options.select_on_save` flag to force saving to
  155
+use the old algorithm.
  156
+
141 157
 Minor features
142 158
 ~~~~~~~~~~~~~~
143 159
 
@@ -222,10 +238,6 @@ Minor features
222 238
 * Generic :class:`~django.contrib.gis.db.models.GeometryField` is now editable
223 239
   with the OpenLayers widget in the admin.
224 240
 
225  
-* The :meth:`Model.save() <django.db.models.Model.save()>` will do
226  
-  ``UPDATE`` - if not updated - ``INSERT`` instead of ``SELECT`` - if not
227  
-  found ``INSERT`` else ``UPDATE`` in case the model's primary key is set.
228  
-
229 241
 * The documentation contains a :doc:`deployment checklist
230 242
   </howto/deployment/checklist>`.
231 243
 
5  tests/basic/models.py
@@ -19,6 +19,11 @@ class Meta:
19 19
     def __str__(self):
20 20
         return self.headline
21 21
 
  22
+class ArticleSelectOnSave(Article):
  23
+    class Meta:
  24
+        proxy = True
  25
+        select_on_save = True
  26
+
22 27
 @python_2_unicode_compatible
23 28
 class SelfRef(models.Model):
24 29
     selfref = models.ForeignKey('self', null=True, blank=True,
60  tests/basic/tests.py
@@ -5,6 +5,7 @@
5 5
 
6 6
 from django.core.exceptions import ObjectDoesNotExist, MultipleObjectsReturned
7 7
 from django.db import connections, DEFAULT_DB_ALIAS
  8
+from django.db import DatabaseError
8 9
 from django.db.models.fields import Field, FieldDoesNotExist
9 10
 from django.db.models.manager import BaseManager
10 11
 from django.db.models.query import QuerySet, EmptyQuerySet, ValuesListQuerySet, MAX_GET_RESULTS
@@ -12,7 +13,7 @@
12 13
 from django.utils import six
13 14
 from django.utils.translation import ugettext_lazy
14 15
 
15  
-from .models import Article, SelfRef
  16
+from .models import Article, SelfRef, ArticleSelectOnSave
16 17
 
17 18
 
18 19
 class ModelTest(TestCase):
@@ -806,3 +807,60 @@ def test_manager_methods(self):
806 807
             sorted(BaseManager._get_queryset_methods(QuerySet).keys()),
807 808
             sorted(self.QUERYSET_PROXY_METHODS),
808 809
         )
  810
+
  811
+class SelectOnSaveTests(TestCase):
  812
+    def test_select_on_save(self):
  813
+        a1 = Article.objects.create(pub_date=datetime.now())
  814
+        with self.assertNumQueries(1):
  815
+            a1.save()
  816
+        asos = ArticleSelectOnSave.objects.create(pub_date=datetime.now())
  817
+        with self.assertNumQueries(2):
  818
+            asos.save()
  819
+        with self.assertNumQueries(1):
  820
+            asos.save(force_update=True)
  821
+        Article.objects.all().delete()
  822
+        with self.assertRaises(DatabaseError):
  823
+            with self.assertNumQueries(1):
  824
+                asos.save(force_update=True)
  825
+
  826
+    def test_select_on_save_lying_update(self):
  827
+        """
  828
+        Test that select_on_save works correctly if the database
  829
+        doesn't return correct information about matched rows from
  830
+        UPDATE.
  831
+        """
  832
+        # Change the manager to not return "row matched" for update().
  833
+        # We are going to change the Article's _base_manager class
  834
+        # dynamically. This is a bit of a hack, but it seems hard to
  835
+        # test this properly otherwise. Article's manager, because
  836
+        # proxy models use their parent model's _base_manager.
  837
+
  838
+        orig_class = Article._base_manager.__class__
  839
+
  840
+        class FakeQuerySet(QuerySet):
  841
+            # Make sure the _update method below is in fact called.
  842
+            called = False
  843
+
  844
+            def _update(self, *args, **kwargs):
  845
+                FakeQuerySet.called = True
  846
+                super(FakeQuerySet, self)._update(*args, **kwargs)
  847
+                return 0
  848
+
  849
+        class FakeManager(orig_class):
  850
+            def get_queryset(self):
  851
+                return FakeQuerySet(self.model)
  852
+        try:
  853
+            Article._base_manager.__class__ = FakeManager
  854
+            asos = ArticleSelectOnSave.objects.create(pub_date=datetime.now())
  855
+            with self.assertNumQueries(2):
  856
+                asos.save()
  857
+                self.assertTrue(FakeQuerySet.called)
  858
+            # This is not wanted behaviour, but this is how Django has always
  859
+            # behaved for databases that do not return correct information
  860
+            # about matched rows for UPDATE.
  861
+            with self.assertRaises(DatabaseError):
  862
+                asos.save(force_update=True)
  863
+            with self.assertRaises(DatabaseError):
  864
+                asos.save(update_fields=['pub_date'])
  865
+        finally:
  866
+            Article._base_manager.__class__ = orig_class

0 notes on commit e973ee6

Please sign in to comment.
Something went wrong with that request. Please try again.