Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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

Closed
wants to merge 1 commit into from

4 participants

django/db/models/base.py
@@ -2,6 +2,7 @@
import sys
from functools import update_wrapper
from itertools import izip
+from sets import ImmutableSet
@alex Collaborator
alex added a note

You don't need this, it's just frozenset.

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

A thought that just popped into my head: what's the interaction between this and defer()/only(). If I use one of those should the save automatically only save fields that I fetched?

@niwinz

No, first get all the fields that have deferred and then save everything.

Example:

>>> a = User.objects.defer("username").get(pk=1)
>>> a.save()

This generates the following queries:

(0.000) SELECT "auth_user"."id", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."password", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."is_superuser", "auth_user"."last_login", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = 1 ; args=(1,)
(0.000) SELECT (1) AS "a" FROM "auth_user" WHERE "auth_user"."id" = 1  LIMIT 1; args=(1,)
(0.000) SELECT "auth_user"."id", "auth_user"."username" FROM "auth_user" WHERE "auth_user"."id" = 1 ; args=(1,)
(0.000) UPDATE "auth_user" SET "username" = andrei, "first_name" = Andrei, "last_name" = Antoukh, "email" = niwi@niwi.be ....
@akaariai
Collaborator

The idea is that .only()/.defer() interaction will be handled later on. No need to handle it in this patch yet, but definitely should be done before 1.5.

@niwinz

Sorry, I misread the question! The interaction with "only" and "difer" would be interesting to be implemented for 1.5.

I'll work on it, the next free time!

@niwinz

Now patch updated to django master and ImmutableSet replaced with frozenset!

@niwinz

I just added a possible candidate for the interaction of parameter update_fields with methods only() and defer().

@akaariai
Collaborator

Lets defer the only() and defer() interaction in a separate pull request (I am just finishing off some last changes to this patch, will send you a pull request).

@niwinz

Well! I have reverted the last commit.

@akaariai
Collaborator

Scratch that pull request: for some reason I can't find your github repo to make that pull... So, if you can manually download the commit d66527f into your branch, rebase it to just one commit and remove the only()/defer() interactions for now. I suggest this as the sole commit message:

Fixed #4102 -- Allow update of specific fields in model.save()

Added the ability to update only part of the model's fields in
model.save() by introducing a new kwarg "update_fields". Thanks
to all the numerous reviewers and commenters in the ticket

(to download the commit:
curl https://github.com/akaariai/django/commit/d66527f79d1555abb04dd933019db4ca8d949988.patch | git am
)

@niwinz

Already updated the pull-request with your patch! Thank you for your corrections.

@akaariai
Collaborator

Thanks a lot! As far as I am concerned this is 100% ready for checkin. A final cursory check (especially of the doc changes) would be welcome.

niwibe: thanks for your patience. We all are still trying to figure how to best work using Trac + github together, and how to use pull requests, so you have been a test subject... :) If you have any opinions to share about this process please post them! It would be informative to hear how this process looks from your perspective...

@niwinz

I am very happy to have django on github, I'm much more easy to track changes.

The combination of using trac and github is not bad. At first I was confused, did not know if what I did was right or wrong, but gradually I have been adapting and learning.

I, on this issue, I have kept updated, patches in trac and pull-request at github. In my opinion there should be only a single site where they are patches. In github or traction, but not both at once.

Now, once this patch is part of the official repo django, I'm thinking of alex proposal (defer / only methods) and really do not know if I have to create a ticket in trac or not. Or just pull it with a request.

I think it should be (if not present) a site that explains the basic procedures to contribute patches, now that django is on github.

thanks

@akaariai
Collaborator

I think I have volunteered to write those guidelines... At the risk of totally side-tracking this pull request, my suggestion is the following: you should create a ticket (the addition is complex enough to warrant one), you should work in a branch of your github repo, and you can announce that latest work from you is available from that branch in the trac. Once the work nears completition, create a pull request. That is, branches are used to work on stuff, pull requests are used only when you really thing you have something ready for a pull.

docs/ref/signals.txt
@@ -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.
@akaariai Collaborator

Still one error here - sorry for not spotting this earlier. This should of course say a frozenset of fields to update... I can fix this when committing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
tests/modeltests/update_only_fields/tests.py
((6 lines not shown))
+
+
+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_m2n(self):
@freakboy3742 Owner

Typo - m2m not m2n

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@freakboy3742 freakboy3742 commented on the diff
docs/ref/models/instances.txt
@@ -334,6 +336,24 @@ 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 in keyword argument
+``update_fields``, only the fields named in that list will be updated.
+This may be desirable if you want to update just one or a few fields on
+an object. 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
@freakboy3742 Owner

"An not None" should be "A non-None"; but the whole sentence is a bit confusing anyway. Consider rewording the whole sentence. "An empty update_fields iterable will skip the save. A value of None will perform an update on all fields".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@niwinz niwinz Fixed #4102 -- Allow update of specific fields in model.save()
Added the ability to update only part of the model's fields in
model.save() by introducing a new kwarg "update_fields". Thanks
to all the numerous reviewers and commenters in the ticket
b671e27
@niwinz

