[Django] #31640: Trunc() function take tzinfo param into account only when DateTimeField() are used as output_field
Django
are used as output_field
-------------------------------------+-------------------------------------
Reporter: Serhiy | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords: trunc timezone tz
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I'm trying to use TruncDay() function like this
{{{
TruncDay('created_at', output_field=DateField(), tzinfo=tz_kyiv)
}}}
but for PostgresSQL the code are generated as
{{{
(DATE_TRUNC('day', "storage_transaction"."created_at"))
}}}
So timezone convertation like
{{{
AT TIME ZONE 'Europe/Kiev'
}}}
was totally missed from sql.
After the investigation of code I've found out that Timezone converting
are applied only when output_field=DateTimeField
{{{
def as_sql(self, compiler, connection):
inner_sql, inner_params = compiler.compile(self.lhs)
if isinstance(self.output_field, DateTimeField):
tzname = self.get_tzname()
sql = connection.ops.datetime_trunc_sql(self.kind, inner_sql,
tzname)
elif isinstance(self.output_field, DateField):
sql = connection.ops.date_trunc_sql(self.kind, inner_sql)
elif isinstance(self.output_field, TimeField):
sql = connection.ops.time_trunc_sql(self.kind, inner_sql)
else:
raise ValueError('Trunc only valid on DateField, TimeField, or
DateTimeField.')
return sql, inner_params
}}}
**Why is that? Is there any reason for it OR is it only such feature that
isn't still implemented?**
--
Ticket URL: <https://code.djangoproject.com/ticket/31640>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
are used as output_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: trunc timezone tz | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
It looks like the logic should be based off `self.lhs.output_field`
instead.
Assuming you are using PostgreSQL does the following patch addresses your
issue?
{{{#!diff
diff --git a/django/db/backends/postgresql/operations.py
b/django/db/backends/postgresql/operations.py
index c67062a4a7..3877a38b43 100644
--- a/django/db/backends/postgresql/operations.py
+++ b/django/db/backends/postgresql/operations.py
@@ -38,10 +38,6 @@ class DatabaseOperations(BaseDatabaseOperations):
else:
return "EXTRACT('%s' FROM %s)" % (lookup_type, field_name)
- def date_trunc_sql(self, lookup_type, field_name):
- # https://www.postgresql.org/docs/current/functions-datetime.html
#FUNCTIONS-DATETIME-TRUNC
- return "DATE_TRUNC('%s', %s)" % (lookup_type, field_name)
-
def _prepare_tzname_delta(self, tzname):
if '+' in tzname:
return tzname.replace('+', '-')
@@ -50,7 +46,7 @@ class DatabaseOperations(BaseDatabaseOperations):
return tzname
def _convert_field_to_tz(self, field_name, tzname):
- if settings.USE_TZ:
+ if tzname and settings.USE_TZ:
field_name = "%s AT TIME ZONE '%s'" % (field_name,
self._prepare_tzname_delta(tzname))
return field_name
@@ -71,7 +67,16 @@ class DatabaseOperations(BaseDatabaseOperations):
# https://www.postgresql.org/docs/current/functions-datetime.html
#FUNCTIONS-DATETIME-TRUNC
return "DATE_TRUNC('%s', %s)" % (lookup_type, field_name)
- def time_trunc_sql(self, lookup_type, field_name):
+ def date_trunc_sql(self, lookup_type, field_name, tzname=None):
+ if tzname:
+ field_name = self._convert_field_to_tz(field_name, tzname)
+ # https://www.postgresql.org/docs/current/functions-datetime.html
#FUNCTIONS-DATETIME-TRUNC
+ return "DATE_TRUNC('%s', %s)" % (lookup_type, field_name)
+
+ def time_trunc_sql(self, lookup_type, field_name, tzname=None):
+ if tzname:
+ field_name = self._convert_field_to_tz(field_name, tzname)
+ field_name = self._convert_field_to_tz(field_name, tzname)
return "DATE_TRUNC('%s', %s)::time" % (lookup_type, field_name)
def json_cast_text_sql(self, field_name):
diff --git a/django/db/models/functions/datetime.py
b/django/db/models/functions/datetime.py
index b6594b043b..52714aa9bf 100644
--- a/django/db/models/functions/datetime.py
+++ b/django/db/models/functions/datetime.py
@@ -191,13 +191,15 @@ class TruncBase(TimezoneMixin, Transform):
def as_sql(self, compiler, connection):
inner_sql, inner_params = compiler.compile(self.lhs)
- if isinstance(self.output_field, DateTimeField):
+ tzname = None
+ if isinstance(self.lhs.output_field, DateTimeField):
tzname = self.get_tzname()
+ if isinstance(self.output_field, DateTimeField):
sql = connection.ops.datetime_trunc_sql(self.kind, inner_sql,
tzname)
elif isinstance(self.output_field, DateField):
- sql = connection.ops.date_trunc_sql(self.kind, inner_sql)
+ sql = connection.ops.date_trunc_sql(self.kind, inner_sql,
tzname)
elif isinstance(self.output_field, TimeField):
- sql = connection.ops.time_trunc_sql(self.kind, inner_sql)
+ sql = connection.ops.time_trunc_sql(self.kind, inner_sql,
tzname)
else:
raise ValueError('Trunc only valid on DateField, TimeField,
or DateTimeField.')
return sql, inner_params
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31640#comment:1>
Django
are used as output_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
It does pass the test suite so it looks like this is definitely a bug. I
think we should go as far as raising an exception when `Extract(tzinfo)`
and `Trunc(tzinfo)` is specified for a left hand side that doesn't have a
`isinstance(lhs.output_field, DateTimeField)`.
--
Ticket URL: <https://code.djangoproject.com/ticket/31640#comment:2>
Django
are used as output_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Serhii Romanov):
Yes, the provided patch solved this issue. I come up with a similar
solution like your today.
--
Ticket URL: <https://code.djangoproject.com/ticket/31640#comment:3>
Django
are used as output_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: drasch (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/31640#comment:4>
Django
are used as output_field
-------------------------------------+-------------------------------------
| Wobrock
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: David Wobrock (added)
* owner: nobody => David Wobrock
* has_patch: 0 => 1
* status: new => assigned
Comment:
Hi,
I tried to tackle this issue, starting from the what Simon suggested.
Here's the PR with more details about the approach:
https://github.com/django/django/pull/13495
--
Ticket URL: <https://code.djangoproject.com/ticket/31640#comment:5>
Django
are used as output_field
-------------------------------------+-------------------------------------
| Wobrock
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/31640#comment:6>
Django
are used as output_field
-------------------------------------+-------------------------------------
| Wobrock
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/31640#comment:7>
Django
are used as output_field
-------------------------------------+-------------------------------------
| Wobrock
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31640#comment:8>