Browse Source

Fixed #34987 -- Fixed queryset crash when mixing aggregate and window annotations.

Regression in f387d024fc75569d2a4a338bfda76cc2f328f627.

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.
Simon Charette 1 year ago
parent
commit
e76cc93b01
3 changed files with 25 additions and 13 deletions
  1. 6 0
      django/db/models/expressions.py
  2. 4 0
      docs/releases/4.2.8.txt
  3. 15 13
      tests/expressions_window/tests.py

+ 6 - 0
django/db/models/expressions.py

@@ -1265,6 +1265,12 @@ class ExpressionList(Func):
         # 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):
     allowed_default = False

+ 4 - 0
docs/releases/4.2.8.txt

@@ -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`).

+ 15 - 13
tests/expressions_window/tests.py

@@ -838,23 +838,24 @@ class WindowFunctionTests(TestCase):
             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,
@@ -862,6 +863,7 @@ class WindowFunctionTests(TestCase):
                 row.department,
                 row.hire_date,
                 row.max,
+                row.past_department_count,
             ),
         )