#23420 closed Bug (fixed)
Custom lookup transformers can't create DateTimeFields when USE_TZ=True
Reported by: | Andy Chosak | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.7 |
Severity: | Normal | Keywords: | lookup, transform, datetimefield |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
If you define a custom lookup transformer that converts to a DateTimeField
, and USE_TZ=True
, then making a query with the transform fails with a cryptic error.
Consider the case where you have a model that stores Unix timestamps (seconds since 1/1/1970) as positive integers. (Granted, Django supports DateTimeFields, but there may be other reasons why you want to store the raw timestamp.) It might be useful to define a custom lookup transform that makes use of MySQL's FROM_UNIXTIME to convert to DateTime representation when queried.
Take for example this model:
from django.db import models class UnixTimestamp(models.Model): ts = models.PositiveIntegerField()
It'd be nice to be able to do something like:
from datetime import datetime, timedelta one_week_ago = datetime.utcnow() - timedelta(days=7) recent_ts = UnixTimestamp.objects.filter(ts__as_datetime__gt=one_week_ago)
You should be able to do this:
class PositiveIntegerDateTimeTransform(models.Transform): lookup_name = 'as_datetime' @property def output_field(self): return models.DateTimeField() def as_sql(self, qn, connection): lhs, params = qn.compile(self.lhs) return 'from_unixtime({})'.format(lhs), params models.PositiveIntegerField.register_lookup(PositiveIntegerDateTimeTransform)
But in practice the above produces output like this (see attached unit test):
Traceback (most recent call last): File "/dev/django/tests/custom_lookups/tests.py", line 253, in test_datetime_output_field UnixTimestamp.objects.filter(ts__as_datetime__gt=year_one), File "/dev/django/django/db/models/manager.py", line 80, in manager_method return getattr(self.get_queryset(), name)(*args, **kwargs) File "/dev/django/django/db/models/query.py", line 702, in filter return self._filter_or_exclude(False, *args, **kwargs) File "/dev/django/django/db/models/query.py", line 720, in _filter_or_exclude clone.query.add_q(Q(*args, **kwargs)) File "/dev/django/django/db/models/sql/query.py", line 1319, in add_q clause, require_inner = self._add_q(where_part, self.used_aliases) File "/dev/django/django/db/models/sql/query.py", line 1346, in _add_q current_negated=current_negated, connector=connector) File "/dev/django/django/db/models/sql/query.py", line 1218, in build_filter condition = self.build_lookup(lookups, col, value) File "/dev/django/django/db/models/sql/query.py", line 1123, in build_lookup return final_lookup(lhs, rhs) File "/dev/django/django/db/models/lookups.py", line 82, in __init__ self.rhs = self.get_prep_lookup() File "/dev/django/django/db/models/lookups.py", line 85, in get_prep_lookup return self.lhs.output_field.get_prep_lookup(self.lookup_name, self.rhs) File "/dev/django/django/db/models/fields/__init__.py", line 1249, in get_prep_lookup return super(DateField, self).get_prep_lookup(lookup_type, value) File "/dev/django/django/db/models/fields/__init__.py", line 651, in get_prep_lookup return self.get_prep_value(value) File "/dev/django/django/db/models/fields/__init__.py", line 1405, in get_prep_value (self.model.__name__, self.name, value), AttributeError: 'DateTimeField' object has no attribute 'model'
This error is caused by a DateTimeField
naive timezone warning (code here) assuming that the field instance is bound to a model, which it is not in this case.
3 attachments to this ticket:
- output_field_docs.patch : clarifies documentation of
output_field
indjango.db.models.Transform
, which is currently wrong - datetime_unbound_warning.patch : modifies
DateTimeField
to warn about naive datetimes with unboundField
instances. - datetime_custom_lookup_test.patch : test that demonstrates the above-described scenario (MySQL only)
Attachments (3)
Change History (8)
by , 10 years ago
Attachment: | output_field_docs.patch added |
---|
by , 10 years ago
Attachment: | datetime_unbound_warning.patch added |
---|
by , 10 years ago
Attachment: | datetime_custom_lookup_test.patch added |
---|
comment:1 by , 10 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 10 years ago
Thanks for the review and comments @aaugustin, I've applied your suggestions. All tests pass for both SQLite and MySQL.
I've created a PR at https://github.com/django/django/pull/3326.
comment:3 by , 10 years ago
Patch needs improvement: | unset |
---|
comment:4 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
The issue is valid and patches look very good.
I would like to avoid the
\
in patch 2 and make the code more robust with this pattern:In patch 3, I would also suggest to:
UnixTimestamp
toMySQLUnixTimestamp
to make it clear that it's database-specific;save()
; creating an instance withUnixTimestamp.objects.create(ts=time.time())
in the tests will be easier to understand for future readers.Would you like to submit a pull request on GitHub? (Add yourself in AUTHORS if you aren't there already.)
Otherwise I can take care of committing the changes.