On Oct 8, 2:20 pm, Russell Keith-Magee <russ...@keith-magee.com> wrote:
> #14043 is clearly a bug to me (hence the accepted status). If I had to > guess at a cause, I'd say it's either: > * The OneToOneField special case not being handled by deletion traversal > * The related object cache on the o2o field not being cleared > correctly when null is assigned, causing the delete cascade to operate > on the older cached object. Turns out the related object didn't have a cache at all. The fix is a single line in SingleRelatedObjectDescriptor.__get__() that sets the back link from the related object, which ends up setting the cache [1]. > #14368 also strikes me as a bug, but it's one that's a little hard to > account for without some other changes. > > In saying bob.soul = None, you then need to save the soul object in > order for the change to take effect. That isn't something that is > currently done. However, interestingly, it *is* done if you do the > analogous operation with a foreign key -- if you assign a queryset to > a reverse FK relation, every object in the queryset is modified and > saved (and looking at the code, this is something we can *massively* > optimize with an update statement). In the interests of removing a > wierd inconsistency, I can cer > > However, this is a situation where pragmatism will need to beat > purity. OneToOne fields are also the cornerstone of inheritance, and > autosaving OneToOne reverse relations strikes me as something that > could backfire in the internals to inheritance. I'd need to see a > sample patch that doesn't break the existing test suite before I could > make any more firm judgements. Just posted a patch [2] that does the autosave and passes all the tests. In addition to the set to None issue, it also fixes the child reassignment problem. For the example above, doing ``bob.soul = other_soul`` currently raises a unique constraint integrity error when attempting to save() since bob would end up with two souls. With the patch, the existing bobs_soul is unlinked from bob and therefore no integrity error is raised. This is consistent with the FK behaviour. If for some reason you object to autosave for this case, it's straightforward to restrict it to only when setting to None. Since you mentioned the optimization of using an update statement instead of save(), the patch uses an update statement. I'm not sure if this is right though; shouldn't the pre_save/post_save signals be sent here (as well as in the FK case) ? George [1] http://code.djangoproject.com/attachment/ticket/14043/ticket-14043.patch [2] http://code.djangoproject.com/attachment/ticket/14368/ticket-14368.patch -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
