Django subtly produces incorrect query when the same keyword appears in both aggregate() and annotate()

[StefanosChaliasos]

I have the following query:

  Model.objects.annotate(foo=F('column')).aggregate(foo=Sum(F('foo')))

Initially, I was running this query on SQLite and Django was producing a result (i.e. 0).
When I switched to MySQL this query crushed with the following exception:

  Traceback (most recent call last):
  File "/dir/.env/lib/python3.8/site-packages/django/db/backends/utils.py", line 86, in _execute
    return self.cursor.execute(sql, params)
  File "/dir/.env/lib/python3.8/site-packages/django/db/backends/mysql/base.py", line 74, in execute
    return self.cursor.execute(query, args)
  File "/dir/.env/lib/python3.8/site-packages/MySQLdb/cursors.py", line 209, in execute
    res = self._query(query)
  File "/dir/.env/lib/python3.8/site-packages/MySQLdb/cursors.py", line 315, in _query
    db.query(q)
  File "/dir/.env/lib/python3.8/site-packages/MySQLdb/connections.py", line 239, in query
    _mysql.connection.query(self, query)
MySQLdb._exceptions.OperationalError: (1054, "Unknown column 'foo' in 'field list'")

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "mysql.py", line 15, in <module>
    ret2 = Model.objects.using('mysql').annotate(foo=F('column')).aggregate(foo=Sum(F('foo')))
  File "/dir/.env/lib/python3.8/site-packages/django/db/models/query.py", line 384, in aggregate
    return query.get_aggregation(self.db, kwargs)
  File "/dir/.env/lib/python3.8/site-packages/django/db/models/sql/query.py", line 502, in get_aggregation
    result = compiler.execute_sql(SINGLE)
  File "/dir/.env/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 1151, in execute_sql
    cursor.execute(sql, params)
  File "/dir/.env/lib/python3.8/site-packages/django/db/backends/utils.py", line 100, in execute
    return super().execute(sql, params)
  File "/dir/.env/lib/python3.8/site-packages/django/db/backends/utils.py", line 68, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/dir/.env/lib/python3.8/site-packages/django/db/backends/utils.py", line 77, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/dir/.env/lib/python3.8/site-packages/django/db/backends/utils.py", line 86, in _execute
    return self.cursor.execute(sql, params)
  File "/dir/.env/lib/python3.8/site-packages/django/db/utils.py", line 90, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/dir/.env/lib/python3.8/site-packages/django/db/backends/utils.py", line 86, in _execute
    return self.cursor.execute(sql, params)
  File "/dir/.env/lib/python3.8/site-packages/django/db/backends/mysql/base.py", line 74, in execute
    return self.cursor.execute(query, args)
  File "/dir/.env/lib/python3.8/site-packages/MySQLdb/cursors.py", line 209, in execute
    res = self._query(query)
  File "/dir/.env/lib/python3.8/site-packages/MySQLdb/cursors.py", line 315, in _query
    db.query(q)
  File "/dir/.env/lib/python3.8/site-packages/MySQLdb/connections.py", line 239, in query
    _mysql.connection.query(self, query)
django.db.utils.OperationalError: (1054, "Unknown column 'foo' in 'field list'")

After examining the SQL query produced by Django, I realized that the query is not correct.
Specifically, Django produced the following SQL query:

  SELECT SUM(`foo`) AS `foo` FROM `model`

Instead of

  SELECT SUM(`foo`) FROM (SELECT `model`.`column` AS `foo` FROM `model`) subquery

The issue appears when a keyword in aggregate function is the same with one of the keywords in annotate function. I initially thought that Django does not have any restriction on the keywords used in aggregate (i.e. a keyword in aggregate can be the same with the name of a column from the inspected model).

If you think that aggregate should not conflict with the annotate, then Django should produce a warning as running this query in SQLite subtly produces an incorrect result.

[Alexandr Tatarinov]

Reproduced on PostgreSQL, Django 3.0.7. Not sure, should this work or produce an error.

[Jacob Walls]

I would expect this to fail with a ValueError. aggregate() supplies an annotation, and a Query’s annotations are keyed on aliases. Supporting this would take a massive rewrite, it seems to me.

[Jacob Walls]

​PR

[Jacob Walls]

Unassigning myself because the attached patch was too restrictive.

[Herbert Poul]

Is this the same as https://code.djangoproject.com/ticket/31880 (which was fixed by ​https://github.com/django/django/pull/13431 )?

[Mariusz Felisiak]

Replying to Herbert Poul:

Is this the same as https://code.djangoproject.com/ticket/31880 (which was fixed by ​https://github.com/django/django/pull/13431 )?

No it is not a duplicate, see comment.

[Kristijan Mitrovic]

  Model.objects.annotate(foo=F('column')).aggregate(foo=Sum(F('foo')))

So in the example above, where both annotated and aggregated kwargs are named foo, the output should actually contain just the last one, right? The first annotated foo column can just be renamed and then removed from the final result (to avoid confusion).

[Edit] Note that current resulting query might not contain some significant changes within firstly called annotate method as query.annotations dictionary's value is just overridden with the new value for key foo

[Edit2] And then there's an issue about referencing the previous defined foo :( (the one that would have been randomly renamed)

[Simon Charette]

​PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In 1297c0d:

Fixed #31679 -- Delayed annotating aggregations.

By avoiding to annotate aggregations meant to be possibly pushed to an
outer query until their references are resolved it is possible to
aggregate over a query with the same alias.

Even if #34176 is a convoluted case to support, this refactor seems
worth it given the reduction in complexity it brings with regards to
annotation removal when performing a subquery pushdown.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 2ee0174:

Refs #34551 -- Fixed QuerySet.aggregate() crash on precending aggregation reference.

Regression in 1297c0d0d76a708017fe196b61a0ab324df76954.

Refs #31679.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 57f499e4:

[4.2.x] Refs #34551 -- Fixed QuerySet.aggregate() crash on precending aggregation reference.

Regression in 1297c0d0d76a708017fe196b61a0ab324df76954.

Refs #31679.

Backport of 2ee01747c32a7275a7a1a5f7862acba7db764921 from main

[Mariusz Felisiak]

Reverted in 2ee01747c32a7275a7a1a5f7862acba7db764921.