Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix #18306 -- Defer and update_fields interaction. #213

Closed
wants to merge 2 commits into from

3 participants

Anssi Kääriäinen Alex Gaynor Andrey Antukh
Anssi Kääriäinen
Collaborator

No description provided.

niwibe and others added some commits
Andrey Antukh niwibe 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.
123dde0
Anssi Kääriäinen akaariai Polished update_only and defer() interaction
The biggest changes are:
  - a bugfix to select_related handling
  - a bugfix for shared ._state.deferred_fields for instances loaded
    through the same queryset.
  - some code cleanup related to field.name <> field.attname handling
  - documentation polish

The bugfixes were done by getting totally rid of ._state.deferred_fields
and instead relying on inspecting the instance's __dict__ and
__class__.__dict__ for deferred field information on save.

Refs #18306.
d2f21c8
Alex Gaynor alex commented on the diff
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
Alex Gaynor Collaborator
alex added a note

These links are wrong, the methods are on QuerySet, not Model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Anssi Kääriäinen
Collaborator

Didn't see Alex' comment before, it is fixed now...

Closing this one as the patch has been applied some time ago...

Anssi Kääriäinen akaariai closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 16, 2012
  1. Andrey Antukh Anssi Kääriäinen

    Fixed #18306 -- Made deferred models issue update_fields on save

    niwibe authored akaariai committed
    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.
  2. Anssi Kääriäinen

    Polished update_only and defer() interaction

    akaariai authored
    The biggest changes are:
      - a bugfix to select_related handling
      - a bugfix for shared ._state.deferred_fields for instances loaded
        through the same queryset.
      - some code cleanup related to field.name <> field.attname handling
      - documentation polish
    
    The bugfixes were done by getting totally rid of ._state.deferred_fields
    and instead relying on inspecting the instance's __dict__ and
    __class__.__dict__ for deferred field information on save.
    
    Refs #18306.
This page is out of date. Refresh to see the latest.
22 django/db/models/base.py
View
@@ -448,6 +448,7 @@ 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.
"""
+ 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.")
@@ -475,6 +476,22 @@ def save(self, force_insert=False, force_update=False, using=None,
"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
@@ -487,7 +504,9 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
need for overrides of save() to pass around internal-only parameters
('raw', 'cls', and 'origin').
"""
- using = using or router.db_for_write(self.__class__, instance=self)
+ if raw:
+ using = using or router.db_for_write(self.__class__, instance=self)
+ assert using is not None
assert not (force_insert and (force_update or update_fields))
assert update_fields is None or len(update_fields) > 0
if cls is None:
@@ -508,6 +527,7 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
# attributes we have been given to the class we have been given.
# We also go through this process to defer the save of proxy objects
# to their actual underlying model.
+
if not raw or meta.proxy:
if meta.proxy:
org = cls
1  django/db/models/query.py
View
@@ -271,6 +271,7 @@ def iterator(self):
aggregate_start = index_start + len(load_fields or self.model._meta.fields)
skip = None
+
if load_fields and not fill_cache:
# Some fields have been deferred, so we have to initialise
# via keyword arguments.
2  django/db/models/query_utils.py
View
@@ -114,7 +114,7 @@ def __set__(self, instance, value):
def _check_parent_chain(self, instance, name):
"""
- Check if the field value can be fetched from a parent field already
+ Check if the field value can be fetched from a parent field already
loaded in the instance. This can be done if the to-be fetched
field is a primary key field.
"""
6 docs/ref/models/instances.txt
View
@@ -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
Alex Gaynor Collaborator
alex added a note

These links are wrong, the methods are on QuerySet, not Model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+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
================
16 docs/ref/models/querysets.txt
View
@@ -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
~~~~~
4 docs/releases/1.5.txt
View
@@ -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() <django.db.models.Model.save()>` documentation for
more details.
1  tests/modeltests/update_only_fields/models.py
View
@@ -13,6 +13,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 __unicode__(self):
return self.name
101 tests/modeltests/update_only_fields/tests.py
View
@@ -18,6 +18,107 @@ def test_update_fields_basic(self):
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',
15 tests/regressiontests/multiple_database/tests.py
View
@@ -1545,6 +1545,21 @@ def test_subquery(self):
# 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
Something went wrong with that request. Please try again.