when saving a model with m2m field in the admin, 'clear' and 'add' m2m_changed signals are fired even when there is no change.

[benc]

I've built a simple test project with one app and two models, Model1 and Model2.
Model2 has a ManyToManyField to Model1.
I've created one Model1 instance and one Model2 instance with a relation to the Model1 instance.

When saving the Model2 instance in the admin, even without a change, m2m_changed fires twice.
I think it shouldn't fire at all when the admin form is saved without a change:

sender  <class 'testproject.testapp.models.Model2_models1'>
instance  Model2 object
action  clear
model  <class 'testproject.testapp.models.Model1'>
sender  <class 'testproject.testapp.models.Model2_models1'>
instance  Model2 object
action  add
model  <class 'testproject.testapp.models.Model1'>

[Ramiro Morales]

reformatted description.

[Russell Keith-Magee]

I'm going to call this working as intended.

The admin code is doing an assignment of the form obj.m2m = [new values], which internally is 2 signals - clear and add signal. This is completely accurate behaviour, and there isn't much we can do to fix this. In order to avoid sending the signals, we need to query the database to do a diff between submitted values and new values, which is potentially an expensive operation.

[halldor89]

An attempt at fixing this issue.

[halldor89]

Even though the current behavior might be intended it does render the m2m_changed signal unusable. It makes it completely impossible to use django.forms to save data that should trigger m2m_changed to database (if it's not only performing a basic task like updating other database rows).
I put together a small patch that modifies the _ _set_ _ method of (Reverse)ManyRelatedObjectsDescriptor so that it performs that one extra query to get current ids and then uses sets to do a diff between submitted and new values. Since the m2m.clear() can also be kinda expensive and m2m.add(*values) does one insert per item I think that one query and some basic calculation will pay off.

It would, however, be awesome if someone more skilled than me could improve the patch (since I'm not sure how efficient sets are etc., it was just the easiest fix I could think of).

[Russell Keith-Magee]

Hyperbole will get you nowhere. The current behavior doesn't render anything *unusable*. You are correctly notified of the signals - you just get more signal responses that you might want. This may be inconvenient, and it may result in more database activity that is desirable, but it isn't *unusable*.

That said, the optimization you describe is worth pursuing - which is why it's open as ticket #6707. The patch you present is essentially the same as the one on that ticket, but that approach is a little naive; at the very least, it neglects the multiple extra queries that occur during _add_items() that already do set subtraction to avoid duplicate writes to the m2m table.

Closing as wontfix again; the original report was specifically about an admin problem, which is wontfix; optimizing m2m is ticket #6707, which is current DDN.