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.

1 files changed, 52 insertions, 21 deletions

diff --git a/tests/admin_changelist/tests.py b/tests/admin_changelist/tests.py
index dc1b987a1e..c5910d6bdb 100644
--- a/tests/admin_changelist/tests.py
+++ b/tests/admin_changelist/tests.py
@@ -299,7 +299,7 @@ class ChangeListTests(TestCase):
         cl.get_results(request)
         self.assertIsInstance(cl.paginator, CustomPaginator)
 
-    def test_distinct_for_m2m_in_list_filter(self):
+    def test_no_duplicates_for_m2m_in_list_filter(self):
         """
         Regression test for #13902: When using a ManyToMany in list_filter,
         results shouldn't appear more than once. Basic ManyToMany.
@@ -319,8 +319,12 @@ class ChangeListTests(TestCase):
 
         # There's only one Group instance
         self.assertEqual(cl.result_count, 1)
+        # Queryset must be deletable.
+        self.assertIs(cl.queryset.query.distinct, False)
+        cl.queryset.delete()
+        self.assertEqual(cl.queryset.count(), 0)
 
-    def test_distinct_for_through_m2m_in_list_filter(self):
+    def test_no_duplicates_for_through_m2m_in_list_filter(self):
         """
         Regression test for #13902: When using a ManyToMany in list_filter,
         results shouldn't appear more than once. With an intermediate model.
@@ -339,12 +343,15 @@ class ChangeListTests(TestCase):
 
         # There's only one Group instance
         self.assertEqual(cl.result_count, 1)
+        # Queryset must be deletable.
+        self.assertIs(cl.queryset.query.distinct, False)
+        cl.queryset.delete()
+        self.assertEqual(cl.queryset.count(), 0)
 
-    def test_distinct_for_through_m2m_at_second_level_in_list_filter(self):
+    def test_no_duplicates_for_through_m2m_at_second_level_in_list_filter(self):
         """
         When using a ManyToMany in list_filter at the second level behind a
-        ForeignKey, distinct() must be called and results shouldn't appear more
-        than once.
+        ForeignKey, results shouldn't appear more than once.
         """
         lead = Musician.objects.create(name='Vox')
         band = Group.objects.create(name='The Hype')
@@ -361,8 +368,12 @@ class ChangeListTests(TestCase):
 
         # There's only one Concert instance
         self.assertEqual(cl.result_count, 1)
+        # Queryset must be deletable.
+        self.assertIs(cl.queryset.query.distinct, False)
+        cl.queryset.delete()
+        self.assertEqual(cl.queryset.count(), 0)
 
-    def test_distinct_for_inherited_m2m_in_list_filter(self):
+    def test_no_duplicates_for_inherited_m2m_in_list_filter(self):
         """
         Regression test for #13902: When using a ManyToMany in list_filter,
         results shouldn't appear more than once. Model managed in the
@@ -382,8 +393,12 @@ class ChangeListTests(TestCase):
 
         # There's only one Quartet instance
         self.assertEqual(cl.result_count, 1)
+        # Queryset must be deletable.
+        self.assertIs(cl.queryset.query.distinct, False)
+        cl.queryset.delete()
+        self.assertEqual(cl.queryset.count(), 0)
 
-    def test_distinct_for_m2m_to_inherited_in_list_filter(self):
+    def test_no_duplicates_for_m2m_to_inherited_in_list_filter(self):
         """
         Regression test for #13902: When using a ManyToMany in list_filter,
         results shouldn't appear more than once. Target of the relationship
@@ -403,11 +418,15 @@ class ChangeListTests(TestCase):
 
         # There's only one ChordsBand instance
         self.assertEqual(cl.result_count, 1)
+        # Queryset must be deletable.
+        self.assertIs(cl.queryset.query.distinct, False)
+        cl.queryset.delete()
+        self.assertEqual(cl.queryset.count(), 0)
 
-    def test_distinct_for_non_unique_related_object_in_list_filter(self):
+    def test_no_duplicates_for_non_unique_related_object_in_list_filter(self):
         """
-        Regressions tests for #15819: If a field listed in list_filters
-        is a non-unique related object, distinct() must be called.
+        Regressions tests for #15819: If a field listed in list_filters is a
+        non-unique related object, results shouldn't appear more than once.
         """
         parent = Parent.objects.create(name='Mary')
         # Two children with the same name
