Transaction handling broken in related manager modification methods

[Anssi Kääriäinen]

Some of the related manager methods do multiple data-modifying operations but do not use any transaction management at all. This is clearly broken. This isn't a regression, so isn't a release blocker for that reason. However, this is a potential data corruption issue, so might be nice to fix for 1.6, too.

I am not sure how to test this. Maybe post_delete/save signals could be used to force exceptions and thus rollback.

EDIT: Forgot to add that a patch for some problematic methods available in ​https://github.com/django/django/pull/1684.

[loic84]

This also has the benefit of fixing #21169 for GFK RelatedManager.remove().

The same applies to FK RelatedManager.remove() which also suffers from this issue. I would also implement it self.using(db).filter(pk__in=[o.pk for o in objs]).delete(), solving both transaction handling and default filtering at once.

I've got the test cases for these stashed in a branch for #21169.

One thing to note though is the mild backward incompatibility: signals won't be fired anymore.

[Anssi Kääriäinen]

The .delete() code takes care of sending signals. Unfortunately it will also reload the instances back from DB to do so. It might be possible to use the lower level deletion APIs to send the already existing objects to be deleted in one go - no reload that way.

[loic84]

It just came back to mind that RelatedManager.remove() for FK doesn't delete(), it only SET_NULL, there's a bit of an incompatibility between FK et GFK here...

So yeah for FK the signal incompatibility would be changing the fly of save() by an update().

[Anssi Kääriäinen <akaariai@…>]

In 1df3c49a1a1c11198d181ddd0ce31bbb42e631d8:

Fixed #21174 -- transaction control in related manager methods

[Anssi Kääriäinen <akaariai@…>]

In 17c3997f6828e88e4646071a8187c1318b65597d:

Fixed #21169 -- Reworked RelatedManager methods use default filtering

The remove() and clear() methods of the related managers created by
ForeignKey, GenericForeignKey, and ManyToManyField suffered from a
number of issues. Some operations ran multiple data modifying queries without
wrapping them in a transaction, and some operations didn't respect default
filtering when it was present (i.e. when the default manager on the related
model implemented a custom get_queryset()).

Fixing the issues introduced some backward incompatible changes:

• The implementation of remove() for ForeignKey related managers changed from a series of Model.save() calls to a single QuerySet.update() call. The change means that pre_save and post_save signals aren't called anymore.

• The remove() and clear() methods for GenericForeignKey related managers now perform bulk delete so Model.delete() isn't called anymore.

• The remove() and clear() methods for ManyToManyField related managers perform nested queries when filtering is involved, which may or may not be an issue depending on the database and the data itself.

Refs. #3871, #21174.

Thanks Anssi Kääriäinen and Tim Graham for the reviews.

[loic84]

Fixed all the missing transactions I could find in related.py: ​https://github.com/django/django/pull/2489.

[Loic Bistuer <loic.bistuer@…>]

In bc9be72bdc9bb4dfc7f967ac3856115f0a6166b8:

Fixed transaction handling for a number of operations on related objects.

Thanks Anssi and Aymeric for the reviews. Refs #21174.