Opened 6 years ago
Last modified 3 weeks ago
#31506 assigned Cleanup/optimization
Clarify that ExpressionWrapper is not the tool to use when coping with DateField and timedelta arithmetic quirks on PostgreSQL and MySQL
| Reported by: | Matthieu Rigal | Owned by: | Hwayoung Cha |
|---|---|---|---|
| Component: | Documentation | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Josh Smeaton, David Sanders | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | yes |
| Easy pickings: | no | UI/UX: | no |
Description
The problem actually exists since 1.11 to 2.2, probably also in 3.X. Lastly tested with Django 2.2.12 and psycopg2 2.8.3
Take the model
StartModel(models.Model):
start = models.DateField()
When working on the queryset in this way:
next_segments = StartModel.objects.filter('start__gt': OuterRef('start')).order_by('start')
qs = StartModel.objects.annotate(
end=ExpressionWrapper(
Subquery(next_segments.values('start')[:1]) - datetime.timedelta(days=1),
output_field=DateField(),
)
)
qs.first().end is of type datetime, not date...
Attachments (1)
Change History (14)
comment:1 by , 6 years ago
| Cc: | added |
|---|---|
| Summary: | ExpressionWrapper on DateField + timedelta always returns DateTimeField, should DateField → ExpressionWrapper() doesn't respect output_field when combining DateField and timedelta on PostgreSQL and MySQL. |
| Triage Stage: | Unreviewed → Accepted |
| Version: | 2.2 → master |
by , 6 years ago
| Attachment: | test_31506.diff added |
|---|
comment:2 by , 6 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:3 by , 5 years ago
Hi, the issue is because the DateField has no field converter or db_converter to convert the value which we got from the database to the DateField type. I can implement the field converter for the data field but Is the DateField not having the field converter or db_converter by design?
comment:4 by , 5 years ago
| Cc: | added |
|---|
ExpressionWrapper allows you to specify the type returned by backend and at least on PostgreSQL it's not date:
test=# SELECT pg_typeof('1999-01-08'::date - '1 days'::interval);
pg_typeof
-----------------------------
timestamp without time zone
(1 row)
Instead you could use
ExpressionWrapper(
Subquery(next_segments.values('start')[:1]) - 1,
output_field=DateField(),
)
comment:5 by , 5 years ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
follow-up: 9 comment:6 by , 3 years ago
There are a few ways to address this but IMHO I don't think anything should be done here as this is documented PostgreSQL behaviour [1]
Therefore I'd recommend this ticket be marked wontfix.
I think it's the developers responsibility to cast in this situation instead of using ExpressionWrapper:
next_segments = StartModel.objects.filter('start__gt': OuterRef('start')).order_by('start')
qs = StartModel.objects.annotate(
end=Cast(
Subquery(next_segments.values('start')[:1]) - datetime.timedelta(days=1),
output_field=DateField(),
)
)
[1] https://www.postgresql.org/docs/current/functions-datetime.html#OPERATORS-DATETIME-TABLE
comment:7 by , 3 years ago
| Cc: | removed |
|---|
comment:8 by , 3 years ago
| Cc: | added |
|---|
comment:9 by , 3 years ago
Replying to David Sanders:
There are a few ways to address this but IMHO I don't think anything should be done here as this is documented PostgreSQL behaviour [1]
Therefore I'd recommend this ticket be marked
wontfix.
I think it's the developers responsibility to cast in this situation instead of using
ExpressionWrapper:
next_segments = StartModel.objects.filter('start__gt': OuterRef('start')).order_by('start') qs = StartModel.objects.annotate( end=Cast( Subquery(next_segments.values('start')[:1]) - datetime.timedelta(days=1), output_field=DateField(), ) )[1] https://www.postgresql.org/docs/current/functions-datetime.html#OPERATORS-DATETIME-TABLE
Thanks David!
Seeing it from a PostgreSQL perspective, it makes sense, as interval has the precision of a timestamp!
However, as this is pretty surprising from a Pythonic point-of-view, I would rather consider extending the documentation to warn about this potential trap. It should be clear that timedelta can only be cast (by Psycopg2) to an interval and therefore does not allow the use of the date +/- integer → date operation of PostgreSQL.
Besides the example with Cast one may add that in case this patterns is recurrent, it may be worth adding that creating a custom Field DateDelta translating to integer on PostrgeSQL side, could be an option...
comment:10 by , 3 years ago
Hi Matthieu,
In terms of new features for the framework, it's always a good idea to start a thread on the mailing list or forum to see what support there is for the feature: https://code.djangoproject.com/wiki/DevelopersMailingList 😊
Documentation updates are always welcomed. Small updates to documentation don't require a ticket. I'm not the authority on what the exact responsibilities are for ExpressionWrapper but I kinda view it as just glueing up the expressions with correct types without any explicit casting 🤷♂️ Will probably need to clarify that with someone closer to the ORM code.
comment:11 by , 4 weeks ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
Hi everyone, I’m Hwayoung from Djangonaut session 5 and I wanna update this ticket.
comment:12 by , 4 weeks ago
| Has patch: | set |
|---|
comment:13 by , 3 weeks ago
| Component: | Database layer (models, ORM) → Documentation |
|---|---|
| Patch needs improvement: | set |
| Summary: | ExpressionWrapper() doesn't respect output_field when combining DateField and timedelta on PostgreSQL and MySQL. → Clarify that ExpressionWrapper is not the tool to use when coping with DateField and timedelta arithmetic quirks on PostgreSQL and MySQL |
| Type: | Bug → Cleanup/optimization |
Thanks for this ticket. I reproduced this issue on MySQL and PostgreSQL. I attached a simple test.
Reproduced at 060d9d4229c436c44cf8e3a301f34c4b1f9f6c85.