mirrors/django
1
0
Fork 0
mirror of https://github.com/django/django.git synced 2024-12-22 17:16:24 +00:00
Code Issues 25 Releases Wiki Activity

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

Browse Source 
This reverts commit 1871182031 which
moved to using Exists() instead due to an overly strict
distinct().delete() check added in #32433.
This commit is contained in:
Simon Charette 2023-07-04 17:45:54 -04:00 committed by Mariusz Felisiak
parent 28e2077148
commit d569c1dcfe
2 changed files with 32 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
django/contrib/admin/views/main.py
View File

@ -29,7 +29,7 @@ from django.core.exceptions import (
SuspiciousOperation, SuspiciousOperation,
) )
from django.core.paginator import InvalidPage from django.core.paginator import InvalidPage
from django.db.models import Exists, F, Field, ManyToOneRel, OrderBy, OuterRef from django.db.models import F, Field, ManyToOneRel, OrderBy
from django.db.models.expressions import Combinable from django.db.models.expressions import Combinable
from django.urls import reverse from django.urls import reverse
from django.utils.deprecation import RemovedInDjango60Warning from django.utils.deprecation import RemovedInDjango60Warning
@ -566,6 +566,13 @@ class ChangeList:
# ValueError, ValidationError, or ?. # ValueError, ValidationError, or ?.
raise IncorrectLookupParameters(e) raise IncorrectLookupParameters(e)
if not qs.query.select_related:
qs = self.apply_select_related(qs)
# Set ordering.
ordering = self.get_ordering(request, qs)
qs = qs.order_by(*ordering)
# Apply search results # Apply search results
qs, search_may_have_duplicates = self.model_admin.get_search_results( qs, search_may_have_duplicates = self.model_admin.get_search_results(
request, request,
@ -580,17 +587,9 @@ class ChangeList:
) )
# Remove duplicates from results, if necessary # Remove duplicates from results, if necessary
if filters_may_have_duplicates | search_may_have_duplicates: if filters_may_have_duplicates | search_may_have_duplicates:
qs = qs.filter(pk=OuterRef("pk")) return qs.distinct()
qs = self.root_queryset.filter(Exists(qs)) else:
return qs
# Set ordering.
ordering = self.get_ordering(request, qs)
qs = qs.order_by(*ordering)
if not qs.query.select_related:
qs = self.apply_select_related(qs)
return qs
def apply_select_related(self, qs): def apply_select_related(self, qs):
if self.list_select_related is True: if self.list_select_related is True:

52
tests/admin_changelist/tests.py
View File

