Re: [Django] #31136: Multiple annotations with Exists() should not group by aliases.
Django
-------------------------------------+-------------------------------------
Reporter: Sigurd Ljødal | Owner: felixxm
Type: Bug | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/12272 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/31136#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Sigurd Ljødal | Owner: felixxm
Component: Database layer | Version: 3.0
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"0f843fdd5b9b2f2307148465cd60f4e1b2befbb4" 0f843fdd]:
{{{
#!CommitTicketReference repository=""
revision="0f843fdd5b9b2f2307148465cd60f4e1b2befbb4"
Fixed #31136 -- Disabled grouping by aliases on
QuerySet.values()/values_list().
Regression in fb3f034f1c63160c0ff13c609acd01c18be12f80.
Thanks Sigurd Ljødal for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31136#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Sigurd Ljødal | Owner: felixxm
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Release blocker | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: fixed =>
Comment:
I found a regression in this fix when I was trying to reproduce #31150,
see
{{{
diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py
index f523b3ca35..0209a9221b 100644
--- a/tests/aggregation/tests.py
+++ b/tests/aggregation/tests.py
@@ -1165,6 +1165,27 @@ class AggregateTestCase(TestCase):
'Sams',
])
+ def test_aggregation_subquery_annotation_distinct(self):
+ books_qs = Book.objects.annotate(
+ first_author_the_same_age=Subquery(
+ Author.objects.filter(
+ age=OuterRef('contact__friends__age'),
+ ).order_by('age').values('id')[:1],
+ )
+ ).filter(
+ publisher=self.p1,
+ first_author_the_same_age__isnull=False,
+ ).annotate(
+ min_age=Min('contact__friends__age'),
+ ).values('name', 'min_age').distinct()
+ self.assertEqual(list(books_qs),[
+ {
+ 'name': 'The Definitive Guide to Django: Web Development
Done Right',
+ 'min_age': 29,
+ },
+ {'name': 'Practical Django Projects', 'min_age': 34},
+ ])
+
@skipUnlessDBFeature('supports_subqueries_in_group_by')
def test_group_by_subquery_annotation(self):
"""
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31136#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Sigurd Ljødal | Owner: felixxm
Type: Bug | Status: new
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by felixxm):
[https://github.com/django/django/pull/12317 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/31136#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Sigurd Ljødal | Owner: felixxm
Type: Bug | Status: new
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31136#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Sigurd Ljødal | Owner: felixxm
Type: Bug | Status: new
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"59b4e99dd00b9c36d56055b889f96885995e4240" 59b4e99]:
{{{
#!CommitTicketReference repository=""
revision="59b4e99dd00b9c36d56055b889f96885995e4240"
Refs #31136 -- Made QuerySet.values()/values_list() group only by selected
annotation.
Regression in 0f843fdd5b9b2f2307148465cd60f4e1b2befbb4.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31136#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Sigurd Ljødal | Owner: felixxm
Type: Bug | Status: new
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"a24686987f21fe6b5339cc13607c13fa906f81a8" a246869]:
{{{
#!CommitTicketReference repository=""
revision="a24686987f21fe6b5339cc13607c13fa906f81a8"
[3.0.x] Refs #31136 -- Made QuerySet.values()/values_list() group only by
selected annotation.
Regression in 0f843fdd5b9b2f2307148465cd60f4e1b2befbb4.
Backport of 59b4e99dd00b9c36d56055b889f96885995e4240 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31136#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Sigurd Ljødal | Owner: felixxm
Component: Database layer | Version: 3.0
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/31136#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Sigurd Ljødal | Owner: felixxm
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: jon.dufresne@… (added)
Comment:
Commit a24686987f21fe6b5339cc13607c13fa906f81a8 causes a regression for my
application. I'll try to pin down a minimal example but for now, here is
the backtrace:
{{{
Traceback (most recent call last):
<MY APP>
File ".../django/db/models/query.py", line 276, in __iter__
self._fetch_all()
File ".../django/db/models/query.py", line 1261, in _fetch_all
self._result_cache = list(self._iterable_class(self))
File ".../django/db/models/query.py", line 184, in __iter__
for row in compiler.results_iter(chunked_fetch=self.chunked_fetch,
chunk_size=self.chunk_size):
File ".../django/db/models/sql/compiler.py", line 1096, in results_iter
results = self.execute_sql(MULTI, chunked_fetch=chunked_fetch,
chunk_size=chunk_size)
File ".../django/db/models/sql/compiler.py", line 1144, in execute_sql
cursor.execute(sql, params)
File ".../django/db/backends/utils.py", line 68, in execute
return self._execute_with_wrappers(sql, params, many=False,
executor=self._execute)
File ".../django/db/backends/utils.py", line 77, in
_execute_with_wrappers
return executor(sql, params, many, context)
File ".../django/db/backends/utils.py", line 86, in _execute
return self.cursor.execute(sql, params)
File ".../django/db/utils.py", line 90, in __exit__
raise dj_exc_value.with_traceback(traceback) from exc_value
File ".../django/db/backends/utils.py", line 86, in _execute
return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: column "myapp_mymodel.myfield" must
appear in the GROUP BY clause or be used in an aggregate function
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31136#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Sigurd Ljødal | Owner: felixxm
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jon Dufresne):
* status: closed => new
* resolution: fixed =>
Comment:
Here is a POC test to demonstrate the regression. Fails with the
postgresql backend.
{{{
diff --git a/tests/aggregation/models.py b/tests/aggregation/models.py
index fd441fe51d..211a96d97c 100644
--- a/tests/aggregation/models.py
+++ b/tests/aggregation/models.py
@@ -42,3 +42,17 @@ class Store(models.Model):
def __str__(self):
return self.name
+
+
+class ThisOrThat(models.Model):
+ pass
+
+
+class This(models.Model):
+ thisorthat = models.ForeignKey("ThisOrThat", models.CASCADE)
+ created_at = models.DateTimeField(auto_now_add=True)
+
+
+class That(models.Model):
+ thisorthat = models.OneToOneField("ThisOrThat", models.CASCADE)
+ created_at = models.DateTimeField(auto_now_add=True)
diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py
index bef415abd4..508b9cf687 100644
--- a/tests/aggregation/tests.py
+++ b/tests/aggregation/tests.py
@@ -1239,3 +1239,20 @@ class AggregateTestCase(TestCase):
contact_publisher__isnull=False,
).annotate(count=Count('authors'))
self.assertSequenceEqual(books_qs, [book])
+
+ def test_regression(self):
+ from .models import ThisOrThat
+ from django.db.models.aggregates import Max
+ from django.db.models.functions import Coalesce
+
+ qs = (
+ ThisOrThat.objects.annotate(
+ Max("this__created_at"),
+ created_at=Coalesce(
+ "this__created_at__max", "that__created_at"
+ ),
+ )
+ .order_by("created_at")
+ .values_list("pk", flat=True)
+ )
+ list(qs)
}}}
Running this test results in the following error:
{{{
======================================================================
ERROR: test_regression (aggregation.tests.AggregateTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/jon/devel/django/django/db/backends/utils.py", line 86, in
_execute
return self.cursor.execute(sql, params)
psycopg2.errors.GroupingError: column "aggregation_that.created_at" must
appear in the GROUP BY clause or be used in an aggregate function
LINE 1: ...BY COALESCE(MAX("aggregation_this"."created_at"), "aggregati...
^
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/home/jon/devel/django/tests/aggregation/tests.py", line 1258, in
test_regression
list(qs)
File "/home/jon/devel/django/django/db/models/query.py", line 258, in
__len__
self._fetch_all()
File "/home/jon/devel/django/django/db/models/query.py", line 1261, in
_fetch_all
self._result_cache = list(self._iterable_class(self))
File "/home/jon/devel/django/django/db/models/query.py", line 184, in
__iter__
for row in compiler.results_iter(chunked_fetch=self.chunked_fetch,
chunk_size=self.chunk_size):
File "/home/jon/devel/django/django/db/models/sql/compiler.py", line
1096, in results_iter
results = self.execute_sql(MULTI, chunked_fetch=chunked_fetch,
chunk_size=chunk_size)
File "/home/jon/devel/django/django/db/models/sql/compiler.py", line
1144, in execute_sql
cursor.execute(sql, params)
File "/home/jon/devel/django/django/db/backends/utils.py", line 68, in
execute
return self._execute_with_wrappers(sql, params, many=False,
executor=self._execute)
File "/home/jon/devel/django/django/db/backends/utils.py", line 77, in
_execute_with_wrappers
return executor(sql, params, many, context)
File "/home/jon/devel/django/django/db/backends/utils.py", line 86, in
_execute
return self.cursor.execute(sql, params)
File "/home/jon/devel/django/django/db/utils.py", line 90, in __exit__
raise dj_exc_value.with_traceback(traceback) from exc_value
File "/home/jon/devel/django/django/db/backends/utils.py", line 86, in
_execute
return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: column "aggregation_that.created_at"
must appear in the GROUP BY clause or be used in an aggregate function
LINE 1: ...BY COALESCE(MAX("aggregation_this"."created_at"), "aggregati...
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31136#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Sigurd Ljødal | Owner: felixxm
Type: Bug | Status: new
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Jon I think you should create another ticket referring this one instead of
reopening this one.
Looks like something is off with `contains_aggregate` handling in
a24686987f21fe6b5339cc13607c13fa906f81a8.
Can you also reproduce the crash by chaining `annotate` calls? Could you
provide the query before and after
a24686987f21fe6b5339cc13607c13fa906f81a8.
--
Ticket URL: <https://code.djangoproject.com/ticket/31136#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: Sigurd Ljødal | Owner: felixxm
Component: Database layer | Version: 3.0
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
I've create a new ticket #31217 for this regression.
--
Ticket URL: <https://code.djangoproject.com/ticket/31136#comment:15>