aminghadersohi commented on code in PR #45671:
URL: https://github.com/apache/superset/pull/45671#discussion_r4436176287
##########
superset/models/helpers.py:
##########
@@ -3129,11 +3153,26 @@ def processing_time_offsets( # pylint:
disable=too-many-locals,too-many-stateme
)
query_object_clone.from_dttm = get_past_or_future(
offset,
- outer_from_dttm,
+ shift_from_dttm,
)
query_object_clone.to_dttm = get_past_or_future(
- offset, outer_to_dttm
+ offset, shift_to_dttm
)
+ if (
+ original_offset == "inherit"
+ and time_grain
+ in (
+ TimeGrain.DAY,
+ TimeGrain.HOUR,
Review Comment:
Alignment covers only Day/Hour, so other grains still join on the fractional
SQL span: for `DATETRUNC(DATETIME('2024-05-31T12:00:00+08:00'), QUARTER) :
2024-07-01`, PT6H fills 0/366 comparison buckets (base 364/364) and Quarter
grain compares Q2 with Q4 2023's 16-hour remainder.
##########
superset/models/helpers.py:
##########
@@ -3803,6 +3927,11 @@ def join_offset_dfs(
x_axis_datetime_format,
resolved_week_offset,
)
+ if inherited_hour:
+ for key in actual_join_keys:
+ if key not in df:
+ df[key] = working_df[key]
+ offset_df.iloc[:, 0] = displayed_offset_values
Review Comment:
With `time_compare_full_range`, comparison-only rows are rebuilt by adding
the UTC alignment shift to display times, so across a DST change they land an
hour off: Berlin, PT1H, `2024-10-26T00:00:00+08:00 : 2024-10-29` with missing
main hours gives 22 duplicate timestamps.
##########
superset/models/helpers.py:
##########
@@ -3387,6 +3440,55 @@ def is_valid_date_range(self, date_range: str) -> bool:
# YYYY-MM-DD:YYYY-MM-DD
return False
+ def _inherit_alignment_offset(
+ self, start: datetime, previous_start: datetime, grain: str
+ ) -> str:
+ """Align grained display buckets separately from SQL window
durations."""
+ if grain == TimeGrain.HOUR:
+ start_bucket = start.replace(minute=0, second=0, microsecond=0)
+ previous_bucket = previous_start.replace(minute=0, second=0,
microsecond=0)
+ return (
+ f"{int((start_bucket - previous_bucket).total_seconds())}
seconds ago"
+ )
+ anchors = pd.DataFrame({DTTM_ALIAS: [start, previous_start]})
+ normalize_dttm_col(
+ anchors,
+ (
+ DateColumn(
+ col_label=DTTM_ALIAS,
+ offset=self.offset,
+ timezone=self.get_dataset_timezone(),
+ ),
+ ),
+ )
+ values = anchors[DTTM_ALIAS]
+ if grain == TimeGrain.DAY:
+ values = values.dt.normalize()
+ duration = values.iloc[0] - values.iloc[1]
+ return f"{int(duration.total_seconds())} seconds ago"
+
+ def _inherit_time_bounds(
+ self,
+ start: datetime,
+ end: datetime,
+ time_col: Optional["TableColumn"] = None,
+ ) -> tuple[datetime, datetime]:
+ """Resolve explicit inherited periods in the SQL filter's coordinate
frame."""
+ if start.utcoffset() is None and end.utcoffset() is None:
+ return start, end
+ if time_col is None:
+ time_col = next(
+ (col for col in self.columns if col.is_dttm),
+ None,
+ )
Review Comment:
When the time column isn't resolved, this picks the first `is_dttm` column
instead of `main_dttm_col`: with columns `[d DATE, ts TIMESTAMP]`, the
factory's INHERITED window for `2024-04-01T12:00:00+08:00 :
2024-04-01T16:30:00+08:00` becomes 07:30–12:00 UTC, overlapping the 04:00–08:30
main window.
```suggestion
main_col = getattr(self, "main_dttm_col", None)
time_col = next(
(col for col in self.columns if col.column_name == main_col),
next((col for col in self.columns if col.is_dttm), None),
)
```
##########
superset/models/helpers.py:
##########
@@ -3840,6 +3969,26 @@ def join_offset_dfs(
return df
+ def _inherit_hour_join_frame(self, frame: pd.DataFrame) -> pd.DataFrame:
+ """Build elapsed-hour join keys without changing the displayed
timestamps."""
+ name = self.get_dataset_timezone()
+ if (
+ not name
+ or frame.empty
+ or not pd.api.types.is_datetime64_any_dtype(frame.iloc[:, 0])
+ ):
+ return frame.copy()
+ try:
+ zone = ZoneInfo(name)
+ except ZoneInfoNotFoundError:
+ return frame.copy()
+ result = frame.copy()
+ values = result.iloc[:, 0]
+ if values.dt.tz is None:
+ values = values.dt.tz_localize(zone, ambiguous="infer")
Review Comment:
On a DST dataset timezone, `ambiguous="infer"` raises `ValueError` when only
one of the two repeated fall-back hours has rows: Europe/Berlin, PT1H,
`2024-10-26T00:00:00+08:00 : 2024-11-01` with no rows 00:00–01:00 UTC on 10-27
fails the chart; the previous commit returned data.
##########
superset/models/helpers.py:
##########
@@ -7402,6 +7580,9 @@ def get_sqla_query( # pylint:
disable=too-many-arguments,too-many-locals,too-ma
time_range=eq,
time_shift=time_shift,
extras=extras,
+ time_range_normalizer=get_time_range_normalizer(
+ self, col_obj.column_name
+ ),
Review Comment:
Nit: No test fails if this normalizer is removed: for a Europe/Berlin
dataset and `2024-04-01 : 2024-07-01T00:00:00+08:00`, the INHERITED filter
window then ends at 04-01 00:00 UTC, 2 hours past the main window's start.
--
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]
