[Django] #36404: Aggregates with filter using OuterRef raise FieldError

[Django]

#36404: Aggregates with filter using OuterRef raise FieldError
-------------------------------------+-------------------------------------
Reporter: Adam | Owner: Adam Johnson
Johnson |
Type: Bug | Status: assigned
Component: Database | Version: dev
layer (models, ORM) |
Severity: Release | Keywords:
blocker |
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
While upgrading a client project to Django 5.2, I encountered some
`FieldError`s like:

{{{#!python
FieldError: Cannot resolve keyword 'isbn' into field. Choices are: book,
book_id, id, isbn_ref
}}}

The issue is reproduced with models:

{{{#!python
from django.db import models


class Book(models.Model):
isbn = models.IntegerField()


class Chapter(models.Model):
book = models.ForeignKey(Book, on_delete=models.CASCADE)
isbn_ref = models.IntegerField()
}}}

And `QuerySet`:

{{{#!python
from django.db.models import Count, OuterRef, Q, Subquery

from example.models import Book, Chapter

qs = Book.objects.annotate(
point_count=Subquery(
Chapter.objects.annotate(
count=Count(
"id",
filter=Q(isbn_ref=OuterRef("isbn")),
)
).values("count")
)
)
}}}

Full traceback:

{{{
$ python t.py
Traceback (most recent call last):
File "/.../t.py", line 14, in <module>
Chapter.objects.annotate(
~~~~~~~~~~~~~~~~~~~~~~~~^
count=Count(
^^^^^^^^^^^^
...<2 lines>...
)
^
).values("count")
^
File "/.../django/django/db/models/manager.py", line 87, in
manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
File "/.../django/django/db/models/query.py", line 1643, in annotate
return self._annotate(args, kwargs, select=True)
~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/.../django/django/db/models/query.py", line 1695, in _annotate
clone.query.add_annotation(
~~~~~~~~~~~~~~~~~~~~~~~~~~^
annotation,
^^^^^^^^^^^
alias,
^^^^^^
select=select,
^^^^^^^^^^^^^^
)
^
File "/.../django/django/db/models/sql/query.py", line 1218, in
add_annotation
annotation = annotation.resolve_expression(self, allow_joins=True,
reuse=None)
File "/.../django/django/db/models/aggregates.py", line 271, in
resolve_expression
result = super().resolve_expression(*args, **kwargs)
File "/.../django/django/db/models/aggregates.py", line 124, in
resolve_expression
c.filter.resolve_expression(query, allow_joins, reuse, summarize)
~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/.../django/django/db/models/expressions.py", line 300, in
resolve_expression
expr.resolve_expression(query, allow_joins, reuse, summarize)
~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/.../django/django/db/models/sql/where.py", line 288, in
resolve_expression
clone._resolve_node(clone, *args, **kwargs)
~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
File "/.../django/django/db/models/sql/where.py", line 280, in
_resolve_node
cls._resolve_node(child, query, *args, **kwargs)
~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/.../django/django/db/models/sql/where.py", line 284, in
_resolve_node
node.rhs = cls._resolve_leaf(node.rhs, query, *args, **kwargs)
~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/.../django/django/db/models/sql/where.py", line 273, in
_resolve_leaf
expr = expr.resolve_expression(query, *args, **kwargs)
File "/.../django/django/db/models/expressions.py", line 958, in
resolve_expression
col = super().resolve_expression(*args, **kwargs)
File "/.../django/django/db/models/expressions.py", line 902, in
resolve_expression
return query.resolve_ref(self.name, allow_joins, reuse, summarize)
~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/.../django/django/db/models/sql/query.py", line 2049, in
resolve_ref
join_info = self.setup_joins(
field_list, self.get_meta(), self.get_initial_alias(),
can_reuse=reuse
)
File "/.../django/django/db/models/sql/query.py", line 1900, in
setup_joins
path, final_field, targets, rest = self.names_to_path(
~~~~~~~~~~~~~~~~~~^
names[:pivot],
^^^^^^^^^^^^^^
...<2 lines>...
fail_on_missing=True,
^^^^^^^^^^^^^^^^^^^^^
)
^
File "/.../django/django/db/models/sql/query.py", line 1805, in
names_to_path
raise FieldError(
...<2 lines>...
)
django.core.exceptions.FieldError: Cannot resolve keyword 'isbn' into
field. Choices are: book, book_id, id, isbn_ref
}}}

