Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Some improvements to niwibe's update_fields patch

The improvements are mostly stylistic cleanup. The non-stylistic
changes are:
  - update_fields can be any iterable. It will be converted to
    ImmutableSet internally so that signals can never alter it.
  - update_fields implies forced update, but not force_update. This
    is technically important, as force_update would result in updates
    even if there were no fields to update (possible in multitable
    situations).
  - Tests added for the above (and also otherwise).
  - Minor doc changes (mainly addition to releases/1.5.txt)
  • Loading branch information...
commit 99cee28b94b6900333fb69be3cfbb903469dda37 1 parent f1f36a0
@akaariai akaariai authored
View
50 django/db/models/base.py
@@ -2,6 +2,7 @@
import sys
from functools import update_wrapper
from itertools import izip
+from sets import ImmutableSet
import django.db.models.manager # Imported to register signal handler.
from django.conf import settings
@@ -11,7 +12,7 @@
from django.db.models.fields import AutoField, FieldDoesNotExist
from django.db.models.fields.related import (ManyToOneRel,
OneToOneField, add_lazy_relation)
-from django.db import (connections, router, transaction, DatabaseError,
+from django.db import (router, transaction, DatabaseError,
DEFAULT_DB_ALIAS)
from django.db.models.query import Q
from django.db.models.query_utils import DeferredAttribute
@@ -458,31 +459,29 @@ def save(self, force_insert=False, force_update=False, using=None, update_fields
that the "save" must be an SQL insert or update (or equivalent for
non-SQL backends), respectively. Normally, they should not be set.
"""
- if force_insert and force_update:
+ if force_insert and (force_update or update_fields):
raise ValueError("Cannot force both insert and updating in model saving.")
if update_fields is not None:
- if not isinstance(update_fields, (list, tuple)):
- raise ValueError("update_fields must be a list or tuple")
-
- # if update_fields is empty, does nothink
+ # If update_fields is empty, skip the save. We do also check for
+ # no-op saves later on for inheritance cases. This bailout is
+ # still needed for skipping signal sending.
if len(update_fields) == 0:
return
+ update_fields = ImmutableSet(update_fields)
- field_names = self._meta.get_all_field_names()
- for field_name in update_fields:
- if field_name not in field_names:
- raise ValueError("%s field does not exist in this model" % (field_name))
-
- force_update = True
-
- self.save_base(using=using, force_insert=force_insert,
- force_update=force_update, update_fields=update_fields)
+ field_names = set(self._meta.get_all_field_names())
+ not_model_fields = update_fields.difference(field_names)
+ if not_model_fields:
+ raise ValueError("The following fields do not exist in this model: %s"
+ % ', '.join(not_model_fields))
+ self.save_base(using=using, force_insert=force_insert,
+ force_update=force_update, update_fields=update_fields)
save.alters_data = True
def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
- force_update=False, using=None, update_fields=None):
+ force_update=False, using=None, update_fields=None):
"""
Does the heavy-lifting involved in saving. Subclasses shouldn't need to
override this method. It's separate from save() in order to hide the
@@ -490,7 +489,8 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
('raw', 'cls', and 'origin').
"""
using = using or router.db_for_write(self.__class__, instance=self)
- assert not (force_insert and force_update)
+ assert not (force_insert and (force_update or update_fields))
+ assert not update_fields or len(update_fields) > 0
if cls is None:
cls = self.__class__
meta = cls._meta
@@ -501,7 +501,7 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
if origin and not meta.auto_created:
signals.pre_save.send(sender=origin, instance=self, raw=raw, using=using,
- update_fields=update_fields)
+ update_fields=update_fields)
# If we are in a raw save, save the object exactly as presented.
# That means that we don't try to be smart about saving attributes
@@ -531,8 +531,8 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
if not meta.proxy:
non_pks = [f for f in meta.local_fields if not f.primary_key]
- if update_fields:
- non_pks = [f for f in non_pks if f.name in update_fields]
+ if update_fields:
+ non_pks = [f for f in non_pks if f.name in update_fields]
# First, try an UPDATE. If that doesn't update anything, do an INSERT.
pk_val = self._get_pk_val(meta)
@@ -541,7 +541,7 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
manager = cls._base_manager
if pk_set:
# Determine whether a record with the primary key already exists.
- if (force_update or (not force_insert and
+ if ((force_update or update_fields) or (not force_insert and
manager.using(using).filter(pk=pk_val).exists())):
# It does already exist, so do an UPDATE.
if force_update or non_pks:
@@ -550,6 +550,8 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
rows = manager.using(using).filter(pk=pk_val)._update(values)
if force_update and not rows:
raise DatabaseError("Forced update did not affect any rows.")
+ if update_fields and not rows:
+ raise DatabaseError("Save with update_fields did not affect any rows.")
else:
record_exists = False
if not pk_set or not record_exists:
@@ -562,7 +564,7 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
fields = meta.local_fields
if not pk_set:
- if force_update:
+ if force_update or update_fields:
raise ValueError("Cannot force an update in save() with no primary key.")
fields = [f for f in fields if not isinstance(f, AutoField)]
@@ -582,8 +584,8 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
# Signal that the save is complete
if origin and not meta.auto_created:
- signals.post_save.send(sender=origin, instance=self, created=(not record_exists),
- update_fields=update_fields, raw=raw, using=using)
+ signals.post_save.send(sender=origin, instance=self, created=(not record_exists),
+ update_fields=update_fields, raw=raw, using=using)
save_base.alters_data = True
View
17 docs/ref/models/instances.txt
@@ -337,18 +337,21 @@ For more details, see the documentation on :ref:`F() expressions
Specifying which fields to save
-------------------------------
-If ``save()`` is passed a list of field names as keyword argument ``update_fields``,
-only the fields named in that list will be saved to the database. This may be
-desirable if you want to update just one or a few fields on an object, as there
-will be a slight performance benefit from preventing all of the model fields
-from being updated in the database. For example:
+.. versionadded:: 1.5
+
+If ``save()`` is passed a list of field names as keyword argument
+``update_fields``, only the fields named in that list will be saved to the
+database. This may be desirable if you want to update just one or a few fields
+on an object, as there will be a slight performance benefit from preventing
+all of the model fields from being updated in the database. For example:
product.name = 'Name changed again'
product.save(update_fields=['name'])
-``update_fields`` must be a list, tuple or None. And this parameter implies ``force_update=True``.
-
+The ``update_fields`` argument can be any iterable containing strings. An
+not None, but empty ``update_fields`` skips the save completely (no signals
+sent). Specifying ``update_fields`` will force an update.
Deleting objects
================
View
4 docs/ref/signals.txt
@@ -124,7 +124,7 @@ Arguments sent with this signal:
The database alias being used.
``update_fields``
- List or tuple of fields to update explicitly specified in the ``save()`` method.
+ List or tuple of fields to update explicitly specified in the ``save()`` method.
``None`` if you do not use this parameter when you call ``save()``.
post_save
@@ -159,7 +159,7 @@ Arguments sent with this signal:
The database alias being used.
``update_fields``
- List or tuple of fields to update explicitly specified in the ``save()`` method.
+ List or tuple of fields to update explicitly specified in the ``save()`` method.
``None`` if you do not use this parameter when you call ``save()``.
pre_delete
View
4 docs/releases/1.5.txt
@@ -41,6 +41,10 @@ Django 1.5 also includes several smaller improvements worth noting:
* The template engine now interprets ``True``, ``False`` and ``None`` as the
corresponding Python objects.
+* :meth:`Model.save() <django.db.models.Model.save()>` has a new argument
+ ``update_fields``. Using this argument allows restricting the set of fields
+ to update when saving model instances.
+
Backwards incompatible changes in 1.5
=====================================
View
2  tests/modeltests/update_only_fields/models.py
@@ -15,7 +15,7 @@ def __unicode__(self):
class Employee(Person):
employee_num = models.IntegerField(default=0)
- profile = models.ForeignKey('Profile', related_name='profiles')
+ profile = models.ForeignKey('Profile', related_name='profiles', null=True)
class Profile(models.Model):
View
63 tests/modeltests/update_only_fields/tests.py
@@ -1,11 +1,11 @@
from __future__ import absolute_import
-from __future__ import with_statement
from django.test import TestCase
+from django.db.models.signals import pre_save, post_save
from .models import Person, Employee, Profile
class UpdateOnlyFieldsTests(TestCase):
- def test_simple_update_fields(self):
+ def test_update_fields_basic(self):
s = Person.objects.create(name='Sara', gender='F')
self.assertEqual(s.gender, 'F')
@@ -17,7 +17,7 @@ def test_simple_update_fields(self):
self.assertEqual(s.gender, 'F')
self.assertEqual(s.name, 'Ian')
- def test_update_field_with_inherited(self):
+ def test_update_fields_inheritance(self):
profile_boss = Profile.objects.create(name='Boss', salary=3000)
profile_receptionist = Profile.objects.create(name='Receptionist', salary=1000)
@@ -41,7 +41,25 @@ def test_update_field_with_inherited(self):
self.assertEqual(e3.name, 'Ian')
self.assertEqual(e3.profile, profile_receptionist)
- def test_update_field_with_incorrect_params(self):
+ def test_update_fields_signals(self):
+ p = Person.objects.create(name='Sara', gender='F')
+ pre_save_data = []
+ def pre_save_receiver(**kwargs):
+ pre_save_data.append(kwargs['update_fields'])
+ pre_save.connect(pre_save_receiver)
+ post_save_data = []
+ def post_save_receiver(**kwargs):
+ post_save_data.append(kwargs['update_fields'])
+ post_save.connect(post_save_receiver)
+ p.save(update_fields=['name'])
+ self.assertEqual(len(pre_save_data), 1)
+ self.assertEqual(len(pre_save_data[0]), 1)
+ self.assertTrue('name' in pre_save_data[0])
+ self.assertEqual(len(post_save_data), 1)
+ self.assertEqual(len(post_save_data[0]), 1)
+ self.assertTrue('name' in post_save_data[0])
+
+ def test_update_fields_incorrect_params(self):
s = Person.objects.create(name='Sara', gender='F')
with self.assertRaises(ValueError):
@@ -50,8 +68,41 @@ def test_update_field_with_incorrect_params(self):
with self.assertRaises(ValueError):
s.save(update_fields="name")
- def test_num_querys_on_save_with_empty_update_fields(self):
+ def test_empty_update_fields(self):
s = Person.objects.create(name='Sara', gender='F')
-
+ pre_save_data = []
+ def pre_save_receiver(**kwargs):
+ pre_save_data.append(kwargs['update_fields'])
+ pre_save.connect(pre_save_receiver)
+ post_save_data = []
+ def post_save_receiver(**kwargs):
+ post_save_data.append(kwargs['update_fields'])
+ post_save.connect(post_save_receiver)
+ # Save is skipped.
with self.assertNumQueries(0):
s.save(update_fields=[])
+ # Signals were skipped, too...
+ self.assertEqual(len(pre_save_data), 0)
+ self.assertEqual(len(post_save_data), 0)
+
+ def test_num_queries_inheritance(self):
+ s = Employee.objects.create(name='Sara', gender='F')
+ s.employee_num = 1
+ s.name = 'Emily'
+ with self.assertNumQueries(1):
+ s.save(update_fields=['employee_num'])
+ s = Employee.objects.get(pk=s.pk)
+ self.assertEqual(s.employee_num, 1)
+ self.assertEqual(s.name, 'Sara')
+ s.employee_num = 2
+ s.name = 'Emily'
+ with self.assertNumQueries(1):
+ s.save(update_fields=['name'])
+ s = Employee.objects.get(pk=s.pk)
+ self.assertEqual(s.name, 'Emily')
+ self.assertEqual(s.employee_num, 1)
+ # A little sanity check that we actually did updates...
+ self.assertEqual(Employee.objects.count(), 1)
+ self.assertEqual(Person.objects.count(), 1)
+ with self.assertNumQueries(2):
+ s.save(update_fields=['name', 'employee_num'])
Please sign in to comment.
Something went wrong with that request. Please try again.