Changes between Initial Version and Version 1 of Ticket #33928, comment 1


Ignore:
Timestamp:
Aug 17, 2022, 12:07:10 AM (2 years ago)
Author:
Simon Charette

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #33928, comment 1

    initial v1  
    55This is likely the case because the collector is able to take [https://github.com/django/django/blob/08688bd7dd8dfa218ecff03d6c94a753b1be8e59/django/db/models/deletion.py#L318-L320 the fast delete route] and avoid object fetching entirely.
    66
    7 If you want to give a shot at a patch you'll want to have a look at `Collector.collect` and have it skip collection entirely when dealing with `SET` and friends likely by adding a branch that turns them into ''fast'' updates. [https://github.com/django/django/blob/08688bd7dd8dfa218ecff03d6c94a753b1be8e59/django/db/models/deletion.py#L500-L504 One branch that worries me is the post-deletion assignment of values to in-memory instances] but I can't understand why this is even necessary given all the instances that are collected for field updates are never make their way out of the collector so I would expect it to be entirely unnecessary.
     7If you want to give a shot at a patch you'll want to have a look at `Collector.collect` and have it skip collection entirely when dealing with `SET` and friends likely by adding a branch that turns them into ''fast'' updates. [https://github.com/django/django/blob/08688bd7dd8dfa218ecff03d6c94a753b1be8e59/django/db/models/deletion.py#L500-L504 One branch that worries me is the post-deletion assignment of values to in-memory instances] but I can't understand why this is even necessary given all the instances that are collected for field updates are never make their way out of the collector so I would expect it to be entirely unnecessary at least all `delete` and `delete_regress` tests pass if I entirely remove it
     8
     9{{{#!diff
     10diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py
     11index 2cb3c88444..2eb8e95281 100644
     12--- a/django/db/models/deletion.py
     13+++ b/django/db/models/deletion.py
     14@@ -496,12 +496,6 @@ def delete(self):
     15                             using=self.using,
     16                             origin=self.origin,
     17                         )
     18-
     19-        # update collected instances
     20-        for instances_for_fieldvalues in self.field_updates.values():
     21-            for (field, value), instances in instances_for_fieldvalues.items():
     22-                for obj in instances:
     23-                    setattr(obj, field.attname, value)
     24         for model, instances in self.data.items():
     25             for instance in instances:
     26                 setattr(instance, model._meta.pk.attname, None)
     27}}}
    828
    929You'll want to make sure to avoid breaking the [https://github.com/django/django/blob/0dd29209091280ccf34e07c9468746c396b7778e/django/contrib/admin/utils.py#L164-L228 admin's collector subclass used to display deletion confirmation pages] but from a quick look it doesn't seem to care about field updates.
Back to Top