Skip to content

Commit

Permalink
[4.2.x] Fixed #34987 -- Fixed queryset crash when mixing aggregate an…
Browse files Browse the repository at this point in the history
…d window annotations.

Regression in f387d02.

Just like `OrderByList` the `ExpressionList` expression used to wrap
`Window.partition_by` must implement `get_group_by_cols` to ensure the
necessary grouping when mixing window expressions with aggregate
annotations is performed against the partition members and not the
partition expression itself.

This is necessary because while `partition_by` is implemented as
a source expression of `Window` it's actually a fragment of the WINDOW
expression at the SQL level and thus it should result in a group by its
members and not the sum of them.

Thanks ElRoberto538 for the report.
Backport of e76cc93 from main
  • Loading branch information
charettes authored and felixxm committed Nov 23, 2023
1 parent 6d7313b commit cf95de9
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 13 deletions.
6 changes: 6 additions & 0 deletions django/db/models/expressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1223,6 +1223,12 @@ def as_sqlite(self, compiler, connection, **extra_context):
# Casting to numeric is unnecessary.
return self.as_sql(compiler, connection, **extra_context)

def get_group_by_cols(self):
group_by_cols = []
for partition in self.get_source_expressions():
group_by_cols.extend(partition.get_group_by_cols())
return group_by_cols


class OrderByList(Func):
template = "ORDER BY %(expressions)s"
Expand Down
4 changes: 4 additions & 0 deletions docs/releases/4.2.8.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,7 @@ Bugfixes
* Fixed a regression in Django 4.2 that caused a crash of
``QuerySet.aggregate()`` with aggregates referencing other aggregates or
window functions through conditional expressions (:ticket:`34975`).

* Fixed a regression in Django 4.2 that caused a crash when annotating a
``QuerySet`` with a ``Window`` expressions composed of a ``partition_by``
clause mixing field types and aggregation expressions (:ticket:`34987`).
28 changes: 15 additions & 13 deletions tests/expressions_window/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -837,30 +837,32 @@ def test_multiple_partitioning(self):
max=Window(
expression=Max("salary"),
partition_by=[F("department"), F("hire_date__year")],
)
),
past_department_count=Count("past_departments"),
).order_by("department", "hire_date", "name")
self.assertQuerySetEqual(
qs,
[
("Jones", 45000, "Accounting", datetime.date(2005, 11, 1), 45000),
("Jenson", 45000, "Accounting", datetime.date(2008, 4, 1), 45000),
("Williams", 37000, "Accounting", datetime.date(2009, 6, 1), 37000),
("Adams", 50000, "Accounting", datetime.date(2013, 7, 1), 50000),
("Wilkinson", 60000, "IT", datetime.date(2011, 3, 1), 60000),
("Moore", 34000, "IT", datetime.date(2013, 8, 1), 34000),
("Miller", 100000, "Management", datetime.date(2005, 6, 1), 100000),
("Johnson", 80000, "Management", datetime.date(2005, 7, 1), 100000),
("Smith", 38000, "Marketing", datetime.date(2009, 10, 1), 38000),
("Johnson", 40000, "Marketing", datetime.date(2012, 3, 1), 40000),
("Smith", 55000, "Sales", datetime.date(2007, 6, 1), 55000),
("Brown", 53000, "Sales", datetime.date(2009, 9, 1), 53000),
("Jones", 45000, "Accounting", datetime.date(2005, 11, 1), 45000, 0),
("Jenson", 45000, "Accounting", datetime.date(2008, 4, 1), 45000, 0),
("Williams", 37000, "Accounting", datetime.date(2009, 6, 1), 37000, 0),
("Adams", 50000, "Accounting", datetime.date(2013, 7, 1), 50000, 0),
("Wilkinson", 60000, "IT", datetime.date(2011, 3, 1), 60000, 0),
("Moore", 34000, "IT", datetime.date(2013, 8, 1), 34000, 0),
("Miller", 100000, "Management", datetime.date(2005, 6, 1), 100000, 1),
("Johnson", 80000, "Management", datetime.date(2005, 7, 1), 100000, 0),
("Smith", 38000, "Marketing", datetime.date(2009, 10, 1), 38000, 0),
("Johnson", 40000, "Marketing", datetime.date(2012, 3, 1), 40000, 1),
("Smith", 55000, "Sales", datetime.date(2007, 6, 1), 55000, 0),
("Brown", 53000, "Sales", datetime.date(2009, 9, 1), 53000, 0),
],
transform=lambda row: (
row.name,
row.salary,
row.department,
row.hire_date,
row.max,
row.past_department_count,
),
)

Expand Down

0 comments on commit cf95de9

Please sign in to comment.