@ -467,7 +467,7 @@ class ChangeListTests(TestCase):
cl.get_results(request) cl.get_results(request)
self.assertIsInstance(cl.paginator, CustomPaginator) self.assertIsInstance(cl.paginator, CustomPaginator)
def test_no_duplicates_for_m2m_in_list_filter(self): def test_distinct_for_m2m_in_list_filter(self):
""" """
Regression test for #13902: When using a ManyToMany in list_filter, Regression test for #13902: When using a ManyToMany in list_filter,
results shouldn't appear more than once. Basic ManyToMany. results shouldn't appear more than once. Basic ManyToMany.
@ -488,11 +488,10 @@ class ChangeListTests(TestCase):
# There's only one Group instance # There's only one Group instance
self.assertEqual(cl.result_count, 1) self.assertEqual(cl.result_count, 1)
# Queryset must be deletable. # Queryset must be deletable.
self.assertIs(cl.queryset.query.distinct, False)
cl.queryset.delete() cl.queryset.delete()
self.assertEqual(cl.queryset.count(), 0) self.assertEqual(cl.queryset.count(), 0)
def test_no_duplicates_for_through_m2m_in_list_filter(self): def test_distinct_for_through_m2m_in_list_filter(self):
""" """
Regression test for #13902: When using a ManyToMany in list_filter, Regression test for #13902: When using a ManyToMany in list_filter,
results shouldn't appear more than once. With an intermediate model. results shouldn't appear more than once. With an intermediate model.
@ -512,14 +511,14 @@ class ChangeListTests(TestCase):
# There's only one Group instance # There's only one Group instance
self.assertEqual(cl.result_count, 1) self.assertEqual(cl.result_count, 1)
# Queryset must be deletable. # Queryset must be deletable.
self.assertIs(cl.queryset.query.distinct, False)
cl.queryset.delete() cl.queryset.delete()
self.assertEqual(cl.queryset.count(), 0) self.assertEqual(cl.queryset.count(), 0)
def test_no_duplicates_for_through_m2m_at_second_level_in_list_filter(self): def test_distinct_for_through_m2m_at_second_level_in_list_filter(self):
""" """
When using a ManyToMany in list_filter at the second level behind a When using a ManyToMany in list_filter at the second level behind a
ForeignKey, results shouldn't appear more than once. ForeignKey, distinct() must be called and results shouldn't appear more
than once.
""" """
lead = Musician.objects.create(name="Vox") lead = Musician.objects.create(name="Vox")
band = Group.objects.create(name="The Hype") band = Group.objects.create(name="The Hype")
@ -537,11 +536,10 @@ class ChangeListTests(TestCase):
# There's only one Concert instance # There's only one Concert instance
self.assertEqual(cl.result_count, 1) self.assertEqual(cl.result_count, 1)
# Queryset must be deletable. # Queryset must be deletable.
self.assertIs(cl.queryset.query.distinct, False)
cl.queryset.delete() cl.queryset.delete()
self.assertEqual(cl.queryset.count(), 0) self.assertEqual(cl.queryset.count(), 0)
def test_no_duplicates_for_inherited_m2m_in_list_filter(self): def test_distinct_for_inherited_m2m_in_list_filter(self):
""" """
Regression test for #13902: When using a ManyToMany in list_filter, Regression test for #13902: When using a ManyToMany in list_filter,
results shouldn't appear more than once. Model managed in the results shouldn't appear more than once. Model managed in the
@ -562,11 +560,10 @@ class ChangeListTests(TestCase):
# There's only one Quartet instance # There's only one Quartet instance
self.assertEqual(cl.result_count, 1) self.assertEqual(cl.result_count, 1)
# Queryset must be deletable. # Queryset must be deletable.
self.assertIs(cl.queryset.query.distinct, False)
cl.queryset.delete() cl.queryset.delete()
self.assertEqual(cl.queryset.count(), 0) self.assertEqual(cl.queryset.count(), 0)
def test_no_duplicates_for_m2m_to_inherited_in_list_filter(self): def test_distinct_for_m2m_to_inherited_in_list_filter(self):
""" """
Regression test for #13902: When using a ManyToMany in list_filter, Regression test for #13902: When using a ManyToMany in list_filter,
results shouldn't appear more than once. Target of the relationship results shouldn't appear more than once. Target of the relationship
@ -586,15 +583,11 @@ class ChangeListTests(TestCase):
# There's only one ChordsBand instance # There's only one ChordsBand instance
self.assertEqual(cl.result_count, 1) 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_no_duplicates_for_non_unique_related_object_in_list_filter(self): def test_distinct_for_non_unique_related_object_in_list_filter(self):
""" """
Regressions tests for #15819: If a field listed in list_filters is a Regressions tests for #15819: If a field listed in list_filters
non-unique related object, results shouldn't appear more than once. is a non-unique related object, distinct() must be called.
""" """
parent = Parent.objects.create(name="Mary") parent = Parent.objects.create(name="Mary")
# Two children with the same name # Two children with the same name
@ -606,10 +599,9 @@ class ChangeListTests(TestCase):
request.user = self.superuser request.user = self.superuser
cl = m.get_changelist_instance(request) cl = m.get_changelist_instance(request)
# Exists() is applied. # Make sure distinct() was called
self.assertEqual(cl.queryset.count(), 1) self.assertEqual(cl.queryset.count(), 1)
# Queryset must be deletable. # Queryset must be deletable.
self.assertIs(cl.queryset.query.distinct, False)
cl.queryset.delete() cl.queryset.delete()
self.assertEqual(cl.queryset.count(), 0) self.assertEqual(cl.queryset.count(), 0)
@ -629,10 +621,10 @@ class ChangeListTests(TestCase):
self.assertEqual(1, len(messages)) self.assertEqual(1, len(messages))
self.assertEqual(error, messages[0]) self.assertEqual(error, messages[0])
def test_no_duplicates_for_non_unique_related_object_in_search_fields(self): def test_distinct_for_non_unique_related_object_in_search_fields(self):
""" """
Regressions tests for #15819: If a field listed in search_fields Regressions tests for #15819: If a field listed in search_fields
is a non-unique related object, Exists() must be applied. is a non-unique related object, distinct() must be called.
""" """
parent = Parent.objects.create(name="Mary") parent = Parent.objects.create(name="Mary")
Child.objects.create(parent=parent, name="Danielle") Child.objects.create(parent=parent, name="Danielle")
@ -643,17 +635,16 @@ class ChangeListTests(TestCase):
request.user = self.superuser request.user = self.superuser
cl = m.get_changelist_instance(request) cl = m.get_changelist_instance(request)
# Exists() is applied. # Make sure distinct() was called
self.assertEqual(cl.queryset.count(), 1) self.assertEqual(cl.queryset.count(), 1)
# Queryset must be deletable. # Queryset must be deletable.
self.assertIs(cl.queryset.query.distinct, False)
cl.queryset.delete() cl.queryset.delete()
self.assertEqual(cl.queryset.count(), 0) self.assertEqual(cl.queryset.count(), 0)
def test_no_duplicates_for_many_to_many_at_second_level_in_search_fields(self): def test_distinct_for_many_to_many_at_second_level_in_search_fields(self):
""" """
When using a ManyToMany in search_fields at the second level behind a When using a ManyToMany in search_fields at the second level behind a
ForeignKey, Exists() must be applied and results shouldn't appear more ForeignKey, distinct() must be called and results shouldn't appear more
than once. than once.
""" """
lead = Musician.objects.create(name="Vox") lead = Musician.objects.create(name="Vox")
@ -670,7 +661,6 @@ class ChangeListTests(TestCase):
# There's only one Concert instance # There's only one Concert instance
self.assertEqual(cl.queryset.count(), 1) self.assertEqual(cl.queryset.count(), 1)
# Queryset must be deletable. # Queryset must be deletable.
self.assertIs(cl.queryset.query.distinct, False)
cl.queryset.delete() cl.queryset.delete()
self.assertEqual(cl.queryset.count(), 0) self.assertEqual(cl.queryset.count(), 0)
@ -820,23 +810,23 @@ class ChangeListTests(TestCase):
cl = m.get_changelist_instance(request) cl = m.get_changelist_instance(request)
self.assertCountEqual(cl.queryset, [abcd]) self.assertCountEqual(cl.queryset, [abcd])
def test_no_exists_for_m2m_in_list_filter_without_params(self): def test_no_distinct_for_m2m_in_list_filter_without_params(self):
""" """
If a ManyToManyField is in list_filter but isn't in any lookup params, If a ManyToManyField is in list_filter but isn't in any lookup params,
the changelist's query shouldn't have Exists(). the changelist's query shouldn't have distinct.
""" """
m = BandAdmin(Band, custom_site) m = BandAdmin(Band, custom_site)
for lookup_params in ({}, {"name": "test"}): for lookup_params in ({}, {"name": "test"}):
request = self.factory.get("/band/", lookup_params) request = self.factory.get("/band/", lookup_params)
request.user = self.superuser request.user = self.superuser
cl = m.get_changelist_instance(request) cl = m.get_changelist_instance(request)
self.assertNotIn(" EXISTS", str(cl.queryset.query)) self.assertIs(cl.queryset.query.distinct, False)
# A ManyToManyField in params does have Exists() applied. # A ManyToManyField in params does have distinct applied.
request = self.factory.get("/band/", {"genres": "0"}) request = self.factory.get("/band/", {"genres": "0"})
request.user = self.superuser request.user = self.superuser
cl = m.get_changelist_instance(request) cl = m.get_changelist_instance(request)
self.assertIn(" EXISTS", str(cl.queryset.query)) self.assertIs(cl.queryset.query.distinct, True)
def test_pagination(self): def test_pagination(self):
""" """