Opened 8 years ago
Last modified 7 years ago
#26650 new Cleanup/optimization
Automatically apply Cast based on output_field
Reported by: | Matt C | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | output_field expression query annotate aggregate |
Cc: | Sergey Fedoseev | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Given the following model:
import django.db.models class TempModel(django.db.models.Model): field1 = django.db.models.DecimalField(max_digits=12, decimal_places=2)
...when I run a query like so:
TempModel.objects.all().aggregate( my_sum=django.db.models.Sum( 'field1', output_field=django.db.models.DateTimeField() ) )
...the result produces a Decimal
result, no matter what output_field
is specified.
I came across this issue by simply wanting to specify the number of decimal places that would get output in queries such as this.
That led me to find #23941, which shocked me in seeing that the "sane" behaviour/implementation was replaced with something that breaks down/invalidates the API.
Then I tried the query above, with all different kinds of output_field
values and realised the problem is more systemic and severe, in the sense that specifying output_field
is often pointless/futile.
Why can't the implementation actually honour the output_field
parameter and construct SQL to cast/coerce fields into the appropriate DB field types?
The only other means (in which I am aware of) for ensuring that a query gets compiled to SQL that honours the clients' field type specifications is to use .extra()
or .raw()
, but then that is no longer DB agnostic.
I would have thought it was the job of output_field
to achieve this in a DB agnostic manner, utilising the power of the ORM.
Change History (6)
comment:1 by , 8 years ago
comment:2 by , 8 years ago
Thank you, I was not aware of the feature.
Cast()
does address my issue.
So it appears that instead of respecting output_field
as I proposed in the description of this ticket, the implementation has just tacked on more API, rather than refactoring the implementation of the existing API.
If the core devs are happy travelling down this path, I guess this ticket has nothing further to offer and hence can be closed.
comment:3 by , 8 years ago
I guess it could be worth investigating if a Cast
should be added when an output_field
is explicitly specified.
comment:4 by , 8 years ago
My suggestion would be to use the implementation of Cast()
everywhere that output_field
is used and to get rid of Cast()
from the public API.
I.e. Why does Cast()
have to be restricted to SQL functions?
comment:5 by , 8 years ago
Summary: | output_field key-word arg not respected in model query API → Automatically apply Cast based on output_field |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
Accepting for further investigation.
comment:6 by , 7 years ago
Cc: | added |
---|
Did you try using the recently introduced
Cast()
expression?