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/1685

I'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:9

The 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

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/b5477805-88b4-4d9e-8dc0-d4a2c98a6a15%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to