The error occurs because the `isbn` field is looked up against the inner
model, `Chapter`, rather than the outer one, `Book`.

I bisected the error to e306687a3a5507d59365ba9bf545010e5fd4b2a8. That
commit referenced #36042 and was part of a PR for #36117. The
simplification left a duplicated call to `resolve_expression()` for
`Aggregate.filter`, leading to “over resolution” and triggering the
`FieldError`. The issue is similar to #36117, which removed some duplicate
`resolve_expression()` methods from `Case` and `When`.

While fixing this issue, I spotted the same mistake had been made for the
newly added `Aggregate.order_by` from #35444, which the PR associated to
this ticket fixes in a second commit. The first commit should be
backported to 5.2, the second left only on `main`.
--
Ticket URL: <https://code.djangoproject.com/ticket/36404>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36404: Aggregates with filter using OuterRef raise FieldError
-------------------------------------+-------------------------------------

Reporter: Adam Johnson | Owner: Adam

| Johnson
Type: Bug | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Adam Johnson:

Old description:

New description:

While upgrading a client project to Django 5.2, I encountered some
`FieldError`s like:

{{{#!python
FieldError: Cannot resolve keyword 'isbn' into field. Choices are: book,
book_id, id, isbn_ref
}}}

The issue is reproduced [https://github.com/adamchainz/django-ticket-36404
in this example project], with models:

--
Ticket URL: <https://code.djangoproject.com/ticket/36404#comment:1>

[Django]

#36404: Aggregates with filter using OuterRef raise FieldError
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
| Johnson
Type: Bug | Status: assigned

Component: Database layer | Version: 5.2

(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* stage: Unreviewed => Accepted
* version: dev => 5.2

Comment:

Thank you I've replicated

> While fixing this issue, I spotted the same mistake had been made for
the newly added Aggregate.order_by from #35444

For admin purposes, I think it might make sense to open a separate ticket
for this as a release blocker for "dev" with a different regression commit
etc
The one for 5.2 would require a release note
--
Ticket URL: <https://code.djangoproject.com/ticket/36404#comment:2>

[Django]

#36404: Aggregates with filter using OuterRef raise FieldError
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
| Johnson
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam Johnson):

Alright, I made a separate ticket for `order_by` in #36405.
--
Ticket URL: <https://code.djangoproject.com/ticket/36404#comment:3>

[Django]

#36404: Aggregates with filter using OuterRef raise FieldError
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
| Johnson
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* cc: Simon Charette (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/36404#comment:4>

[Django]

#36404: Aggregates with filter using OuterRef raise FieldError
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
| Johnson
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

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

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36404#comment:5>

[Django]

#36404: Aggregates with filter using OuterRef raise FieldError
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
| Johnson

Type: Bug | Status: closed

Component: Database layer | Version: 5.2
(models, ORM) |

Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce <42296566+sarahboyce@…>):

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

Comment:

