diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 14a727e998..2543755952 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -161,7 +161,12 @@ class SQLCompiler: # present in the grouped columns. This is done by identifying all # tables that have their primary key included in the grouped # columns and removing non-primary key columns referring to them. - pks = {expr for expr in expressions if hasattr(expr, 'target') and expr.target.primary_key} + # Unmanaged models are excluded because they could be representing + # database views on which the optimization might not be allowed. + pks = { + expr for expr in expressions + if hasattr(expr, 'target') and expr.target.primary_key and expr.target.model._meta.managed + } aliases = {expr.alias for expr in pks} expressions = [ expr for expr in expressions if expr in pks or getattr(expr, 'alias', None) not in aliases diff --git a/tests/aggregation_regress/tests.py b/tests/aggregation_regress/tests.py index bc501fc94e..7dc9a95688 100644 --- a/tests/aggregation_regress/tests.py +++ b/tests/aggregation_regress/tests.py @@ -2,6 +2,7 @@ import datetime import pickle from decimal import Decimal from operator import attrgetter +from unittest import mock from django.contrib.contenttypes.models import ContentType from django.core.exceptions import FieldError @@ -1262,6 +1263,42 @@ class AggregationTests(TestCase): ] ) + @skipUnlessDBFeature('allows_group_by_selected_pks') + def test_aggregate_ummanaged_model_columns(self): + """ + Unmanaged models are sometimes used to represent database views which + may not allow grouping by selected primary key. + """ + def assertQuerysetResults(queryset): + self.assertEqual( + [(b.name, b.num_authors) for b in queryset.order_by('name')], + [ + ('Artificial Intelligence: A Modern Approach', 2), + ('Paradigms of Artificial Intelligence Programming: Case Studies in Common Lisp', 1), + ('Practical Django Projects', 1), + ('Python Web Development with Django', 3), + ('Sams Teach Yourself Django in 24 Hours', 1), + ('The Definitive Guide to Django: Web Development Done Right', 2), + ] + ) + queryset = Book.objects.select_related('contact').annotate(num_authors=Count('authors')) + # Unmanaged origin model. + with mock.patch.object(Book._meta, 'managed', False): + _, _, grouping = queryset.query.get_compiler(using='default').pre_sql_setup() + self.assertEqual(len(grouping), len(Book._meta.fields) + 1) + for index, field in enumerate(Book._meta.fields): + self.assertIn(field.name, grouping[index][0]) + self.assertIn(Author._meta.pk.name, grouping[-1][0]) + assertQuerysetResults(queryset) + # Unmanaged related model. + with mock.patch.object(Author._meta, 'managed', False): + _, _, grouping = queryset.query.get_compiler(using='default').pre_sql_setup() + self.assertEqual(len(grouping), len(Author._meta.fields) + 1) + self.assertIn(Book._meta.pk.name, grouping[0][0]) + for index, field in enumerate(Author._meta.fields): + self.assertIn(field.name, grouping[index + 1][0]) + assertQuerysetResults(queryset) + def test_reverse_join_trimming(self): qs = Author.objects.annotate(Count('book_contact_set__contact')) self.assertIn(' JOIN ', str(qs.query))