Subquery wrapping is required in QuerySet.aggregate() for aggregates referencing nested subquery.

[Haldun Komsuoglu]

Aggregation in a query employing a subquery expression fails with the following error:

ProgrammingError: ('42000', '[42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Cannot perform an aggregate function on an expression containing an aggregate or a subquery. (130) (SQLExecDirectW)')

However, the same query works in Django 4.1.10.

To generate the error for following example can be used:

class Exchange(models.Model):
  date = models.DateField()
  value = models.FloatField()

class Invoice(models.Model):
  date = models.DateField()
  gross = models.FloatField()

exchange = Subquery(Exchange.objects.filter(date__lte=OuterRef('date')).order_by('-date').values('value')[:1])

Invoice.objects.annotate(
  exchange=exchange, 
  gross_currency=F('gross') / F('exchange')
).aggregate(
  avg_gross_currency=Avg('gross_currency')
)

[Mariusz Felisiak]

Thanks for the report, however I cannot reproduce it using any builtin backend. You should try to report it to the issue tracker of the 3rd-party database backend that you're using.

[Simon Charette]

This seems to be related to #34551 somehow. The refs_subquery check added in e5c844d6f2a4ac6ae674d741b5f1fa2a688cedf4 is not transitive in the sense that it would work in cases where exchange is referenced directly but not when not through combined expression. I'm surprised that the test added there doesn't fail if the aggregation is made through another F annotation referring the subquery annotation.

[Simon Charette]

As an example the following patch

tests/aggregation/tests.py

@@ -2249 +2249 @@
         with self.assertNumQueries(1) as ctx:
             aggregate = (
                 Author.objects.annotate(
-                    total_books=Subquery(total_books_qs.values("total"))
+                    total_books=Subquery(total_books_qs.values("total")),
+                    total_books_ref=F("total_books") / 1,
                 )
-                .values("pk", "total_books")
+                .values("pk", "total_books_ref")
                 .aggregate(
-                    sum_total_books=Sum("total_books"),
+                    sum_total_books=Sum("total_books_ref"),
                 )
             )
         sql = ctx.captured_queries[0]["sql"].lower()

Makes the aggregation.tests.AggregateAnnotationPruningTests.test_referenced_subquery_requires_wrapping test fail because the total_books_ref annotation doesn't have a .subquery property. ​What I think is required here is an Expression.returns_set -> bool method (better name welcome) that is transitive in the sense that it behaves like contains_aggregate does with regards to nested expressions.

This flag would also have ​other possible use cases but I wonder if it'd be too invasive for a backport.

Here's what it a backportable solution could look like

django/db/models/expressions.py

@@ -256 +256 @@
             for expr in self.get_source_expressions()
         )
 
+    @cached_property
+    def contains_subquery(self):
+        return (
+            expr and expr.contains_subquery for expr in self.get_source_expressions()
+        )
+
     def resolve_expression(
         self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False
     ):
@@ -1544 +1550 @@
     contains_aggregate = False
     empty_result_set_value = None
     subquery = True
+    contains_subquery = True
 
     def __init__(self, queryset, output_field=None, **extra):
         # Allow the usage of both QuerySet and sql.Query objects.

django/db/models/sql/query.py

@@ -189 +189 @@
 
     filter_is_sticky = False
     subquery = False
+    contains_subquery = True
 
     # SQL-related attributes.
     # Select and related select clauses are expressions to use in the SELECT
@@ -420 +421 @@
             # members of `aggregates` to resolve against each others.
             self.append_annotation_mask([alias])
             refs_subquery |= any(
-                getattr(self.annotations[ref], "subquery", False)
+                getattr(self.annotations[ref], "contains_subquery", False)
                 for ref in aggregate.get_refs()
             )
             refs_window |= any(

[Mariusz Felisiak]

Simon, thanks for the extra details! As far as I'm aware it's a regression in 59bea9efd2768102fc9d3aedda469502c218e9b7.

[Simon Charette]

Submitted a MR without release notes as I wasn't sure if we wanted to backport to 4.2. I assume so?

[Mariusz Felisiak]

Replying to Simon Charette:

Submitted a MR without release notes as I wasn't sure if we wanted to backport to 4.2. I assume so?

Yes, I added release notes.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 3b4a5712:

Fixed #34798 -- Fixed QuerySet.aggregate() crash when referencing expressions containing subqueries.

Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7,
complements e5c844d6f2a4ac6ae674d741b5f1fa2a688cedf4.

Refs #28477, #34551.

Thanks Haldun Komsuoglu for the report.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 4ccca9ee:

[5.0.x] Fixed #34798 -- Fixed QuerySet.aggregate() crash when referencing expressions containing subqueries.

Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7,
complements e5c844d6f2a4ac6ae674d741b5f1fa2a688cedf4.

Refs #28477, #34551.

Thanks Haldun Komsuoglu for the report.

Backport of 3b4a571275d967512866012955eb0b3ae486d63c from main

[Mariusz Felisiak <felisiak.mariusz@…>]

In 803caec:

[4.2.x] Fixed #34798 -- Fixed QuerySet.aggregate() crash when referencing expressions containing subqueries.

Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7,
complements e5c844d6f2a4ac6ae674d741b5f1fa2a688cedf4.

Refs #28477, #34551.

Thanks Haldun Komsuoglu for the report.

Backport of 3b4a571275d967512866012955eb0b3ae486d63c from main