Fixed #34699 -- Added warning about using Trunc functions in filters #18660
Conversation
…r when the timezone is not UTC to database-functions. Added tests to confirm that the documentation is correct.
There was a problem hiding this comment.
Hello! Thank you for your contribution 💪
As it's your first contribution be sure to check out the patch review checklist.
If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!
If you have any design or process questions then you can ask in the Django forum.
Welcome aboard ⛵️!
…le to be inside warning block.
…cket_34699_docs_tests
… the blacken message.
…Trunc function test
There was a problem hiding this comment.
Hello! I reviewed the changes to the date and the warning section. I confirmed that the offset for Melbourne is +11 on 2025-04-01, and the week starts on 2015-06-15 (Monday). I added some questions that came to mind as I was reading through the warning section.
| timezone naive value. This can lead to unexp 10000 ected results if you are using | ||
| timezones other than UTC. Django will store date/time values in the | ||
| database in the UTC timezone. The following example demonstrates what | ||
| happens when using the timezone "Europe/Berlin" and how to adjust for this: |
There was a problem hiding this comment.
Is "Europe/Berlin" coming from settings.TIME_ZONE?
There was a problem hiding this comment.
Yes, I am referring to the time zone in the settings file. I selected "Europe/Berlin" because the ticket's reporter had used that time zone.
| ... trunc_start=TruncSecond("start_datetime") | ||
| ... ).filter(trunc_start__lte=start) | ||
| >>> find_this_exp.count() | ||
| 0 # We expect to find one result but 0 are found |
There was a problem hiding this comment.
Would it help to include a comment that shows the value of start as well as the value of trunc_start on the database level?
There was a problem hiding this comment.
That is a good idea. It might be better to not include it as a comment in the code section though based on what I've seen in the Django documentation. Normally, a statement is run as if we're executing code and then a result is given where the comment comes after. In this case it wouldn't work to do it that way because when we print the value of trunc_start, it would be converted to a time zone aware value by the ORM. I could, instead, make a note after that explains what the values are at the database level. What do you think?
There was a problem hiding this comment.
A note sounds like a good place for this!
|
🙌 @coolbootscoder Thank you for the changes! The note clarifies how the values differ at the database and ORM level. I am not sure if the tests were meant for sanity check only or if they will be committed to the code base, but I'll leave it to other reviewers to weigh in. Eventually, you'll want to squash your commits into a single commit. You'll also want to rename the PR and format the commit message to follow the format |
Added a warning in the Trunc documentation when using Trunc in filters when the timezone is not UTC. Added tests that demonstrate that the documentation is correct.
…cket_34699_docs_tests
Trac ticket number
ticket-34699
Branch description
A warning about using Trunc functions in filters when the timezone is not UTC was added. Tests that replicate the unexpected behavior and demonstrate one of the possible workarounds was added.
Checklist
mainbranch.