Somewhat long post ahead - jump to the list in the end of post to see the backwards incompatible changes.
Ticket #21169 deals with a bunch of different related manager improvements. Some are bug fixes, some are there to make related manager methods work consistently and finally the ticket's original issue is solved. That is custom related manager's possible filters are taken in account when removing or clearing the relation. These changes affect .clear() and .remove() of ManyToManyField, GenericForeignKey and reverse ForeignKey fields. The .add() methods aren't dealt with in the ticket.
A proposed patch is available from
https://github.com/django/django/pull/1685I'll start with the ticket's actual issue. Currently, if you have a custom related manager which has default filtering (say, .filter(deleted=False) in get_queryset()), that limitation isn't taken in account when clearing the relation or when removing items from the relation. In short, you can remove items from the relation even if they aren't visible in obj.relation.all(). There is one exception to this, that is reverse foreign key's.clear() did take the filtering in account.
There are two bug fixes in the proposed patch. First, some of the related manager methods used multiple data modifying queries without using transactions. Second, symmetric m2m.clear() has a concurrency issue - it was possible that just other side of a symmetric relation got cleared, but the other half was left in place.
Consistency issues are:
1) Some of the methods used consecutive save or delete calls, some used bulk deletion or direct update() calls.
2) As mentioned earlier, most of the manager methods didn't respect related manager's default filtering. Now they all do that.
3) Due to usage of different style of operations in the methods, sometimes signals were sent, sometimes not.
The actual changes fall mainly in two categories. First, use bulk operations always, and just one bulk operation per remove() and clear() call. This takes care of transaction and concurrency issues, as well as making the API consistent. The other category of changes is applying default filtering to those bulk operations. Notably in some cases (especially symmetric m2m with custom filtering) this means using complex subqueries. I wonder if some databases will choke on those subqueries.
Usage of bulk operations will result in changing how signals work. For example reverse foreign key .remove() used multiple save() calls, and thus signals were sent for each .save(). .clear() used an update() operation, and so no signals were sent. The .remove() is changed to use .update(), too, so no signals any more in that case. The .add() method was left alone - it is possible to still add items that are out of the related manager's domain. This is consistent with QuerySet .create() and .delete() - you can create items outside the current query's domain, but delete naturally applies the query's filters to the operation.
Loic Bistuer has written a patch that addresses all of these issues. The list of issues and changes he has written is worth reading. See comment:
https://code.djangoproject.com/ticket/21169#comment:9The problem here is that the above changes *will* result in backwards compatibility problems for some users. The problems have a possibility to be data-corruption issues. The exact working of these methods isn't documented so I am proposing to use this as a loophole to avoid backwards compatibility rules.
This is a nasty problem to solve. If we leave things as they are, the API will remain inconsistent. If we fix these issues, it is impossible to have a deprecation period for these changes. Even a check command seems hard to write.
The actual bug fixes will need to be done. It is possible to fix the bugs without doing backwards incompatible changes. But having a consistent related manager experience would of course be nice...
I am slightly in favour of doing these changes. If somebody has good ideas how to add deprecation periods to these changes I am all ears.
Here is the full list of changes that potential for breaking user code:
- If related object's default manager has default filtering, then .remove() and .clear() will not clear those items that are filtered out.
- Reverse ForeignKey .remove() will not use .save() - it will use .update() - so no model save signals, and overridden model.save() is missed, too.
-
GFK.remove() and
GFK.clear() will use queryset.delete() -
so model.delete() is not called anymore (signals are sent in this case as QuerySet.delete() does that).
Loic's list of fixes & changes is also a good summary of this ticket:
https://code.djangoproject.com/ticket/21169#comment:9 - Anssi