Fixed #26348 -- Added TruncTime and exposed it through the __time lookup. #6744
Conversation
| @@ -616,6 +619,61 @@ that deal with date-parts can be used with ``DateField``:: | |||
| 2016-01-01 00:00:00+11:00 1 | |||
| 2014-06-01 00:00:00+10:00 1 | |||
|
|
|||
| .. versionadded:: 1.11 | |||
There was a problem hiding this comment.
These annotations go below the headings.
|
Just letting you know I do have this in my queue, but just don't have time right now. Will try and have a look during the week. What were your plans regarding Oracle? Were you looking for advice on how to tackle it, or help from someone with access to an Oracle database? |
|
No worries Josh, it's targeting 1.11 as a new feature anyway I just wanted to get familar with the internals of I should be able to add Oracle support in the next few days, I've got the VM setup locally. I just didn't feel like firing it up before getting any input from your side. |
|
Without going into a lot of detail it looks reasonably good to me. The only concern is really with something that I began - "not a DateField or DateTimeField or TimeField" seems messy from a user perspective, even though it simplifies the code. If there's a nice way to avoid this kind of error without duplicating code everywhere I think that'd be worthwhile. Will give it a better review this week sometime. |
charettes
force-pushed
the
ticket-26348
branch
2 times, most recently
from
de8b92c
to
cf5f81b
Compare
| with less precision. ``expression`` can have an ``output_field`` of either | ||
| ``TimeField`` or ``DateTimeField``. | ||
|
|
||
| Since ``TimeField``\s don't have a time component, only ``Trunc`` subclasses |
|
@jarshwah I'm thinking of fixing the "not a DateField or DateTimeField or TimeField" case in another commit. |
|
The only remaining failure is related to |
|
77b73e7 looks like the culprit. |
|
Hmm it looks like Oracle timezone conversion always dropped sub-second precision (e74e207#diff-54b46d05e1da568b3cc987c423e00c50R166) and 77b73e7 was just an adjustment to make it behave consistently when |
|
Buildbot, test on oracle. |
|
Yes, I was surprised by the behaviour of the timezone casting too. I think I was meant to come back to it and forgot to do so. Nice to see you've fixed it up, thanks! |
| @@ -128,6 +128,13 @@ def datetime_cast_date_sql(self, field_name, tzname): | |||
| sql = 'TRUNC(%s)' % field_name | |||
| return sql, [] | |||
|
|
|||
| def datetime_cast_time_sql(self, field_name, tzname): | |||
| # Since `TimeField` are stored as TIMESTAMP where the "date" part | |||
charettes
force-pushed
the
ticket-26348
branch
2 times, most recently
from
0f932c7
to
a14d563
Compare
| @@ -194,6 +194,8 @@ Models | |||
|
|
|||
| * :class:`~django.db.models.ImageField` now has a default | |||
| :data:`~django.core.validators.validate_image_file_extension` validator. | |||
| * Added support for time truncation to | |||
|
Repeated from last review: Add a mention of the new DatabaseOperations methods in the release notes, similar to DatabaseOperations.time_extract_sql in 1.9. |
Will do, sorry I missed that. |
|
@timgraham, just pushed the missing release notes. |
| @@ -263,7 +266,12 @@ Backwards incompatible changes in 1.11 | |||
| Database backend API | |||
| -------------------- | |||
|
|
|||
| * ... | |||
| * The ``DatabaseOperations.time_trunc_sql()`` method was added to support | |||
There was a problem hiding this comment.
I've been using "is" instead of "was".
| @@ -263,7 +266,12 @@ Backwards incompatible changes in 1.11 | |||
| Database backend API | |||
| -------------------- | |||
|
|
|||
| * ... | |||
| * The ``DatabaseOperations.time_trunc_sql()`` method was added to support | |||
| ``TimeField`` truncation. It should accept a ``lookup_type`` and | |||
…ing microseonds.
…ields. Thanks Josh for the amazing testing setup and Tim for the review.
…kup. Thanks Tim for the review.
https://code.djangoproject.com/ticket/26348