@@ -419,8 +438,12 @@ class ChangeListTests(TestCase):
         request.user = self.superuser
 
         cl = m.get_changelist_instance(request)
-        # Make sure distinct() was called
+        # Exists() is applied.
         self.assertEqual(cl.queryset.count(), 1)
+        # Queryset must be deletable.
+        self.assertIs(cl.queryset.query.distinct, False)
+        cl.queryset.delete()
+        self.assertEqual(cl.queryset.count(), 0)
 
     def test_changelist_search_form_validation(self):
         m = ConcertAdmin(Concert, custom_site)
@@ -438,10 +461,10 @@ class ChangeListTests(TestCase):
                 self.assertEqual(1, len(messages))
                 self.assertEqual(error, messages[0])
 
-    def test_distinct_for_non_unique_related_object_in_search_fields(self):
+    def test_no_duplicates_for_non_unique_related_object_in_search_fields(self):
         """
         Regressions tests for #15819: If a field listed in search_fields
-        is a non-unique related object, distinct() must be called.
+        is a non-unique related object, Exists() must be applied.
         """
         parent = Parent.objects.create(name='Mary')
         Child.objects.create(parent=parent, name='Danielle')
@@ -452,13 +475,17 @@ class ChangeListTests(TestCase):
         request.user = self.superuser
 
         cl = m.get_changelist_instance(request)
-        # Make sure distinct() was called
+        # Exists() is applied.
         self.assertEqual(cl.queryset.count(), 1)
+        # Queryset must be deletable.
+        self.assertIs(cl.queryset.query.distinct, False)
+        cl.queryset.delete()
+        self.assertEqual(cl.queryset.count(), 0)
 
-    def test_distinct_for_many_to_many_at_second_level_in_search_fields(self):
+    def test_no_duplicates_for_many_to_many_at_second_level_in_search_fields(self):
         """
         When using a ManyToMany in search_fields at the second level behind a
-        ForeignKey, distinct() must be called and results shouldn't appear more
+        ForeignKey, Exists() must be applied and results shouldn't appear more
         than once.
         """
         lead = Musician.objects.create(name='Vox')
@@ -474,6 +501,10 @@ class ChangeListTests(TestCase):
         cl = m.get_changelist_instance(request)
         # There's only one Concert instance
         self.assertEqual(cl.queryset.count(), 1)
+        # Queryset must be deletable.
+        self.assertIs(cl.queryset.query.distinct, False)
+        cl.queryset.delete()
+        self.assertEqual(cl.queryset.count(), 0)
 
     def test_pk_in_search_fields(self):
         band = Group.objects.create(name='The Hype')
@@ -566,23 +597,23 @@ class ChangeListTests(TestCase):
         cl = m.get_changelist_instance(request)
         self.assertCountEqual(cl.queryset, [abcd])
 
-    def test_no_distinct_for_m2m_in_list_filter_without_params(self):
+    def test_no_exists_for_m2m_in_list_filter_without_params(self):
         """
         If a ManyToManyField is in list_filter but isn't in any lookup params,
-        the changelist's query shouldn't have distinct.
+        the changelist's query shouldn't have Exists().
         """
         m = BandAdmin(Band, custom_site)
         for lookup_params in ({}, {'name': 'test'}):
             request = self.factory.get('/band/', lookup_params)
             request.user = self.superuser
             cl = m.get_changelist_instance(request)
-            self.assertFalse(cl.queryset.query.distinct)
+            self.assertNotIn(' EXISTS', str(cl.queryset.query))
 
-        # A ManyToManyField in params does have distinct applied.
+        # A ManyToManyField in params does have Exists() applied.
         request = self.factory.get('/band/', {'genres': '0'})
         request.user = self.superuser
         cl = m.get_changelist_instance(request)
-        self.assertTrue(cl.queryset.query.distinct)
+        self.assertIn(' EXISTS', str(cl.queryset.query))
 
     def test_pagination(self):
         """