Opened 8 years ago

Closed 7 years ago

Last modified 21 months ago

#5599 closed Uncategorized (invalid)

Cannot save value via OneToOneField

Reported by: baumer1122 Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: onetoone
Cc: sgt.hulka@…, mocksoul@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Trying to save a value into a OneToOne child instance via it's parent has no effect.

>>> p = Place.objects.get(pk=1)
>>> p.restaurant.waiters
10
>>> p.restaurant.waiters = 5
>>> p.save()
>>> p.restaurant.waiters
10
>>> p.restaurant.save()
>>> p.restaurant.waiters
10

Saving via the child works as expected.

>>> r = Restaurant.objects.get(pk=1)
>>> r.waiters
10
>>> r.waiters = 5
>>> r.save()
>>> r.waiters
5

Attachments (3)

onetoone_relation_field_saves.diff (868 bytes) - added by MockSoul 7 years ago.
Fast fixup.
onetoone_relation_field_saves_v2.diff (1.1 KB) - added by MockSoul 7 years ago.
onetoone_relation_field_saves_v3.diff (1.1 KB) - added by MockSoul <mocksoul@…> 7 years ago.
Both prev diffs were invalid, sorry.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 8 years ago by baumer1122

  • Cc sgt.hulka@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 7 years ago by MockSoul

  • Cc mocksoul@… added

Confirmed with latest SVN (r7003). Value does not even save in the model.

>>> p = Place.objects.get(pk=1)
>>> p.restaurant.waiters
10
>>> p.restaurant.waiters = 5
>>> p.restaurant.waiters
10

comment:3 Changed 7 years ago by MockSoul

  • Has patch set
  • Needs tests set

Fast fixup. I'm sorry I have no a lot of time to test this, but works fine for me.
So, please make your checks :).

  • db/models/fields/related.py

     
    126126    def __get__(self, instance, instance_type=None):
    127127        if instance is None:
    128128            raise AttributeError, "%s must be accessed via instance" % self.related.opts.object_name
     129        try:
     130            self.__cached_rel_obj
     131        except AttributeError:
     132            params = {'%s__pk' % self.related.field.name: instance._get_pk_val()}
     133            rel_obj = self.related.model._default_manager.get(**params)
     134            self.__cached_rel_obj = rel_obj
    129135
    130         params = {'%s__pk' % self.related.field.name: instance._get_pk_val()}
    131         rel_obj = self.related.model._default_manager.get(**params)
    132         return rel_obj
     136        return self.__cached_rel_obj
    133137
    134138    def __set__(self, instance, value):
    135139        if instance is None:

Changed 7 years ago by MockSoul

Fast fixup.

comment:4 Changed 7 years ago by MockSoul

More accurate fixup

  • db/models/fields/related.py

     
    126126    def __get__(self, instance, instance_type=None):
    127127        if instance is None:
    128128            raise AttributeError, "%s must be accessed via instance" % self.related.opts.object_name
     129        cache_name = self.related.field.get_cache_name()
     130        try:
     131            return getattr(self.related, cache_name)
     132        except AttributeError:
     133            params = {'%s__pk' % self.related.field.name: instance._get_pk_val()}
     134            rel_obj = self.related.model._default_manager.get(**params)
     135            setattr(self.related, cache_name, rel_obj)
     136            return rel_obj
    129137
    130         params = {'%s__pk' % self.related.field.name: instance._get_pk_val()}
    131         rel_obj = self.related.model._default_manager.get(**params)
    132         return rel_obj
    133 
    134138    def __set__(self, instance, value):
    135139        if instance is None:
    136140            raise AttributeError, "%s must be accessed via instance" % self.related.opts.object_name

Changed 7 years ago by MockSoul

Changed 7 years ago by MockSoul <mocksoul@…>

Both prev diffs were invalid, sorry.

comment:5 follow-up: Changed 7 years ago by jacob

  • Resolution set to invalid
  • Status changed from new to closed

Yeah, setting attributes on related objects doesn't automatically save them; this'll happen with an FK or M2M, too. You need to do something like:

r = place.restaurant
r.waiters = 5
r.save()

comment:6 in reply to: ↑ 5 ; follow-up: Changed 7 years ago by Vadim Fint <mocksoul@…>

Replying to jacob:

Yeah, setting attributes on related objects doesn't automatically save them; this'll happen with an FK or M2M, too. You need to do something like:

This is confusing me a little. Why did such design decision was choosen?

comment:7 in reply to: ↑ 6 ; follow-up: Changed 7 years ago by ubernostrum

Replying to Vadim Fint <mocksoul@gmail.com>:

Replying to jacob:

Yeah, setting attributes on related objects doesn't automatically save them; this'll happen with an FK or M2M, too. You need to do something like:

This is confusing me a little. Why did such design decision was choosen?

It's consistent; Django doesn't push changes to an object's field values down to your DB until you call save(); this is the case with any field type, and is generally considered useful because it means that you don't end up doing a hit to your DB every time you assign to an attribute.

comment:8 in reply to: ↑ 7 Changed 21 months ago by daotranbang@…

  • Easy pickings unset
  • Severity set to Normal
  • Type set to Uncategorized
  • UI/UX unset

Replying to ubernostrum:

Replying to Vadim Fint <mocksoul@gmail.com>:

Replying to jacob:

Yeah, setting attributes on related objects doesn't automatically save them; this'll happen with an FK or M2M, too. You need to do something like:

This is confusing me a little. Why did such design decision was choosen?

It's consistent; Django doesn't push changes to an object's field values down to your DB until you call save(); this is the case with any field type, and is generally considered useful because it means that you don't end up doing a hit to your DB every time you assign to an attribute.

I think he mean to ask why did this design was choosen:

varname = object.related_name
varname.key = value
varname.save()

instead of

object.related_name.key = value
object.related_name.save()
Note: See TracTickets for help on using tickets.
Back to Top