Twiink commented on code in PR #45671:
URL: https://github.com/apache/superset/pull/45671#discussion_r4435437501
##########
superset/models/helpers.py:
##########
@@ -3409,12 +3466,18 @@ def get_offset_custom_or_inherit(
# the range against itself, and a 36-hour range would shift by a
# single day and overlap it. Whole-day ranges resolve to the same
# instant either way.
- duration = outer_to_dttm - outer_from_dttm
+ from_comparison, to_comparison = get_comparable_bounds(
+ *self._inherit_time_bounds(outer_from_dttm, outer_to_dttm)
+ )
+ duration = to_comparison - from_comparison
Review Comment:
Fixed in 3c33585a. The comparison query keeps the exact, adjacent SQL
window, while Day/Hour joins use a separate bucket-alignment offset. Day joins
compare calendar bucket keys; inherited Hour joins use UTC working keys when
the dataset display timezone would otherwise introduce DST drift. Displayed
timestamps are retained.
Added tests that execute the datasource-generated SQL against SQLite, then
run the real normalize_df and join_offset_dfs implementations. Your Day example
now fills 92/92 comparison buckets while comparison_end still equals
main_start. The tests also cover +05:30, Hour grain, SQL Lab Query datasources,
Europe/Berlin spring/fall DST, full-range comparisons, and cache hits. The
earlier boundary-only tests are retained alongside these complete pipeline
regressions.
Local validation: 919 passed, 1 skipped; the same two previously verified
Windows baseline failures were deselected. Ruff/formatting passed, Linux-target
mypy passed for all 14 changed Python files, and direct project-configured
Pylint scored 10/10. New remote CI has not yet been confirmed.
##########
superset/models/helpers.py:
##########
@@ -3387,6 +3422,28 @@ def is_valid_date_range(self, date_range: str) -> bool:
# YYYY-MM-DD:YYYY-MM-DD
return False
+ 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.column_name ==
self.main_dttm_col),
Review Comment:
Fixed in 3c33585a. get_offset_custom_or_inherit no longer re-enters SQL
normalization after the caller has normalized the range. The fallback in
_inherit_time_bounds reads TableColumn.is_dttm directly instead of accessing
main_dttm_col. Added both a direct regression with a real SQL Lab Query
datasource and full SQL-generation/normalization/comparison-join regressions
using that datasource.
##########
superset/utils/date_parser.py:
##########
@@ -739,32 +753,34 @@ def get_since_until( # pylint:
disable=too-many-arguments,too-many-locals,too-m
elif instant_time_comparison_range == InstantTimeComparison.WEEK:
time_unit = "WEEK"
elif instant_time_comparison_range ==
InstantTimeComparison.INHERITED:
- delta_in_days = (_until - _since).days if _since and _until
else None
+ if _since and _until:
+ since_comparison, until_comparison = get_comparable_bounds(
+ _since, _until
+ )
+ delta_in_days = (until_comparison - since_comparison).days
Review Comment:
Fixed in 3c33585a. For explicit-offset ranges, INHERITED uses the exact
normalized SQL duration and returns the preceding window with explicit UTC
endpoints, instead of truncating a wall-clock duration to days. The query
factory and temporal-filter SQL path pass the destination column's
normalization into range parsing, so dataset timezone and DATE semantics are
available at the point the shift is resolved. Standalone parsing uses UTC for
naive timestamp endpoints. Fully naive ranges retain their existing calendar
behavior.
Added real query-factory regressions for both mixed-bound directions, no
dataset timezone and Europe/Berlin, plus an aware sub-day range. They assert
equal SQL durations and comparison_end == main_start. The reported 8-hour
overlap is eliminated.
--
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]
