aminghadersohi commented on code in PR #45671:
URL: https://github.com/apache/superset/pull/45671#discussion_r4402539044
##########
superset/mcp_service/dashboard_scope.py:
##########
@@ -773,23 +774,29 @@ def intersect_time_ranges(scope_range: str, model_range:
str | None) -> str:
since = _latest(scope_since, model_since)
until = _earliest(scope_until, model_until)
- if since is not None and until is not None and since >= until:
- raise MCPDashboardScopeError(
- f"the requested time_range {model_range!r} does not overlap the "
- f"dashboard time range {scope_range!r}.",
- _ASK_USER,
- )
+ if since is not None and until is not None:
+ since_comparison, until_comparison = get_comparable_bounds(since,
until)
+ if since_comparison >= until_comparison:
+ raise MCPDashboardScopeError(
+ f"the requested time_range {model_range!r} does not overlap
the "
+ f"dashboard time range {scope_range!r}.",
+ _ASK_USER,
+ )
return f"{_iso(since)} : {_iso(until)}"
def _latest(first: datetime | None, second: datetime | None) -> datetime |
None:
- candidates = [value for value in (first, second) if value is not None]
- return max(candidates) if candidates else None
+ if first is None or second is None:
+ return first or second
+ first_comparison, second_comparison = get_comparable_bounds(first, second)
+ return first if first_comparison >= second_comparison else second
Review Comment:
Mixed bounds are picked by wall clock here, but the dataset filter reads the
aware one as an instant: scope
`DATETRUNC(DATETIME('2024-05-31T12:00:00-05:00'), QUARTER) : 2024-07-01` with
model start `2024-04-01T03:00:00` filters from `03:00`, before the scope's own
`05:00`.
##########
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:
On a DATE column with no dataset timezone this moves the calendar date:
`DATETIME('2024-05-01T00:00:00+08:00') : DATETIME('2024-06-01T00:00:00+08:00')`
renders Postgres `d >= TO_DATE('2024-04-30')` and `d < TO_DATE('2024-05-31')`;
master kept 05-01..06-01.
##########
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:
With Hour Offset 8, an aware `+08:00` quarter start is converted to UTC
above and then shifted by the offset again here: head filters from `2024-03-31
08:00:00`, master from `2024-03-31 16:00:00`. The Hour Offset should apply only
to bounds without an explicit offset.
--
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]
