sadpandajoe commented on code in PR #45671:
URL: https://github.com/apache/superset/pull/45671#discussion_r4406993151
##########
superset/models/helpers.py:
##########
@@ -4611,6 +4625,13 @@ def get_time_filter( # pylint:
disable=too-many-arguments # noqa: C901
)
dataset_timezone = None
+ # Without a dataset timezone, explicit offsets still identify UTC
+ # instants. Strip tzinfo only after conversion so text-based timestamp
+ # comparisons do not depend on an engine's handling of offset suffixes.
+ if not dataset_timezone:
+ start_dttm = _to_naive_utc(start_dttm)
Review Comment:
Quarter bounds can now stay offset-aware, but only `get_time_filter`
converts them to naive UTC; other consumers of the same parsed bounds still
take them raw. The Jinja `get_time_filter('ts')` macro (`jinja_context.py`,
`_format_dttm`) passes the aware datetime straight to `convert_dttm`, so on
SQLite a `DATETRUNC(DATETIME('2024-05-31T12:00:00+08:00'), QUARTER) :
DATETIME('2024-07-01T00:00:00+08:00')` range renders `'2024-04-01
00:00:00+08:00'` where the chart filter uses `2024-03-31 16:00:00`, so a UTC
row at `2024-03-31 20:00:00` is dropped. Likewise `fill_time_range` resampling
receives the original aware bounds and `_coerce_bound` in `resample.py` strips
the offset without converting, so with one point on May 1 and hourly rules the
output spans April 1 00:00 to June 30 23:00 instead of the queried March 31
16:00 to June 30 15:00 UTC. Should the bounds be normalized once where they are
parsed, or should these two consumers convert like `get_time_filter` does?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
