Skip to content

Commit

Permalink
[3.0.x] Fixed #31377 -- Disabled grouping by aliases on QuerySet.valu…
Browse files Browse the repository at this point in the history
…es()/values_list() when they collide with field names.

Regression in fb3f034.

Thanks Holovashchenko Vadym for the report.

Backport of 10866a1 from master
  • Loading branch information
hramezani authored and felixxm committed Mar 25, 2020
1 parent 600d7d8 commit 72652bc
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 1 deletion.
15 changes: 14 additions & 1 deletion django/db/models/sql/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -1931,6 +1931,19 @@ def set_group_by(self, allow_aliases=True):
primary key, and the query would be equivalent, the optimization
will be made automatically.
"""
# Column names from JOINs to check collisions with aliases.
if allow_aliases:
column_names = set()
seen_models = set()
for join in list(self.alias_map.values())[1:]: # Skip base table.
model = join.join_field.related_model
if model not in seen_models:
column_names.update({
field.column
for field in model._meta.local_concrete_fields
})
seen_models.add(model)

group_by = list(self.select)
if self.annotation_select:
for alias, annotation in self.annotation_select.items():
Expand All @@ -1945,7 +1958,7 @@ def set_group_by(self, allow_aliases=True):
warnings.warn(msg, category=RemovedInDjango40Warning)
group_by_cols = annotation.get_group_by_cols()
else:
if not allow_aliases:
if not allow_aliases or alias in column_names:
alias = None
group_by_cols = annotation.get_group_by_cols(alias=alias)
group_by.extend(group_by_cols)
Expand Down
4 changes: 4 additions & 0 deletions docs/releases/3.0.5.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ Bugfixes

* Added the ability to handle ``.po`` files containing different plural
equations for the same language (:ticket:`30439`).

* Fixed a regression in Django 3.0 where ``QuerySet.values()`` and
``values_list()`` crashed if a queryset contained an aggregation and
``Subquery()`` annotation that collides with a field name (:ticket:`31377`).
1 change: 1 addition & 0 deletions tests/aggregation/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class Author(models.Model):
name = models.CharField(max_length=100)
age = models.IntegerField()
friends = models.ManyToManyField('self', blank=True)
rating = models.FloatField(null=True)

def __str__(self):
return self.name
Expand Down
16 changes: 16 additions & 0 deletions tests/aggregation/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1190,6 +1190,22 @@ def test_aggregation_subquery_annotation_values(self):
},
])

def test_aggregation_subquery_annotation_values_collision(self):
books_rating_qs = Book.objects.filter(
publisher=OuterRef('pk'),
price=Decimal('29.69'),
).values('rating')
publisher_qs = Publisher.objects.filter(
book__contact__age__gt=20,
name=self.p1.name,
).annotate(
rating=Subquery(books_rating_qs),
contacts_count=Count('book__contact'),
).values('rating').annotate(total_count=Count('rating'))
self.assertEqual(list(publisher_qs), [
{'rating': 4.0, 'total_count': 2},
])

@skipUnlessDBFeature('supports_subqueries_in_group_by')
@skipIf(
connection.vendor == 'mysql',
Expand Down

0 comments on commit 72652bc

Please sign in to comment.