Opened 4 years ago

Closed 4 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

Description

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?

Change History (10)

comment:1 by Simon Charette, 4 years ago

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/operations.py

    diff --git a/django/db/backends/postgresql/operations.py b/django/db/backends/postgresql/operations.py
    index c67062a4a7..3877a38b43 100644
    a b class DatabaseOperations(BaseDatabaseOperations):  
    3838        else:
    3939            return "EXTRACT('%s' FROM %s)" % (lookup_type, field_name)
    4040
    41     def date_trunc_sql(self, lookup_type, field_name):
    42         # https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-TRUNC
    43         return "DATE_TRUNC('%s', %s)" % (lookup_type, field_name)
    44 
    4541    def _prepare_tzname_delta(self, tzname):
    4642        if '+' in tzname:
    4743            return tzname.replace('+', '-')
    class DatabaseOperations(BaseDatabaseOperations):  
    5046        return tzname
    5147
    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
    5652
    class DatabaseOperations(BaseDatabaseOperations):  
    7167        # https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-TRUNC
    7268        return "DATE_TRUNC('%s', %s)" % (lookup_type, field_name)
    7369
    74     def time_trunc_sql(self, lookup_type, field_name):
     70    def date_trunc_sql(self, lookup_type, field_name, tzname=None):
     71        if tzname:
     72            field_name = self._convert_field_to_tz(field_name, tzname)
     73        # https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-TRUNC
     74        return "DATE_TRUNC('%s', %s)" % (lookup_type, field_name)
     75
     76    def time_trunc_sql(self, lookup_type, field_name, tzname=None):
     77        if tzname:
     78            field_name = self._convert_field_to_tz(field_name, tzname)
     79        field_name = self._convert_field_to_tz(field_name, tzname)
    7580        return "DATE_TRUNC('%s', %s)::time" % (lookup_type, field_name)
    7681
    7782    def json_cast_text_sql(self, field_name):
  • django/db/models/functions/datetime.py

    diff --git a/django/db/models/functions/datetime.py b/django/db/models/functions/datetime.py
    index b6594b043b..52714aa9bf 100644
    a b class TruncBase(TimezoneMixin, Transform):  
    191191
    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
Version 0, edited 4 years ago by Simon Charette (next)

comment:2 by Simon Charette, 4 years ago

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 by Serhii Romanov, 4 years ago

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

comment:4 by David Rasch, 4 years ago

Cc: David Rasch added

comment:5 by David Wobrock, 4 years ago

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

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

comment:6 by Mariusz Felisiak, 4 years ago

Patch needs improvement: set

comment:7 by David Wobrock, 4 years ago

Patch needs improvement: unset

comment:8 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin

comment:9 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In a0571c10:

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

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

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