[Django] #34527: Unexpected behavior with division in aggregation
Django
-------------------------------------+-------------------------------------
Reporter: Egor R | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 3.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I came across an unexpected behavior (code taken from a real project, and
the models are renamed):
{{{
django.__version__
# '3.2.18'
MyModel.objects.all().annotate(
score=Sum("relatedmodel__points", output_field=FloatField(),
filter=Q(relatedmodel__owner=user),)
/ Count("relatedmodel__id", filter=Q(relatedmodel__owner=user)),
).values_list("score", flat=True)
# <QuerySet [None, 10.0, 8.0, None, 9.0, None]>
MyModel.objects.all().annotate(
score=Sum("relatedmodel__points", output_field=FloatField(),
filter=Q(relatedmodel__owner=user),)
).values_list("score", flat=True)
# <QuerySet [None, 40.0, 35.0, None, 37.0, None]>
MyModel.objects.all().annotate(
score=Count("relatedmodel__id", filter=Q(relatedmodel__owner=user),),
).values_list("score", flat=True)
# <QuerySet [0, 4, 4, 0, 4, 0]>
}}}
Since we're specifying `output_field=FloatField()` for `Sum`, I expected
to get 10, 8.75 and 9.25 as the results of the first query, but I'm
getting 10.0/8.0/9.0.
I looked into SQL code generated by the query - there's no casting of
`Sum` to float there, so it's somewhat understandable why it is happening.
But, shouldn't Django cast `Sum` to float in SQL?
Explicitly casting `Count` as `FloatField` works, though:
{{{
MyModel.objects.all().annotate(
score=Sum("relatedmodel__points", output_field=FloatField(),
filter=Q(relatedmodel__owner=user),)
/ Cast(Count("relatedmodel__id", filter=Q(relatedmodel__owner=user)),
FloatField())
).values_list("score", flat=True)
# <QuerySet [None, 10.0, 8.75, None, 9.25, None]>
}}}
Relevant snippets of the models:
{{{
class MyModel(models.Model):
...
class RelatedModel(models.Model):
points = models.PositiveSmallIntegerField(
"description", default=10, null=True, blank=True
)
mymodel = models.ForeignKey("myapp.MyModel", on_delete=models.CASCADE)
owner = models.ForeignKey(User, on_delete=models.CASCADE)
...
}}}
I created an MRE, will attach it to the ticket.
Tested on Postgres 12 (real project) and 15 (MRE).
--
Ticket URL: <https://code.djangoproject.com/ticket/34527>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Egor R | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "dj_agg_bug_clean.zip" added.
zip of the MRE
Django
-------------------------------------+-------------------------------------
Reporter: Egor R | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
{{{
django.__version__
# '3.2.18'
{{{
class MyModel(models.Model):
...
PS Real project uses django-tenants, and I included it in the MRE as I
tried to track down the bug. Can retest and create MRE without django-
tenants if needed.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34527#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Egor R | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Sanders):
Hi,
Casting is required here, `output_field` just lets Django know what type
the returned data is:
> The output_field argument requires a model field instance, like
IntegerField() or BooleanField(), into which Django will load the value
after it’s retrieved from the database.
Ref: https://docs.djangoproject.com/en/4.2/ref/models/expressions
/#aggregate-expressions
--
Ticket URL: <https://code.djangoproject.com/ticket/34527#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Egor R | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Egor R):
Thanks, got it. Not quite obvious, though.
Should I close the ticket?
--
Ticket URL: <https://code.djangoproject.com/ticket/34527#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Egor R | Owner: nobody
Component: Database layer | Version: 3.2
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => invalid
--
Ticket URL: <https://code.djangoproject.com/ticket/34527#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Egor R | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Sanders):
> Thanks, got it. Not quite obvious, though.
> Should I close the ticket?
Looks like Mariusz beat me to it xD
If there's anything in the docs that you think could be improved we're
always open to suggestions! Best place to start a conversation is on the
Django Forum: https://code.djangoproject.com/wiki/DevelopersMailingList
--
Ticket URL: <https://code.djangoproject.com/ticket/34527#comment:5>