Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Ticket #4102 - Allow UPDATE of only specific fields in model.save() #4

Closed
wants to merge 3 commits into from

2 participants

@niwibe

It's the same patch that is on the ticket. - https://code.djangoproject.com/ticket/4102

(What a joy to have django on github officially)

@akaariai
Collaborator

I will try to work on this ticket/pull request - limited time, unlimited amount of work and all that.

akaariai and others added some commits
@akaariai akaariai 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)
99cee28
@niwibe niwibe Merge pull request #1 from akaariai/ticket_4102_niwibe
Ticket 4102 niwibe
46954de
@akaariai
Collaborator

I sent you a pull request for some updates into your patch, nothing big. I hope you can squash my update and your patch to one commit and update your branch to current Django head.

If you wish I can also do this myself by closing this pull request and opening another one from my repository.

To me it seems this feature is close to commit-ready. I think this one is big enough to require a review in Trac.

@niwibe

It seems right.

@niwibe niwibe closed this
@akaariai
Collaborator

There is still the problem that you need to update your branch to current django/django head. And I would hope you could form a single patch/commit from the above.

Maybe the easiest way forward is to create a patch to Trac, then we could close this pull request and get a review for the patch in Trac, then again a new pull request and hopefully finally merged to django head.

I am not sure what is the best way forward, this git stuff is new to me, too...

@niwibe

No problem, I create the patch and I will go to trac, and I will open another pull request with a single commit.

;)

@mjtamlyn mjtamlyn referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 28, 2012
  1. @niwibe
Commits on May 1, 2012
  1. @akaariai

    Some improvements to niwibe's update_fields patch

    akaariai authored
    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)
  2. @niwibe

    Merge pull request #1 from akaariai/ticket_4102_niwibe

    niwibe authored
    Ticket 4102 niwibe
