Re: [Django] #34975: aggregate() crashes when referencing existing aggregations or window expressions through conditional expressions

17 views
Skip to first unread message

Django

unread,
Nov 18, 2023, 1:49:35 AM11/18/23
to django-...@googlegroups.com
#34975: aggregate() crashes when referencing existing aggregations or window
expressions through conditional expressions
-------------------------------------+-------------------------------------
Reporter: Sergey Nesterenko | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: QuerySet, Window, | Triage Stage: Accepted
Aggregate, F |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sergey Nesterenko):

Replying to [comment:7 Simon Charette]:
> `filter=Q(new_ordering=F('ordering'))` instead of
`filter=Q(ordering=F('new_ordering'))`.

Yes, it solves my problem. Thankx!

--
Ticket URL: <https://code.djangoproject.com/ticket/34975#comment:11>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Nov 18, 2023, 9:43:12 AM11/18/23
to django-...@googlegroups.com
#34975: aggregate() crashes when referencing existing aggregations or window
expressions through conditional expressions
-------------------------------------+-------------------------------------
Reporter: Sergey Nesterenko | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: QuerySet, Window, | Triage Stage: Ready for
Aggregate, F | checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/34975#comment:12>

Django

unread,
Nov 18, 2023, 10:52:20 AM11/18/23
to django-...@googlegroups.com
#34975: aggregate() crashes when referencing existing aggregations or window
expressions through conditional expressions
-------------------------------------+-------------------------------------
Reporter: Sergey Nesterenko | Owner: Simon
| Charette
Type: Bug | Status: closed

Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed

Keywords: QuerySet, Window, | Triage Stage: Ready for
Aggregate, F | 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:"911b1619ab62317aaa7594d5fd52c1b8e43969a0" 911b161]:
{{{
#!CommitTicketReference repository=""
revision="911b1619ab62317aaa7594d5fd52c1b8e43969a0"
Refs #34975 -- Handled optional source expressions in
Expression.get_refs().

While no code is directly exercising get_refs in a way that triggers
a crash some expressions such as Window stash None in source_expressions
which can obscure the origin of some bugs.

Handling None values like we do in other source_expression traversing
methods such as .contains_aggregates ensures we don't run into surprises
in the future where get_refs() might be used for a different purpose.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34975#comment:15>

Django

unread,
Nov 18, 2023, 10:52:20 AM11/18/23
to django-...@googlegroups.com
#34975: aggregate() crashes when referencing existing aggregations or window
expressions through conditional expressions
-------------------------------------+-------------------------------------
Reporter: Sergey Nesterenko | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: QuerySet, Window, | Triage Stage: Ready for
Aggregate, F | 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:"15cb3c262a7df568bb5c0e3e07d5078c08ef59f4" 15cb3c26]:
{{{
#!CommitTicketReference repository=""
revision="15cb3c262a7df568bb5c0e3e07d5078c08ef59f4"
Refs #34975 -- Complemented rhs filtering aggregations for __in lookup.

While this isn't a regression it's clear that similar logic should be
applied when dealing with lists of expressions passed as a lookup value.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34975#comment:14>

Django

unread,
Nov 18, 2023, 10:52:20 AM11/18/23
to django-...@googlegroups.com
#34975: aggregate() crashes when referencing existing aggregations or window
expressions through conditional expressions
-------------------------------------+-------------------------------------
Reporter: Sergey Nesterenko | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: QuerySet, Window, | Triage Stage: Ready for
Aggregate, F | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"7530cf3900ab98104edcde69e8a2a415e82b345a" 7530cf39]:
{{{
#!CommitTicketReference repository=""
revision="7530cf3900ab98104edcde69e8a2a415e82b345a"
Fixed #34975 -- Fixed crash of conditional aggregate() over aggregations.

Adjustments made to solve_lookup_type to defer the resolving of
references for summarized aggregates failed to account for similar
requirements for lookup values which can also reference annotations
through Aggregate.filter.

Regression in b181cae2e3697b2e53b5b67ac67e59f3b05a6f0d.

Refs #25307.

Thanks Sergey Nesterenko for the report.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34975#comment:13>

Django

unread,
Nov 18, 2023, 10:53:21 AM11/18/23
to django-...@googlegroups.com
#34975: aggregate() crashes when referencing existing aggregations or window
expressions through conditional expressions
-------------------------------------+-------------------------------------
Reporter: Sergey Nesterenko | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: QuerySet, Window, | Triage Stage: Ready for
Aggregate, F | 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:"49f1ced86398177c125e256d7e812eb34fae672e" 49f1ced8]:
{{{
#!CommitTicketReference repository=""
revision="49f1ced86398177c125e256d7e812eb34fae672e"
[5.0.x] Fixed #34975 -- Fixed crash of conditional aggregate() over
aggregations.

Adjustments made to solve_lookup_type to defer the resolving of
references for summarized aggregates failed to account for similar
requirements for lookup values which can also reference annotations
through Aggregate.filter.

Regression in b181cae2e3697b2e53b5b67ac67e59f3b05a6f0d.

Refs #25307.

Thanks Sergey Nesterenko for the report.

Backport of 7530cf3900ab98104edcde69e8a2a415e82b345a from main
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34975#comment:16>

Django

unread,
Nov 18, 2023, 10:53:44 AM11/18/23
to django-...@googlegroups.com
#34975: aggregate() crashes when referencing existing aggregations or window
expressions through conditional expressions
-------------------------------------+-------------------------------------
Reporter: Sergey Nesterenko | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: QuerySet, Window, | Triage Stage: Ready for
Aggregate, F | 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:"acf4cee95144c55a12492cdd71fa795d7accfe26" acf4cee]:
{{{
#!CommitTicketReference repository=""
revision="acf4cee95144c55a12492cdd71fa795d7accfe26"
[4.2.x] Fixed #34975 -- Fixed crash of conditional aggregate() over
aggregations.

Adjustments made to solve_lookup_type to defer the resolving of
references for summarized aggregates failed to account for similar
requirements for lookup values which can also reference annotations
through Aggregate.filter.

Regression in b181cae2e3697b2e53b5b67ac67e59f3b05a6f0d.

Refs #25307.

Thanks Sergey Nesterenko for the report.

Backport of 7530cf3900ab98104edcde69e8a2a415e82b345a from main
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34975#comment:17>

Django

unread,
Feb 6, 2025, 10:57:54 AMFeb 6
to django-...@googlegroups.com
#34975: aggregate() crashes when referencing existing aggregations or window
expressions through conditional expressions
-------------------------------------+-------------------------------------
Reporter: Sergey Nesterenko | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: QuerySet, Window, | Triage Stage: Ready for
Aggregate, F | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"0bac41fc7e4a842e8d20319cba31cc645501c245" 0bac41f]:
{{{#!CommitTicketReference repository=""
revision="0bac41fc7e4a842e8d20319cba31cc645501c245"
Refs #34975 -- Removed unnecessary lookups.In.get_refs().

Now that In.get_source_expression() includes its right-hand-side when it
contains expressions (refs #36025) it no longer requires a specialized
get_refs() method.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34975#comment:18>

Django

unread,
Feb 6, 2025, 11:03:06 AMFeb 6
to django-...@googlegroups.com
#34975: aggregate() crashes when referencing existing aggregations or window
expressions through conditional expressions
-------------------------------------+-------------------------------------
Reporter: Sergey Nesterenko | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: QuerySet, Window, | Triage Stage: Ready for
Aggregate, F | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"0690c06013808d92ca8804dd4267adebdba20ca6" 0690c060]:
{{{#!CommitTicketReference repository=""
revision="0690c06013808d92ca8804dd4267adebdba20ca6"
[5.2.x] Refs #34975 -- Removed unnecessary lookups.In.get_refs().

Now that In.get_source_expression() includes its right-hand-side when it
contains expressions (refs #36025) it no longer requires a specialized
get_refs() method.

Backport of 0bac41fc7e4a842e8d20319cba31cc645501c245 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34975#comment:19>
Reply all
Reply to author
Forward
0 new messages