Reverse manager assignments doing an implicit save

131 views
Skip to first unread message

Anssi Kääriäinen

unread,
Apr 20, 2012, 10:00:01 AM4/20/12
to Django developers
I was going through ORM tickets marked ready for checkin and stumbled
upon ticket #14368. The ticket is about caching of o2o fields. The
interesting part is not the caching, but this functionality:

class A:
pass
class B:
a = O2O(A, null=True, related_name='b')
a1 = A.save()
b1 = B.save()
# The below does not do a save currently.
a1.b = b1
# But after the patch there is an implicit save of b1

The implicit save is implemented because there is a similar
functionality for ForeinKeys:
>>> someobj.reverse_fk_set = RelatedObj.objects.all()
What happens is that the RelatedObj.objects.all() queryset is
iterated, and each object of the queryset is saved with the fk changed
to point to the someobj. An implicit save again.

Now, I don't like the implicit saves at all. A variable assignment
should not cause a database save. So, I would like to deprecate the
current behavior. Assignment to reverse_fk_set (and I guess this goes
for m2m, too) is no longer allowed. Instead you will need to
explicitly do:
>>> someobj.reverse_fk_set.set(RelatedObj.objects.all())
Now you are calling a method, and saving in this situation is OK and
analogous to other related manager methods.

Lets raise a deprecation warning on direct assignment to the
reverse_fk_set and remove it then in 1.7. The message would be
something like "Direct assignment to the reverse side of a related set
is deprecated. Use .set() instead. See ... for more details."

A related thread discussing this same issue:
http://groups.google.com/group/django-developers/tree/browse_frm/thread/634499444687556f#doc_f1a254e8bf4ad8d9

Comments?

- Anssi

Tim Graham

unread,
Oct 8, 2015, 5:22:42 PM10/8/15
to Django developers (Contributions to Django itself)
The deprecation seems reasonable to me as it would clear up confusion about why most model attribute assignments require a model save but not this one.

Implemented here: https://github.com/django/django/pull/5413
Reply all
Reply to author
Forward
0 new messages