Opened 5 years ago

Closed 4 weeks ago

Last modified 4 weeks ago

#14476 closed Bug (fixed)

annotate, default aggregation naming and filter annoyance

Reported by: dirleyrls Owned by: ramiro
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: annotate, FieldError
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Going straight to the point, if I do the following:

from django.db import models
...
class Answer(models.Model):
    entry = models.ForeignKey(Question)
...
qsa = Question.objects.annotate(answer_count=models.Count('answer')).filter(answer_count=0)
qsb = Question.objects.annotate(models.Count('answer')).filter(answer__count=0)

qsa will be correctly evaluted, but qsb will raise a "FieldError: Cannot resolve keyword 'count' into field. Choices are ... answer_ _count".

Now, isn't this annoying? The error message is lying!

Attachments (2)

patch (580 bytes) - added by dirleyrls 5 years ago.
14476-annotate-aggregate-r17389.diff (570 bytes) - added by mrmachine 4 years ago.

Download all attachments as: .zip

Change History (11)

Changed 5 years ago by dirleyrls

comment:1 Changed 5 years ago by dirleyrls

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

The patch applies to trunk r14233.

comment:2 Changed 5 years ago by russellm

  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

There's a slight problem here in that filter(answer__count) could be referring to a default aggregate alias, or a field called count on the answer model. However, I think we can live with that ambiguity.

Patch requires tests.

Last edited 4 weeks ago by timgraham (previous) (diff)

comment:3 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:4 Changed 4 years ago by ramiro

  • Owner set to ramiro
  • Resolution set to fixed
  • Status changed from new to closed

In [16252]:

(The changeset message doesn't reference this ticket)

comment:5 Changed 4 years ago by mrmachine

  • Easy pickings unset
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • UI/UX unset

This is still a problem for aggregates.

>>> from django.contrib.auth.models import *
>>> from django.db.models import *
>>> User.objects.annotate(xyz=Count('groups')).aggregate(Max('xyz'))
{'xyz__max': 13}
>>> User.objects.annotate(Count('groups')).aggregate(Max('groups__count'))
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Users/mrmachine/django/db/models/query.py", line 331, in aggregate
    is_summary=True)
  File "/Users/mrmachine/django/db/models/sql/query.py", line 1000, in add_aggregate
    field_list, opts, self.get_initial_alias(), False)
  File "/Users/mrmachine/django/db/models/sql/query.py", line 1288, in setup_joins
    "Choices are: %s" % (name, ", ".join(names)))
FieldError: Cannot resolve keyword 'count' into field. Choices are: id, name, permissions, user, groups__count

exclude(), filter(), values(), values_list() and order_by() all work, so I assume that aggregate() should as well.

Can we apply the same fix? Let me know if I should open a new ticket instead of re-opening this one.

Patch with updated test attached.

Last edited 4 years ago by mrmachine (previous) (diff)

Changed 4 years ago by mrmachine

comment:6 Changed 2 years ago by fhahn

  • Component changed from ORM aggregation to Database layer (models, ORM)
  • Version changed from 1.2 to master

comment:7 Changed 2 years ago by aaugustin

  • Status changed from reopened to new

comment:8 Changed 4 weeks ago by timgraham

  • Resolution set to fixed
  • Status changed from new to closed

That test case is fixed by f59fd15c4928caf3dfcbd50f6ab47be409a43b01 (Django 1.8). I will commit the test.

comment:9 Changed 4 weeks ago by Tim Graham <timograham@…>

In 3e1bb5c:

Refs #14476 -- Added a test for default annotation name access in aggregate.

Fixed in f59fd15c4928caf3dfcbd50f6ab47be409a43b01

Note: See TracTickets for help on using tickets.
Back to Top