From 99321e30cebbffeafc6ae19f4f92a0a665cbf19b Mon Sep 17 00:00:00 2001 From: Andrei Antoukh Date: Sun, 12 Aug 2012 22:17:54 +0300 Subject: [PATCH] Fixed #18306 -- Made deferred models issue update_fields on save Deferred models now automatically update only the fields which are loaded from the db (with .only() or .defer()). In addition, any field set manually after the load is updated on save. --- django/db/models/base.py | 18 ++++ docs/ref/models/instances.txt | 6 ++ docs/ref/models/querysets.txt | 16 +++ docs/releases/1.5.txt | 4 + tests/modeltests/update_only_fields/models.py | 1 + tests/modeltests/update_only_fields/tests.py | 101 ++++++++++++++++++ .../multiple_database/tests.py | 15 +++ 7 files changed, 161 insertions(+) diff --git a/django/db/models/base.py b/django/db/models/base.py index 30e07be3a7..1e1a138714 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -475,6 +475,7 @@ class Model(six.with_metaclass(ModelBase, object)): that the "save" must be an SQL insert or update (or equivalent for non-SQL backends), respectively. Normally, they should not be set. """ + using = using or router.db_for_write(self.__class__, instance=self) if force_insert and (force_update or update_fields): raise ValueError("Cannot force both insert and updating in model saving.") @@ -502,6 +503,23 @@ class Model(six.with_metaclass(ModelBase, object)): "model or are m2m fields: %s" % ', '.join(non_model_fields)) + # If saving to the same database, and this model is deferred, then + # automatically do a "update_fields" save on the loaded fields. + elif not force_insert and self._deferred and using == self._state.db: + field_names = set() + for field in self._meta.fields: + if not field.primary_key and not hasattr(field, 'through'): + field_names.add(field.attname) + deferred_fields = [ + f.attname for f in self._meta.fields + if f.attname not in self.__dict__ + and isinstance(self.__class__.__dict__[f.attname], + DeferredAttribute)] + + loaded_fields = field_names.difference(deferred_fields) + if loaded_fields: + update_fields = frozenset(loaded_fields) + self.save_base(using=using, force_insert=force_insert, force_update=force_update, update_fields=update_fields) save.alters_data = True diff --git a/docs/ref/models/instances.txt b/docs/ref/models/instances.txt index 14541ad0d1..b7ae3890cf 100644 --- a/docs/ref/models/instances.txt +++ b/docs/ref/models/instances.txt @@ -386,6 +386,12 @@ perform an update on all fields. Specifying ``update_fields`` will force an update. +When saving a model fetched through deferred model loading +(:meth:`~Model.only()` or :meth:`~Model.defer()`) only the fields loaded from +the DB will get updated. In effect there is an automatic ``update_fields`` in +this case. If you assign or change any deferred field value, these fields will +be added to the updated fields. + Deleting objects ================ diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index 8c188c67c3..b59b2ece82 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -1110,6 +1110,14 @@ one, doing so will result in an error. reader, is slightly faster and consumes a little less memory in the Python process. +.. versionchanged:: 1.5 + +.. note:: + + When calling :meth:`~Model.save()` for instances with deferred fields, + only the loaded fields will be saved. See :meth:`~Model.save()` for more + details. + only ~~~~ @@ -1154,6 +1162,14 @@ All of the cautions in the note for the :meth:`defer` documentation apply to options. Also note that using :meth:`only` and omitting a field requested using :meth:`select_related` is an error as well. +.. versionchanged:: 1.5 + +.. note:: + + When calling :meth:`~Model.save()` for instances with deferred fields, + only the loaded fields will be saved. See :meth:`~Model.save()` for more + details. + using ~~~~~ diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index 4aeb61af94..2896acb36b 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -42,6 +42,10 @@ 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. +Deferred instances (those loaded by .only() or .defer()) will automatically +save just the loaded fields. If any field is set manually after load, that +field will also get updated on save. + See the :meth:`Model.save() ` documentation for more details. diff --git a/tests/modeltests/update_only_fields/models.py b/tests/modeltests/update_only_fields/models.py index 01184f715b..bf5dd99166 100644 --- a/tests/modeltests/update_only_fields/models.py +++ b/tests/modeltests/update_only_fields/models.py @@ -15,6 +15,7 @@ class Account(models.Model): class Person(models.Model): name = models.CharField(max_length=20) gender = models.CharField(max_length=1, choices=GENDER_CHOICES) + pid = models.IntegerField(null=True, default=None) def __str__(self): return self.name diff --git a/tests/modeltests/update_only_fields/tests.py b/tests/modeltests/update_only_fields/tests.py index bce53ca621..c904c97236 100644 --- a/tests/modeltests/update_only_fields/tests.py +++ b/tests/modeltests/update_only_fields/tests.py @@ -18,6 +18,107 @@ class UpdateOnlyFieldsTests(TestCase): self.assertEqual(s.gender, 'F') self.assertEqual(s.name, 'Ian') + def test_update_fields_deferred(self): + s = Person.objects.create(name='Sara', gender='F', pid=22) + self.assertEqual(s.gender, 'F') + + s1 = Person.objects.defer("gender", "pid").get(pk=s.pk) + s1.name = "Emily" + s1.gender = "M" + + with self.assertNumQueries(1): + s1.save() + + s2 = Person.objects.get(pk=s1.pk) + self.assertEqual(s2.name, "Emily") + self.assertEqual(s2.gender, "M") + + def test_update_fields_only_1(self): + s = Person.objects.create(name='Sara', gender='F') + self.assertEqual(s.gender, 'F') + + s1 = Person.objects.only('name').get(pk=s.pk) + s1.name = "Emily" + s1.gender = "M" + + with self.assertNumQueries(1): + s1.save() + + s2 = Person.objects.get(pk=s1.pk) + self.assertEqual(s2.name, "Emily") + self.assertEqual(s2.gender, "M") + + def test_update_fields_only_2(self): + s = Person.objects.create(name='Sara', gender='F', pid=22) + self.assertEqual(s.gender, 'F') + + s1 = Person.objects.only('name').get(pk=s.pk) + s1.name = "Emily" + s1.gender = "M" + + with self.assertNumQueries(2): + s1.save(update_fields=['pid']) + + s2 = Person.objects.get(pk=s1.pk) + self.assertEqual(s2.name, "Sara") + self.assertEqual(s2.gender, "F") + + def test_update_fields_only_repeated(self): + s = Person.objects.create(name='Sara', gender='F') + self.assertEqual(s.gender, 'F') + + s1 = Person.objects.only('name').get(pk=s.pk) + s1.gender = 'M' + with self.assertNumQueries(1): + s1.save() + # Test that the deferred class does not remember that gender was + # set, instead the instace should remember this. + s1 = Person.objects.only('name').get(pk=s.pk) + with self.assertNumQueries(1): + s1.save() + + def test_update_fields_inheritance_defer(self): + profile_boss = Profile.objects.create(name='Boss', salary=3000) + e1 = Employee.objects.create(name='Sara', gender='F', + employee_num=1, profile=profile_boss) + e1 = Employee.objects.only('name').get(pk=e1.pk) + e1.name = 'Linda' + with self.assertNumQueries(1): + e1.save() + self.assertEqual(Employee.objects.get(pk=e1.pk).name, + 'Linda') + + def test_update_fields_fk_defer(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 = Employee.objects.only('profile').get(pk=e1.pk) + e1.profile = profile_receptionist + with self.assertNumQueries(1): + e1.save() + self.assertEqual(Employee.objects.get(pk=e1.pk).profile, profile_receptionist) + e1.profile_id = profile_boss.pk + with self.assertNumQueries(1): + e1.save() + self.assertEqual(Employee.objects.get(pk=e1.pk).profile, profile_boss) + + def test_select_related_only_interaction(self): + profile_boss = Profile.objects.create(name='Boss', salary=3000) + e1 = Employee.objects.create(name='Sara', gender='F', + employee_num=1, profile=profile_boss) + e1 = Employee.objects.only('profile__salary').select_related('profile').get(pk=e1.pk) + profile_boss.name = 'Clerk' + profile_boss.salary = 1000 + profile_boss.save() + # The loaded salary of 3000 gets saved, the name of 'Clerk' isn't + # overwritten. + with self.assertNumQueries(1): + e1.profile.save() + reloaded_profile = Profile.objects.get(pk=profile_boss.pk) + self.assertEqual(reloaded_profile.name, profile_boss.name) + self.assertEqual(reloaded_profile.salary, 3000) + def test_update_fields_m2m(self): profile_boss = Profile.objects.create(name='Boss', salary=3000) e1 = Employee.objects.create(name='Sara', gender='F', diff --git a/tests/regressiontests/multiple_database/tests.py b/tests/regressiontests/multiple_database/tests.py index 74a5f2f550..782fe2bfc6 100644 --- a/tests/regressiontests/multiple_database/tests.py +++ b/tests/regressiontests/multiple_database/tests.py @@ -1546,6 +1546,21 @@ class RouterTestCase(TestCase): # If you evaluate the query, it should work, running on 'other' self.assertEqual(list(qs.values_list('title', flat=True)), ['Dive into Python']) + def test_deferred_models(self): + mark_def = Person.objects.using('default').create(name="Mark Pilgrim") + mark_other = Person.objects.using('other').create(name="Mark Pilgrim") + orig_b = Book.objects.using('other').create(title="Dive into Python", + published=datetime.date(2009, 5, 4), + editor=mark_other) + b = Book.objects.using('other').only('title').get(pk=orig_b.pk) + self.assertEqual(b.published, datetime.date(2009, 5, 4)) + b = Book.objects.using('other').only('title').get(pk=orig_b.pk) + b.editor = mark_def + b.save(using='default') + self.assertEqual(Book.objects.using('default').get(pk=b.pk).published, + datetime.date(2009, 5, 4)) + + class AuthTestCase(TestCase): multi_db = True