[Django] #33468: aggregate() us 'default' after any annotate() generates invalid SQL
Django
-------------------------------------+-------------------------------------
Reporter: Adam | Owner: nobody
Johnson |
Type: Bug | Status: new
Component: Database | Version: 4.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I saw this on a PostgreSQL project and reproduced it with SQLite. Django
4.0.1.
Annotate (anything) then aggregate works fine:
{{{
$ ./manage.py shell
Python 3.10.2 (main, Jan 21 2022, 19:45:54) [Clang 13.0.0
(clang-1300.0.29.30)]
Type 'copyright', 'credits' or 'license' for more information
IPython 7.30.1 -- An enhanced Interactive Python. Type '?' for help.
In [1]: from django.db.models import *
In [2]: from django.db.models.functions import *
In [3]: from example.core.models import *
In [4]: Book.objects.count()
Out[4]: 95
In [5]: Book.objects.annotate(idx=F("id")).aggregate(Sum("id"))
Out[5]: {'id__sum': 4560}
}}}
But add the aggregate classes’ `default` argument (new in 4.0), and it
breaks:
{{{
In [6]: Book.objects.annotate(idx=F("id")).aggregate(Sum("id", default=0))
---------------------------------------------------------------------------
OperationalError Traceback (most recent call
last)
...
OperationalError: near "FROM": syntax error
}}}
The generated SQL:
{{{
In [7]: %debug
> /.../django/db/backends/sqlite3/base.py(416)execute()
414 return Database.Cursor.execute(self, query)
415 query = self.convert_query(query)
--> 416 return Database.Cursor.execute(self, query, params)
417
418 def executemany(self, query, param_list):
ipdb> query
'SELECT FROM (SELECT "core_book"."id" AS "idx",
COALESCE(SUM("core_book"."id"), ?) AS "id__sum" FROM "core_book")
subquery'
ipdb> params
(0,)
ipdb>
}}}
The “long form” using `Coalesce` works:
{{{
In [8]: Book.objects.annotate(idx=F("id")).aggregate(x=Coalesce(Sum("id"),
0))
Out[8]: {'x': 4560}
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33468>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Nick Pope (added)
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Thanks for the report! Would you like to prepare a patch? If not, you can
assign it me as 4.0.2 will be issued on Tuesday.
--
Ticket URL: <https://code.djangoproject.com/ticket/33468#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: nobody
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Adam Johnson):
I have had a quick look but I got a bit lost. `Aggregate.default`
generates a `Coalesce` internally so it seems a little lower level. If you
have the capacity that would be appreciated.
--
Ticket URL: <https://code.djangoproject.com/ticket/33468#comment:2>
Django
-------------------------------------+-------------------------------------
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Mariusz Felisiak
* status: new => assigned
Comment:
Replying to [comment:2 Adam Johnson]:
> If you have the capacity that would be appreciated.
Always, that's my job 😉
--
Ticket URL: <https://code.djangoproject.com/ticket/33468#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Adam Johnson):
Just to note - the title isn't quite accurate. The crash happens whether
or not the aggregate uses the annotated field.
--
Ticket URL: <https://code.djangoproject.com/ticket/33468#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
--
Ticket URL: <https://code.djangoproject.com/ticket/33468#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
This issue should be fixed by:
{{{#!diff
diff --git a/django/db/models/aggregates.py
b/django/db/models/aggregates.py
index 8c4eae7906..e4c81547c1 100644
--- a/django/db/models/aggregates.py
+++ b/django/db/models/aggregates.py
@@ -65,7 +65,9 @@ class Aggregate(Func):
if hasattr(default, 'resolve_expression'):
default = default.resolve_expression(query, allow_joins,
reuse, summarize)
c.default = None # Reset the default argument before wrapping.
- return Coalesce(c, default, output_field=c._output_field_or_none)
+ coalesce = Coalesce(c, default,
output_field=c._output_field_or_none)
+ coalesce.is_summary = True
+ return coalesce
@property
def default_alias(self):
}}}
I will prepare a proper patch in the evening.
--
Ticket URL: <https://code.djangoproject.com/ticket/33468#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Adam Johnson):
I can confirm that change fixes my test project.
--
Ticket URL: <https://code.djangoproject.com/ticket/33468#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Nick Pope):
Thanks to both of you!
--
Ticket URL: <https://code.djangoproject.com/ticket/33468#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/15375 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/33468#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Mariusz
| Felisiak
Component: Database layer | Version: 4.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:"71e7c8e73712419626f1c2b6ec036e8559a2d667" 71e7c8e]:
{{{
#!CommitTicketReference repository=""
revision="71e7c8e73712419626f1c2b6ec036e8559a2d667"
Fixed #33468 -- Fixed QuerySet.aggregate() after annotate() crash on
aggregates with default.
Thanks Adam Johnson for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33468#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Mariusz
| Felisiak
Type: Bug | Status: closed
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"aff79be03a8c787ba0cc7e00dcec9eeb8ad01c87" aff79be0]:
{{{
#!CommitTicketReference repository=""
revision="aff79be03a8c787ba0cc7e00dcec9eeb8ad01c87"
[4.0.x] Fixed #33468 -- Fixed QuerySet.aggregate() after annotate() crash
on aggregates with default.
Thanks Adam Johnson for the report.
Backport of 71e7c8e73712419626f1c2b6ec036e8559a2d667 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33468#comment:11>