Now I fixed this typos. ;)

@akaariai
Collaborator

Closed in commit 365853d - some final editorializing done based on the above comments.

Thanks for everybody participating. Of course, biggest thanks go to niwibe.

@akaariai akaariai closed this
@akaariai
Collaborator

I created a ticket for .only()/.defer() interaction, see #18306.

I missed your latest changes and instead did the final cleanup myself - concurrent working has its downsides.

@niwinz

Well! I responded on the ticket, and indicated where my proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 12, 2012
  1. @niwinz

    Fixed #4102 -- Allow update of specific fields in model.save()

    niwinz authored
    Added the ability to update only part of the model's fields in
    model.save() by introducing a new kwarg "update_fields". Thanks
    to all the numerous reviewers and commenters in the ticket
This page is out of date. Refresh to see the latest.
View
55 django/db/models/base.py
@@ -11,7 +11,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 +449,8 @@ 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,32 @@ 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 = frozenset(update_fields)
+ field_names = set([field.name for field in self._meta.fields
+ if not field.primary_key])
+ non_model_fields = update_fields.difference(field_names)
+
+ if non_model_fields:
+ raise ValueError("The following fields do not exist in this "
+ "model or are m2m fields: %s"
+ % ', '.join(non_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 +492,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 update_fields is None or len(update_fields) > 0
if cls is None:
cls = self.__class__
meta = cls._meta
@@ -483,7 +503,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 +524,8 @@ 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,22 +535,27 @@ 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
record_exists = True
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
+ # Determine if we should do an update (pk already exists, forced update,
+ # no force_insert)
+ 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:
values = [(f, None, (raw and getattr(self, f.attname) or f.pre_save(self, False))) for f in non_pks]
if values:
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 +568,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 +588,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
22 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.
@@ -289,6 +289,8 @@ almost always do the right thing and trying to override that will lead to
errors that are difficult to track down. This feature is for advanced use
only.
+Using ``update_fields`` will force an update similarly to ``force_update``.
+
Updating attributes based on existing fields
--------------------------------------------
@@ -334,6 +336,24 @@ 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 in keyword argument
+``update_fields``, only the fields named in that list will be updated.
+This may be desirable if you want to update just one or a few fields on
+an object. 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
@freakboy3742 Owner

"An not None" should be "A non-None"; but the whole sentence is a bit confusing anyway. Consider rewording the whole sentence. "An empty update_fields iterable will skip the save. A value of None will perform an update on all fields".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+empty ``update_fields`` iterable will skip the save. A value of None will
+perform an update on all fields.
+
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``
+ A frozenset 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``
+ A frozenset 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
11 docs/releases/1.5.txt
@@ -33,6 +33,17 @@ version compatible with Python 2.6.
What's new in Django 1.5
========================
+Support for saving a subset of model's fields
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The method :meth:`Model.save() <django.db.models.Model.save()>` has a new
+keyword argument ``update_fields``. By using this argument it is possible to
+save only a select list of model's fields. This can be useful for performance
+reasons or when trying to avoid overwriting concurrent changes.
+
+See the :meth:`Model.save() <django.db.models.Model.save()>` documentation for
+more details.
+
Minor features
~~~~~~~~~~~~~~
View
0  tests/modeltests/update_only_fields/__init__.py
No changes.
View
37 tests/modeltests/update_only_fields/models.py
@@ -0,0 +1,37 @@
+
+from django.db import models
+
+GENDER_CHOICES = (
+ ('M', 'Male'),
+ ('F', 'Female'),
+)
+
+class Account(models.Model):
+ num = models.IntegerField()
+
+
+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)
+ accounts = models.ManyToManyField('Account', related_name='employees', blank=True, null=True)
+
+
+class Profile(models.Model):
+ name = models.CharField(max_length=200)
+ salary = models.FloatField(default=1000.0)
+
+ def __unicode__(self):
+ return self.name
+
+
+class ProxyEmployee(Employee):
+ class Meta:
+ proxy = True
View
146 tests/modeltests/update_only_fields/tests.py
@@ -0,0 +1,146 @@
+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, ProxyEmployee, Profile, Account
+
+
+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_m2m(self):
+ profile_boss = Profile.objects.create(name='Boss', salary=3000)
+ e1 = Employee.objects.create(name='Sara', gender='F',
+ employee_num=1, profile=profile_boss)
+
+ a1 = Account.objects.create(num=1)
+ a2 = Account.objects.create(num=2)
+
+ e1.accounts = [a1,a2]
+
+ with self.assertRaises(ValueError):
+ e1.save(update_fields=['accounts'])
+
+ 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_inheritance_with_proxy_model(self):
+ profile_boss = Profile.objects.create(name='Boss', salary=3000)
+ profile_receptionist = Profile.objects.create(name='Receptionist', salary=1000)
+
+ e1 = ProxyEmployee.objects.create(name='Sara', gender='F',
+ employee_num=1, profile=profile_boss)
+
+ e1.name = 'Ian'
+ e1.gender = 'M'
+ e1.save(update_fields=['name'])
+
+ e2 = ProxyEmployee.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 = ProxyEmployee.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.