summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon Charette <charette.s@gmail.com>2017-04-29 14:53:43 -0400
committerSimon Charette <charette.s@gmail.com>2017-05-11 20:00:57 -0400
commitdaf2bd3efe53cbfc1c9fd00222b8315708023792 (patch)
tree8a511bc9124f8eb7bf7739b54ce52f5d071aef9b
parentbdf192c59357a0d8117f6f34c94fb32a51e7a774 (diff)
Fixed #28107 -- Disabled grouping of selected primary keys for unmanaged models.
The grouping caused an issue with database views as PostgreSQL's query planer isn't smart enough to introspect primary keys through views. Django doesn't support database views but documents that unmanaged models should be used to query them. Thanks powderflask for the detailed report and investigation.
-rw-r--r--django/db/models/sql/compiler.py7
-rw-r--r--tests/aggregation_regress/tests.py37
2 files changed, 43 insertions, 1 deletions
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))