Deleting objects after searching related many to many field crashes the admin page

[Zain Patel]

Minimal reproduction:

# models.py
class Post(models.Model):
  title = models.String(...)
  authors = models.ManyToMany("User", ...)

class User(models.Model):
  email = models.String(...)

# admin.py

class PostAdmin(admin.ModelAdmin):
  search_fields = ("title", "authors__email")

then opening the admin site, opening the post page that contains only one post (any title and author assigned) and entering a search term (e.g the first 2 characters of the title), selecting the post and then using the delete action results in an Internal Sever Error 500 with an error/stack-trace:

Internal Server Error: /admin/post/post/
Traceback (most recent call last):
  File "...lib/python3.7/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "...lib/python3.7/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "...lib/python3.7/site-packages/django/contrib/admin/options.py", line 616, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "...lib/python3.7/site-packages/django/utils/decorators.py", line 130, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "...lib/python3.7/site-packages/django/views/decorators/cache.py", line 44, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "...lib/python3.7/site-packages/django/contrib/admin/sites.py", line 241, in inner
    return view(request, *args, **kwargs)
  File "...lib/python3.7/site-packages/django/utils/decorators.py", line 43, in _wrapper
    return bound_method(*args, **kwargs)
  File "...lib/python3.7/site-packages/django/utils/decorators.py", line 130, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "...lib/python3.7/site-packages/django/contrib/admin/options.py", line 1737, in changelist_view
    response = self.response_action(request, queryset=cl.get_queryset(request))
  File "...lib/python3.7/site-packages/django/contrib/admin/options.py", line 1406, in response_action
    response = func(self, request, queryset)
  File "...lib/python3.7/site-packages/django/contrib/admin/actions.py", line 45, in delete_selected
    modeladmin.delete_queryset(request, queryset)
  File "...lib/python3.7/site-packages/django/contrib/admin/options.py", line 1107, in delete_queryset
    queryset.delete()
  File "...lib/python3.7/site-packages/django/db/models/query.py", line 728, in delete
    raise TypeError('Cannot call delete() after .distinct().')
TypeError: Cannot call delete() after .distinct().
"POST /admin/post/post/?q=my HTTP/1.1" 500 137654

I can confirm that pip install django==3.1.8 fixes the error, and after having a look at the diff between stable/3.2.x and 3.1.8, I suspect the "regression" comes about from the work done on preserving the filters on delete or something along those lines - I haven't done a thorough investigation yet. Presumably .distinct() is being called because of the search involving the many to many field.

I am using a Postgres database.

[Mariusz Felisiak]

This exception was introduce in 6307c3f1a123f5975c73b231e8ac4f115fd72c0d and revealed a possible data loss issue in the admin. IMO we should use Exists() instead of distinct(), e.g.

diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py
index fefed29933..e9816ddd15 100644
--- a/django/contrib/admin/views/main.py
+++ b/django/contrib/admin/views/main.py
@@ -475,9 +475,8 @@ class ChangeList:
         if not qs.query.select_related:
             qs = self.apply_select_related(qs)
 
-        # Set ordering.
+        # Get ordering.
         ordering = self.get_ordering(request, qs)
-        qs = qs.order_by(*ordering)
 
         # Apply search results
         qs, search_use_distinct = self.model_admin.get_search_results(request, qs, self.query)
@@ -487,11 +486,14 @@ class ChangeList:
             new_params=remaining_lookup_params,
             remove=self.get_filters_params(),
         )
+
         # Remove duplicates from results, if necessary
         if filters_use_distinct | search_use_distinct:
-            return qs.distinct()
+            from django.db.models import Exists, OuterRef
+            qs = qs.filter(pk=OuterRef('pk'))
+            return self.root_queryset.filter(Exists(qs)).order_by(*ordering)
         else:
-            return qs
+            return qs.order_by(*ordering)
 
     def apply_select_related(self, qs):
         if self.list_select_related is True:

[Mariusz Felisiak]

​PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In 4074f38e:

Refs #32682 -- Fixed QuerySet.delete() crash on querysets with self-referential subqueries on MySQL.

[Mariusz Felisiak <felisiak.mariusz@…>]

In cd74aad:

Refs #32682 -- Renamed use_distinct variable to may_have_duplicates.

QuerySet.distinct() is not the only way to avoid duplicate, it's also
not preferred.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 18711820:

Fixed #32682 -- Made admin changelist use Exists() instead of distinct() for preventing duplicates.

Thanks Zain Patel for the report and Simon Charette for reviews.

The exception introduced in 6307c3f1a123f5975c73b231e8ac4f115fd72c0d
revealed a possible data loss issue in the admin.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 7ad70340:

[3.2.x] Refs #32682 -- Fixed QuerySet.delete() crash on querysets with self-referential subqueries on MySQL.

Backport of 4074f38e1dcc93b859bbbfd6abd8441c3bca36b3 from main

[Mariusz Felisiak <felisiak.mariusz@…>]

In fbea64b:

[3.2.x] Refs #32682 -- Renamed use_distinct variable to may_have_duplicates.

QuerySet.distinct() is not the only way to avoid duplicate, it's also
not preferred.

Backport of cd74aad90e09865ae6cd8ca0377ef0a5008d14e9 from main

[Mariusz Felisiak <felisiak.mariusz@…>]

In 34981f39:

[3.2.x] Fixed #32682 -- Made admin changelist use Exists() instead of distinct() for preventing duplicates.

Thanks Zain Patel for the report and Simon Charette for reviews.

The exception introduced in 6307c3f1a123f5975c73b231e8ac4f115fd72c0d
revealed a possible data loss issue in the admin.

Backport of 187118203197801c6cb72dc8b06b714b23b6dd3d from main

[GitHub <noreply@…>]

In baba733d:

Refs #32682 -- Renamed lookup_needs_distinct() to lookup_spawns_duplicates().

Follow up to 187118203197801c6cb72dc8b06b714b23b6dd3d.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 28e20771:

Refs #32433 -- Reallowed calling QuerySet.delete() after distinct().

While values(*field_excluding_pk).distinct() and
distinct(*field_excluding_pk) can reduce the number of resulting rows
in a way that makes subsequent delete() calls ambiguous standalone
.distinct() calls cannot.

Since delete() already disallows chain usages with values() the only
case that needs to be handled, as originally reported, is when
DISTINCT ON is used via distinct(*fields).

Refs #32682 which had to resort to subqueries to prevent duplicates in
the admin and caused significant performance regressions on MySQL
(refs #34639).

This partly reverts 6307c3f1a123f5975c73b231e8ac4f115fd72c0d.

[Mariusz Felisiak <felisiak.mariusz@…>]

In d569c1dc:

Fixed #34639 -- Reverted "Fixed #32682 -- Made admin changelist use Exists() instead of distinct() for preventing duplicates."

This reverts commit 187118203197801c6cb72dc8b06b714b23b6dd3d which
moved to using Exists() instead due to an overly strict
distinct().delete() check added in #32433.