In [changeset:"b8e5a8a9a2a767f584cbe89a878a42363706f939" b8e5a8a]:
{{{#!CommitTicketReference repository=""
revision="b8e5a8a9a2a767f584cbe89a878a42363706f939"
Fixed #36404 -- Fixed Aggregate.filter using OuterRef.

Regression in a76035e925ff4e6d8676c65cb135c74b993b1039.
Thank you to Simon Charette for the review.

co-authored-by: Simon Charette <chare...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36404#comment:6>

[Django]

#36404: Aggregates with filter using OuterRef raise FieldError
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
| Johnson
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| 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:"c29e3092fdb1136f0945ce2be8f14c3853d6c8da" c29e3092]:
{{{#!CommitTicketReference repository=""
revision="c29e3092fdb1136f0945ce2be8f14c3853d6c8da"
[5.2.x] Fixed #36404 -- Fixed Aggregate.filter using OuterRef.

Regression in a76035e925ff4e6d8676c65cb135c74b993b1039.
Thank you to Simon Charette for the review.

co-authored-by: Simon Charette <chare...@gmail.com>

Backport of b8e5a8a9a2a767f584cbe89a878a42363706f939 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36404#comment:7>

[Django]

#36404: Aggregates with filter using OuterRef raise FieldError
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
| Johnson
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by GitHub <noreply@…>):

In [changeset:"2a6e0bd72d4a69725b957d6748a4b834f21b12b5" 2a6e0bd7]:
{{{#!CommitTicketReference repository=""
revision="2a6e0bd72d4a69725b957d6748a4b834f21b12b5"
Fixed #36751 -- Fixed empty filtered aggregation crash over annotated
queryset.

Regression in b8e5a8a9a2a767f584cbe89a878a42363706f939.

Refs #36404.

The replace_expressions method was innapropriately dealing with falsey
but not None source expressions causing them to also be potentially
evaluated when __bool__ was invoked (e.g. QuerySet.__bool__ evaluates
the queryset).

The changes introduced in b8e5a8a9a2, which were to deal with a similar
issue, surfaced the problem as aggregation over an annotated queryset
requires an inlining (or pushdown) of aggregate references which is
achieved through replace_expressions.

In cases where an empty Q object was provided as an aggregate filter,
such as when the admin facetting feature was used as reported, it would
wrongly be turned into None, instead of an empty WhereNode, causing a
crash at aggregate filter compilation.

Note that the crash signature differed depending on whether or not the
backend natively supports aggregate filtering
(supports_aggregate_filter_clause) as the fallback, which makes use
Case / When expressions, would result in a TypeError instead of a
NoneType AttributeError.

Thanks Rafael Urben for the report, Antoliny and Youngkwang Yang for
the triage.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36404#comment:8>

[Django]

#36404: Aggregates with filter using OuterRef raise FieldError
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
| Johnson
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| 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:"abce62994620956c3c9cce6edcebf8fcfb2670d4" abce629]:
{{{#!CommitTicketReference repository=""
revision="abce62994620956c3c9cce6edcebf8fcfb2670d4"
[6.0.x] Fixed #36751 -- Fixed empty filtered aggregation crash over

annotated queryset.

Regression in b8e5a8a9a2a767f584cbe89a878a42363706f939.

Refs #36404.

The replace_expressions method was innapropriately dealing with falsey
but not None source expressions causing them to also be potentially
evaluated when __bool__ was invoked (e.g. QuerySet.__bool__ evaluates
the queryset).

The changes introduced in b8e5a8a9a2, which were to deal with a similar
issue, surfaced the problem as aggregation over an annotated queryset
requires an inlining (or pushdown) of aggregate references which is
achieved through replace_expressions.

In cases where an empty Q object was provided as an aggregate filter,
such as when the admin facetting feature was used as reported, it would
wrongly be turned into None, instead of an empty WhereNode, causing a
crash at aggregate filter compilation.

Note that the crash signature differed depending on whether or not the
backend natively supports aggregate filtering
(supports_aggregate_filter_clause) as the fallback, which makes use
Case / When expressions, would result in a TypeError instead of a
NoneType AttributeError.

Thanks Rafael Urben for the report, Antoliny and Youngkwang Yang for
the triage.

Backport of 2a6e0bd72d4a69725b957d6748a4b834f21b12b5 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36404#comment:9>

[Django]

#36404: Aggregates with filter using OuterRef raise FieldError
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
| Johnson
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| 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:"1e7327770c908cb51f888c9269322247fa3525a5" 1e73277]:
{{{#!CommitTicketReference repository=""
revision="1e7327770c908cb51f888c9269322247fa3525a5"
[5.2.x] Fixed #36751 -- Fixed empty filtered aggregation crash over

annotated queryset.

Regression in b8e5a8a9a2a767f584cbe89a878a42363706f939.

Refs #36404.

The replace_expressions method was innapropriately dealing with falsey
but not None source expressions causing them to also be potentially
evaluated when __bool__ was invoked (e.g. QuerySet.__bool__ evaluates
the queryset).

The changes introduced in b8e5a8a9a2, which were to deal with a similar
issue, surfaced the problem as aggregation over an annotated queryset
requires an inlining (or pushdown) of aggregate references which is
achieved through replace_expressions.

In cases where an empty Q object was provided as an aggregate filter,
such as when the admin facetting feature was used as reported, it would
wrongly be turned into None, instead of an empty WhereNode, causing a
crash at aggregate filter compilation.

Note that the crash signature differed depending on whether or not the
backend natively supports aggregate filtering
(supports_aggregate_filter_clause) as the fallback, which makes use
Case / When expressions, would result in a TypeError instead of a
NoneType AttributeError.

Thanks Rafael Urben for the report, Antoliny and Youngkwang Yang for
the triage.
Backport of 2a6e0bd72d4a69725b957d6748a4b834f21b12b5 from main
}}}
--

Ticket URL: <https://code.djangoproject.com/ticket/36404#comment:10>