This page is out of date. Refresh to see the latest.
View
47 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
@@ -449,7 +450,7 @@ def serializable_value(self, field_name):
return getattr(self, field_name)
return getattr(self, field.attname)
- def save(self, force_insert=False, force_update=False, using=None):
+ def save(self, force_insert=False, force_update=False, using=None, update_fields=None):
"""
Saves the current instance. Override this in a subclass if you want to
control the saving process.
@@ -458,14 +459,29 @@ def save(self, force_insert=False, force_update=False, using=None):
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.")
- self.save_base(using=using, force_insert=force_insert, force_update=force_update)
+ if update_fields is not None:
+ # 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 = 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):
+ 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
@@ -473,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
@@ -483,7 +500,8 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
meta = cls._meta
if origin and not meta.auto_created:
- signals.pre_save.send(sender=origin, instance=self, raw=raw, using=using)
+ signals.pre_save.send(sender=origin, instance=self, raw=raw, using=using,
+ 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
@@ -503,7 +521,7 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
if field and getattr(self, parent._meta.pk.attname) is None and getattr(self, field.attname) is not None:
setattr(self, parent._meta.pk.attname, getattr(self, field.attname))
- self.save_base(cls=parent, origin=org, using=using)
+ self.save_base(cls=parent, origin=org, using=using, update_fields=update_fields)
if field:
setattr(self, field.attname, self._get_pk_val(parent._meta))
@@ -513,6 +531,9 @@ 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]
+
# First, try an UPDATE. If that doesn't update anything, do an INSERT.
pk_val = self._get_pk_val(meta)
pk_set = pk_val is not None
@@ -520,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:
@@ -529,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:
@@ -541,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)]
@@ -561,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), 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
4 django/db/models/signals.py
@@ -5,8 +5,8 @@
pre_init = Signal(providing_args=["instance", "args", "kwargs"])
post_init = Signal(providing_args=["instance"])
-pre_save = Signal(providing_args=["instance", "raw", "using"])
-post_save = Signal(providing_args=["instance", "raw", "created", "using"])
+pre_save = Signal(providing_args=["instance", "raw", "using", "update_fields"])
+post_save = Signal(providing_args=["instance", "raw", "created", "using", "update_fields"])
pre_delete = Signal(providing_args=["instance", "using"])
post_delete = Signal(providing_args=["instance", "using"])
View
21 docs/ref/models/instances.txt
@@ -135,7 +135,7 @@ Saving objects
To save an object back to the database, call ``save()``:
-.. method:: Model.save([force_insert=False, force_update=False, using=DEFAULT_DB_ALIAS])
+.. method:: Model.save([force_insert=False, force_update=False, using=DEFAULT_DB_ALIAS, update_fields=None])
.. versionadded:: 1.2
The ``using`` argument was added.
@@ -334,6 +334,25 @@ For more details, see the documentation on :ref:`F() expressions
<query-expressions>` and their :ref:`use in update queries
<topics-db-queries-update>`.
+Specifying which fields to save
+-------------------------------
+
+.. 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'])
+
+
+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
8 docs/ref/signals.txt
@@ -123,6 +123,10 @@ Arguments sent with this signal:
``using``
The database alias being used.
+``update_fields``
+ 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
---------
@@ -154,6 +158,10 @@ Arguments sent with this signal:
``using``
The database alias being used.
+``update_fields``
+ 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
3  tests/modeltests/update_only_fields/__init__.py
@@ -0,0 +1,3 @@
+# -*- coding: utf-8 -*-
+
+
View
26 tests/modeltests/update_only_fields/models.py
@@ -0,0 +1,26 @@
+
+from django.db import models
+
+GENDER_CHOICES = (
+ ('M', 'Male'),
+ ('F', 'Female'),
+)
+
+class Person(models.Model):
+ name = models.CharField(max_length=20)
+ gender = models.CharField(max_length=1, choices=GENDER_CHOICES)
+
+ def __unicode__(self):
+ return self.name
+
+class Employee(Person):
+ employee_num = models.IntegerField(default=0)
+ profile = models.ForeignKey('Profile', related_name='profiles', null=True)
+
+
+class Profile(models.Model):
+ name = models.CharField(max_length=200)
+ salary = models.FloatField(default=1000.0)
+
+ def __unicode__(self):
+ return self.name
View
108 tests/modeltests/update_only_fields/tests.py
@@ -0,0 +1,108 @@
+from __future__ import absolute_import
+
+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_update_fields_basic(self):
+ s = Person.objects.create(name='Sara', gender='F')
+ self.assertEqual(s.gender, 'F')
+
+ s.gender = 'M'
+ s.name = 'Ian'
+ s.save(update_fields=['name'])
+
+ s = Person.objects.get(pk=s.pk)
+ self.assertEqual(s.gender, 'F')
+ self.assertEqual(s.name, 'Ian')
+
+ def test_update_fields_inheritance(self):
+ profile_boss = Profile.objects.create(name='Boss', salary=3000)
+ profile_receptionist = Profile.objects.create(name='Receptionist', salary=1000)
+
+ e1 = Employee.objects.create(name='Sara', gender='F',
+ employee_num=1, profile=profile_boss)
+
+ e1.name = 'Ian'
+ e1.gender = 'M'
+ e1.save(update_fields=['name'])
+
+ e2 = Employee.objects.get(pk=e1.pk)
+ self.assertEqual(e2.name, 'Ian')
+ self.assertEqual(e2.gender, 'F')
+ self.assertEqual(e2.profile, profile_boss)
+
+ e2.profile = profile_receptionist
+ e2.name = 'Sara'
+ e2.save(update_fields=['profile'])
+
+ e3 = Employee.objects.get(pk=e1.pk)
+ self.assertEqual(e3.name, 'Ian')
+ self.assertEqual(e3.profile, profile_receptionist)
+
+ 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):
+ s.save(update_fields=['first_name'])
+
+ with self.assertRaises(ValueError):
+ s.save(update_fields="name")
+
+ 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'])
Something went wrong with that request. Please try again.