Fixed #33928 -- Avoided unnecessary queries when cascade updating.

[charettes]

Models that use SET, SET_NULL, and SET_DEFAULT as on_delete handler
don't have to fetch objects for the sole purpose of passing them back to
a follow up UPDATE query filtered by the retrieved objects primary key.

This was achieved by flagging SET handlers as lazy and having the collector
logic deferr object collections until the last minute. This should ensure that
the rare cases where custom on_delete handlers are defined remain uncalled when
when dealing with an empty collection of instances.

This reduces the number queries required to apply SET handlers from 2 to 1
where the remaining UPDATE use the same predicate as the non removed SELECT
query.

In a lot of ways this is similar to the fast-delete optimization that was added
in #18676 but for updates this time. The conditions only happen to be simpler
in this case because SET handlers are always terminal; they never cascade to
more deletes that can be combined.

Thanks @rgehan for the report.

[charettes]

The only potentially undesirable side effect of this change is that a callable passed to SET or a callable field default called by SET_DEFAULT will now be called even if not necessary. This can be avoided by disabling this optimization with the following changes if deemed necessary

diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py
index be2002d23e..b268f01540 100644
--- a/django/db/models/deletion.py
+++ b/django/db/models/deletion.py
@@ -53,6 +53,10 @@ def SET(value):
     if callable(value):

         def set_on_delete(collector, field, sub_objs, using):
+            # Avoid potential side effects (e.g. queries) that value()
+            # might incur.
+            if not sub_objs:
+                return
             collector.add_field_update(field, value(), sub_objs)

     else:
@@ -73,6 +77,10 @@ def SET_NULL(collector, field, sub_objs, using):


 def SET_DEFAULT(collector, field, sub_objs, using):
+    if callable(field.default) and not sub_objs:
+        # Avoid potential side effects (e.g. queries) that field.get_default()
+        # might incur.
+        return
     collector.add_field_update(field, field.get_default(), sub_objs)

[charettes]

Using models.QuerySet instead of a direct QuerySet import to avoid circular imports.

[felixxm]

@charettes Thanks 👍 I really like the proposed approach 🥇

[dutkulang]

Unsubscribe

…

On Fri, Aug 26, 2022, 10:23 AM Mariusz Felisiak ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In django/db/models/deletion.py <#15969 (comment)>: > - for (field, value), instances in instances_for_fieldvalues.items(): + for (field, value), instances_list in self.field_updates.items(): + updates = [] + objs = [] + for instances in instances_list: + if ( + isinstance(instances, models.QuerySet) + and instances._result_cache is None + ): + updates.append(instances) + else: + objs.extend(instances) + if updates: + combined_updates = reduce(or_, updates) + combined_updates.update(**{field.name: value}) + if objs: Do we need this branch? 🤔 All built-in handlers use non-evaluated querysets returned by related_objects(), so tests work without it: # update fields for (field, value), instances_list in self.field_updates.items(): updates = reduce(or_, instances_list) updates.update(**{field.name: value}) — Reply to this email directly, view it on GitHub <#15969 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQT5ZXP4RYPJ3Z3VY2SKJFTV3BWIDANCNFSM563NPN3Q> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[19greg96]

This might not be the best place to ask this, but is there any reason why this is not field.model instead?

When using polymorphism, this line causes database consistency too break, as it only updates the table of the derived class which corresponds to the first object in objs. However if it were field.model instead, the base class' table would be updated and consistency would be honored.

Should I create a PR?

[felixxm]

Interesting, please create a new ticket in Trac with a sample project and follow our bug reporting guidelines. All bugfixes require an accepted ticket.