FIX: Updates TimeZone support for DATETIMEOFFSET type, fixes incorrect Now() in USE_TZ=True cases.#484
Conversation
This fixes the handling of the timezone part for a DATETIMEOFFSET type.
When USE_TZ=True, it's important that Now() reflect the current time, with timezone offset. If the database system OS is not using UTC , the SYSDATETIME() is a local-timezone value, which incorrectly CASTED as UTC (because offset = 00:00). This fix and the previous one allows to leverage the timezone part of SYSDATETIMEOFFSET() to ensure the timezone offset makes Now() the correct value.
|
@microsoft-github-policy-service agree |
|
An explainer for fix 2. sqlserver_now Notably, if your model still use a DATETIME2 column, and the Database timezone is not UTC, you will still experience a bug when using Now() or a model with |
There was a problem hiding this comment.
Pull request overview
This PR improves timezone correctness in the SQL Server backend by (1) decoding DATETIMEOFFSET values into timezone-aware Python datetimes (including the stored offset) and (2) updating the SQL emitted for Django’s Now() so tz-aware projects don’t receive misleading “local time treated as UTC” results.
Changes:
- Parse
DATETIMEOFFSETbinary values intodatetimeobjects with atzinfoderived from the stored offset. - Emit
SYSDATETIMEOFFSET()forNow()whensettings.USE_TZ=True(and keepSYSDATETIME()otherwise).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
mssql/base.py |
Extends handle_datetimeoffset() to include timezone offset when constructing Python datetime values. |
mssql/functions.py |
Updates Now() SQL generation to use SYSDATETIMEOFFSET() under USE_TZ. |
…t tests - Add missing 'from django.conf import settings' to functions.py (fixes NameError when Now() is compiled) - Update handle_datetimeoffset comment to document full 9-element tuple - Add tests for positive/negative timezone offsets in handle_datetimeoffset - Add NowSQLTemplateTests to verify SYSDATETIMEOFFSET vs SYSDATETIME based on USE_TZ setting - Remove unnecessary else branch in sqlserver_now Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cover edge cases raised during review: -09:30 (Marquesas) and +05:45 (Nepal) to verify signed offset components are handled correctly by handle_datetimeoffset. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…ame) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@trolldbois - thanks a lot for raising this PR |
handle_datetimeoffset Fixes the datetime parsing of column type DATETIMEOFFSET. This updates Pull Add TimeZone support #140 that forgot to parse the timezone offset part of the type. This is a straightforward bugfix. (see Datetimeoffset support ? #136 )
sqlserver_now Fixes Now(), to use timezone-aware SYSDATETIMEOFFSET(), to ensure a timezone aware (USE_TZ) django get a correct value for Now() when the database OS is not configured as UTC timezone.
This also fixes #371 .