#33073 closed Bug (fixed)
AttributeError when annotating with Exists(none()) and aggregation
Reported by: | Matthijs Kooijman | Owned by: | David Wobrock |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | David Wobrock, Simon Charette | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I ran into a problem with annotating a field with an Exists subquery, passing MyModel.objects.none()
to Exists. On Django main branch, this throws an AttributeError, on 2.2.24 this returns an empty queryset (and printing the query attribute of the queryset raises EmptyResultSet).
Strangely enough this only seems to occur when I add a second annotation using an aggregation (I tried Sum, Max and Count). Adding a second aggregation using a plain field (e.g. F('id')
) does not seem to trigger the problem. Each of the annotations individually also work, but not together.
It seems this already fails on the most basic (empty) model definition, no options or specific fields required.
I have not really investigated the details, I'm not too familiar with the query generating code.
Below is a testcase (ready to be included as a regression test).
--- a/tests/model_regress/tests.py +++ b/tests/model_regress/tests.py @@ -236,6 +236,18 @@ class Horse(models.Model, metaclass=HorseBase): self.assertEqual(Horse.horns, 1) + def test_exists_none_with_aggregate(self): + Party.objects.create() + qs = Party.objects.all().annotate( + foo=models.Count('id'), + bar=models.Exists(Party.objects.none()), + ) + # Throws EmptyResultSet on 2.2.24, or AttributeError: 'NothingNode' object has no attribute + # 'get_source_expressions' on 3.2.6 + qs.query + # Returns empty result on 2.2.24 or throws on 3.2.6 + self.assertEqual(len(qs), 1) + ./runtests.py model_regress.tests.ModelTests.test_exists_none_with_aggregate (... snip some stuff ...) Traceback (most recent call last): File "django/tests/model_regress/tests.py", line 249, in test_exists_empty_with_aggregate self.assertEqual(len(qs), 1) File "django/db/models/query.py", line 262, in __len__ self._fetch_all() File "django/db/models/query.py", line 1352, in _fetch_all self._result_cache = list(self._iterable_class(self)) File "django/db/models/query.py", line 51, in __iter__ results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size) File "django/db/models/sql/compiler.py", line 1185, in execute_sql sql, params = self.as_sql() File "django/db/models/sql/compiler.py", line 527, in as_sql extra_select, order_by, group_by = self.pre_sql_setup() File "django/db/models/sql/compiler.py", line 64, in pre_sql_setup group_by = self.get_group_by(self.select + extra_select, order_by) File "django/db/models/sql/compiler.py", line 129, in get_group_by cols = expr.get_group_by_cols() File "django/db/models/expressions.py", line 1159, in get_group_by_cols external_cols = self.get_external_cols() File "django/db/models/expressions.py", line 1143, in get_external_cols return self.query.get_external_cols() File "django/db/models/sql/query.py", line 1057, in get_external_cols return [ File "django/db/models/sql/query.py", line 1057, in <listcomp> return [ File "django/db/models/sql/query.py", line 1697, in _gen_cols expr.get_source_expressions(), AttributeError: 'NothingNode' object has no attribute 'get_source_expressions'
(Can be pulled from https://github.com/matthijskooijman/django/blob/testcase-exists-none/tests/model_regress/tests.py#L239)
Change History (8)
comment:1 by , 3 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Bug |
comment:2 by , 3 years ago
Hi,
Indeed, the bug seems close to #33018 but their scopes differ in my opinion.
From a quick bisect, it would seem that this regression has been introduced in #31094 and more precisely this commit https://github.com/django/django/pull/12227/commits/5a4d7285bd10bd40d9f7e574a7c421eb21094858
I'll try to have a look and submit a patch.
comment:3 by , 3 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Submitted a PR, with a tiny diff :)
comment:4 by , 3 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
Yes it is regression in 5a4d7285bd10bd40d9f7e574a7c421eb21094858.
comment:5 by , 3 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
comment:6 by , 3 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:8 by , 3 years ago
Thanks for the superquick fix. I can't quickly test it on my original problem (my project is still stuck on 2.x, really need to upgrade soon), but I'll try to remember to do so once I've upgraded :-)
Thanks for the report. It's really similar to the #33018, however the proposed patch doesn't fix this issue.