Twiink commented on code in PR #45671:
URL: https://github.com/apache/superset/pull/45671#discussion_r4414286037
##########
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:
Fixed in d498877b. The shared normalization distinguishes DATE from
timestamp columns: explicit DATE bounds retain their calendar dates instead of
shifting to the previous UTC date. Added SQLite/PostgreSQL DATE regressions and
DATE fill_time_range coverage; the reported range remains May 1 through June 1.
##########
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)
+ end_dttm = _to_naive_utc(end_dttm)
+
if not dataset_timezone and (offset_hours := getattr(self, "offset",
0) or 0):
Review Comment:
Fixed in d498877b. Explicit-offset bounds bypass the legacy Hour Offset;
only naive bounds receive that adjustment. Added mixed-bound cases with Hour
Offset 8 and invalid-timezone fallback, and updated the SQLite execution
regressions to assert that aware bounds are converted exactly once.
##########
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:
Fixed in d498877b. The Jinja time-filter macro delegates to the datasource's
bound adjustments, with the same timestamp/DATE handling as the chart filter.
Resampling converts aware bounds to UTC for a naive index or to the index's
timezone, and fill_time_range aligns bounds with datasource-normalized display
frames. Added an actual SQLite query proving that the macro and chart both
include the 2024-03-31 20:00 UTC row, plus hourly fill tests for UTC,
timezone-aware indexes, dataset display timezone/Hour Offset, and DATE columns.
Local validation: 728 passed, 1 skipped; two independently reproduced
Windows baseline failures were deselected. Ruff, formatting, Pylint (10/10),
and Linux-target mypy passed. Remote CI for this commit has not yet been
confirmed.
--
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]
