Opened 3 years ago

Closed 2 years ago

#31640 closed Bug (fixed)

Trunc() function take tzinfo param into account only when DateTimeField() are used as output_field

Reported by: Serhii Romanov Owned by: David Wobrock
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: trunc timezone tz
Cc: David Rasch, David Wobrock Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


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)
            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?

Change History (10)

comment:1 Changed 3 years ago by Simon Charette

Triage Stage: UnreviewedAccepted

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?

  • django/db/backends/postgresql/

    diff --git a/django/db/backends/postgresql/ b/django/db/backends/postgresql/
    index c67062a4a7..c63e4dc8c0 100644
    a b class DatabaseOperations(BaseDatabaseOperations): 
    3838        else:
    3939            return "EXTRACT('%s' FROM %s)" % (lookup_type, field_name)
    41     def date_trunc_sql(self, lookup_type, field_name):
    42         #
    43         return "DATE_TRUNC('%s', %s)" % (lookup_type, field_name)
    4541    def _prepare_tzname_delta(self, tzname):
    4642        if '+' in tzname:
    4743            return tzname.replace('+', '-')
    class DatabaseOperations(BaseDatabaseOperations): 
    5046        return tzname
    5248    def _convert_field_to_tz(self, field_name, tzname):
    53         if settings.USE_TZ:
     49        if tzname and settings.USE_TZ:
    5450            field_name = "%s AT TIME ZONE '%s'" % (field_name, self._prepare_tzname_delta(tzname))
    5551        return field_name
    class DatabaseOperations(BaseDatabaseOperations): 
    7167        #
    7268        return "DATE_TRUNC('%s', %s)" % (lookup_type, field_name)
    74     def time_trunc_sql(self, lookup_type, field_name):
     70    def date_trunc_sql(self, lookup_type, field_name, tzname=None):
     71        field_name = self._convert_field_to_tz(field_name, tzname)
     72        #
     73        return "DATE_TRUNC('%s', %s)" % (lookup_type, field_name)
     75    def time_trunc_sql(self, lookup_type, field_name, tzname=None):
     76        field_name = self._convert_field_to_tz(field_name, tzname)
     77        field_name = self._convert_field_to_tz(field_name, tzname)
    7578        return "DATE_TRUNC('%s', %s)::time" % (lookup_type, field_name)
    7780    def json_cast_text_sql(self, field_name):
  • django/db/models/functions/

    diff --git a/django/db/models/functions/ b/django/db/models/functions/
    index b6594b043b..52714aa9bf 100644
    a b class TruncBase(TimezoneMixin, Transform): 
    192192    def as_sql(self, compiler, connection):
    193193        inner_sql, inner_params = compiler.compile(self.lhs)
    194         if isinstance(self.output_field, DateTimeField):
     194        tzname = None
     195        if isinstance(self.lhs.output_field, DateTimeField):
    195196            tzname = self.get_tzname()
     197        if isinstance(self.output_field, DateTimeField):
    196198            sql = connection.ops.datetime_trunc_sql(self.kind, inner_sql, tzname)
    197199        elif isinstance(self.output_field, DateField):
    198             sql = connection.ops.date_trunc_sql(self.kind, inner_sql)
     200            sql = connection.ops.date_trunc_sql(self.kind, inner_sql, tzname)
    199201        elif isinstance(self.output_field, TimeField):
    200             sql = connection.ops.time_trunc_sql(self.kind, inner_sql)
     202            sql = connection.ops.time_trunc_sql(self.kind, inner_sql, tzname)
    201203        else:
    202204            raise ValueError('Trunc only valid on DateField, TimeField, or DateTimeField.')
    203205        return sql, inner_params
Last edited 3 years ago by Simon Charette (previous) (diff)

comment:2 Changed 3 years ago 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).

comment:3 Changed 3 years ago by Serhii Romanov

Yes, the provided patch solved this issue. I come up with a similar solution like your today.

comment:4 Changed 3 years ago by David Rasch

Cc: David Rasch added

comment:5 Changed 2 years ago by David Wobrock

Cc: David Wobrock added
Has patch: set
Owner: changed from nobody to David Wobrock
Status: newassigned


I tried to tackle this issue, starting from the what Simon suggested.

Here's the PR with more details about the approach:

comment:6 Changed 2 years ago by Mariusz Felisiak

Patch needs improvement: set

comment:7 Changed 2 years ago by David Wobrock

Patch needs improvement: unset

comment:8 Changed 2 years ago by Mariusz Felisiak

Triage Stage: AcceptedReady for checkin

comment:9 Changed 2 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In a0571c10:

Refs #31640 -- Made Extract raise ValueError when using tzinfo with Date/TimeField.

comment:10 Changed 2 years ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In ee00532:

Fixed #31640 -- Made Trunc() truncate datetimes to Date/TimeField in a specific timezone.

Note: See TracTickets for help on using tickets.
Back to Top