fix(charts): render time comparison without a time grain (#42054)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: sadpandajoe

superset/models/helpers.py

@@ -140,6 +140,8 @@
 )
 from superset.utils.date_parser import (
     get_past_or_future,
+    is_constant_human_timedelta,
+    is_parseable_human_timedelta,
     normalize_time_delta,
     TimeDeltaAmbiguousError,
 )
@@ -172,6 +174,16 @@ class ValidationResultDict(TypedDict):
 R_SUFFIX = "__right_suffix"
 
 
+def _as_wall_clock(series: pd.Series) -> pd.Series:
+    """
+    Return a datetime series as local wall-clock readings, dropping any
+    timezone. Series of other dtypes are returned unchanged.
+    """
+    if isinstance(series.dtype, pd.DatetimeTZDtype):
+        return series.dt.tz_localize(None)
+    return series
+
+
 class CachedTimeOffset(TypedDict):
     """Result type for time offset processing"""
 
@@ -1912,6 +1924,22 @@ def processing_time_offsets(  # pylint: disable=too-many-locals,too-many-stateme
                             outer_from_dttm,
                             outer_to_dttm,
                         )
+                    elif not is_parseable_human_timedelta(offset):
+                        # get_past_or_future silently returns the source time
+                        # for offsets it cannot parse; querying with an
+                        # unshifted window would present the current period's
+                        # data as the comparison series. The parse flag (not
+                        # a zero delta) is the unparseability signal, so
+                        # legitimate zero-shift offsets like "0 days ago"
+                        # pass through.
+                        raise QueryObjectValidationError(
+                            _(
+                                "Unable to interpret the time offset: "
+                                "%(offset)s. Use a relative time such as "
+                                '"1 month ago".',
+                                offset=offset,
+                            )
+                        )
                     query_object_clone.from_dttm = get_past_or_future(
                         offset,
                         outer_from_dttm,
@@ -2118,6 +2146,7 @@ def processing_time_offsets(  # pylint: disable=too-many-locals,too-many-stateme
                 time_grain,
                 join_keys,
                 full_range=getattr(query_object, "time_compare_full_range", False),
+                x_axis_label=get_x_axis_label(query_object.columns),
             )
 
         return CachedTimeOffset(df=df, queries=queries, cache_keys=cache_keys)
@@ -2172,8 +2201,15 @@ def get_offset_custom_or_inherit(
         :returns: The time offset.
         """
         if offset == "inherit":
-            # return the difference in days between the from and the to dttm formatted as a string with the " days ago" suffix  # noqa: E501
-            return f"{(outer_to_dttm - outer_from_dttm).days} days ago"
+            # Shift back by the full length of the range, so the comparison
+            # covers the period immediately preceding it. The duration is
+            # expressed in seconds rather than days because ``timedelta.days``
+            # truncates: a 12-hour range would yield "0 days ago" and compare
+            # 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
+            return f"{int(duration.total_seconds())} seconds ago"
         if self.is_valid_date(offset):
             # return the offset as the difference in days between the outer from dttm and the offset date (which is a YYYY-MM-DD string) formatted as a string with the " days ago" suffix  # noqa: E501
             offset_date = datetime.strptime(offset, "%Y-%m-%d")
@@ -2268,7 +2304,7 @@ def _apply_cleanup_logic(
             )
         else:
             df.drop(
-                list(df.filter(regex=f"{R_SUFFIX}")),
+                list(df.filter(regex=f"{OFFSET_JOIN_COLUMN_SUFFIX}|{R_SUFFIX}")),
                 axis=1,
                 inplace=True,
             )
@@ -2284,6 +2320,7 @@ def _determine_join_keys(
         join_keys: list[str],
         is_date_range_offset: bool,
         join_column_producer: Any,
+        x_axis_label: str | None = None,
     ) -> tuple[pd.DataFrame, list[str]]:
         """Determine appropriate join keys and modify DataFrames if needed."""
         if time_grain and not is_date_range_offset:
@@ -2312,8 +2349,110 @@ def _determine_join_keys(
             return self._process_date_range_offset(offset_df, join_keys)
 
         else:
+            return self._align_offset_without_time_grain(
+                df, offset_df, offset, join_keys, x_axis_label
+            )
+
+    def _align_offset_without_time_grain(
+        self,
+        df: pd.DataFrame,
+        offset_df: pd.DataFrame,
+        offset: str,
+        join_keys: list[str],
+        x_axis_label: str | None = None,
+    ) -> tuple[pd.DataFrame, list[str]]:
+        """
+        Determine join keys for a relative offset when no time grain is set.
+
+        Without a time grain there is no truncated join column, but the two
+        series can still be aligned exactly: shifting the main series'
+        timestamps by the offset delta lands them on the offset series' raw
+        timestamps (normalize_time_delta returns a negative delta for "... ago"
+        offsets). Timestamps without an exact counterpart in the offset series
+        produce nulls, mirroring the grain-based join, and null timestamps in
+        the two series join to each other (both stringify to "NaT"). When
+        there is no temporal join key the original join keys are used as-is.
+
+        The shift is computed on wall-clock time, with any timezone dropped for
+        the duration of the alignment. The offset query's own time range was
+        shifted the same way -- ``get_past_or_future`` reads naive timestamps
+        -- so the rows it returns carry the source wall clock, and matching on
+        it is what aligns the two series. Re-localizing the result would only
+        reintroduce the DST edge cases that wall-clock arithmetic sidesteps:
+        shifting onto a skipped or repeated local hour raises out of pandas.
+        Both sides are normalized identically, so the two readings of a
+        repeated hour still align with each other.
+
+        Month, quarter, and year offsets shift via ``DateOffset``, which clamps
+        to a valid calendar day (e.g. Mar 29, 30, and 31 all shift back one
+        month to Feb 28), so on daily/irregular data several end-of-month rows
+        can align to the same offset timestamp. This mirrors the inherent
+        ambiguity of "the same day N months ago" without a time grain to
+        truncate against.
+        """
+        # Prefer the query's temporal x-axis when it is a join key; otherwise
+        # use the first datetime join key.
+        candidate_keys = sorted(join_keys, key=lambda key: key != x_axis_label)
+        temporal_join_key = next(
+            (
+                key
+                for key in candidate_keys
+                if key in df.columns
+                and key in offset_df.columns
+                and pd.api.types.is_datetime64_any_dtype(df[key])
+            ),
+            None,
+        )
+        if not temporal_join_key:
             return offset_df, join_keys
 
+        source = _as_wall_clock(df[temporal_join_key])
+
+        try:
+            delta: DateOffset | None = DateOffset(**normalize_time_delta(offset))
+        except (ValueError, TimeDeltaAmbiguousError):
+            delta = None
+
+        column_name = OFFSET_JOIN_COLUMN_SUFFIX + offset
+        if delta is not None:
+            # DateOffset addition is vectorized over the datetime column; NaT
+            # rows shift to NaT (they join to the offset series' NaT rows).
+            shifted = source + delta
+        else:
+            # Free-form offsets (e.g. "one year ago") don't match the
+            # normalize_time_delta grammar; shift with the same parser that
+            # shifted the offset query's time range. parsedatetime resolves
+            # second resolution only, so compute each row's delta from a
+            # truncated copy and apply it to the original value, preserving
+            # sub-second precision.
+            if not is_constant_human_timedelta(offset):
+                # Anchors such as "yesterday" resolve every source time within
+                # a day onto one timestamp rather than shifting each by a
+                # fixed amount, so they cannot align two series row by row:
+                # distinct timestamps would collapse onto a single join key.
+                # A time grain gives the join a truncated column to match on
+                # instead of a shifted one.
+                raise QueryObjectValidationError(
+                    _("Time Grain must be specified when using Time Comparison.")
+                )
+
+            def shift(value: pd.Timestamp) -> pd.Timestamp:
+                if pd.isna(value):
+                    return value
+                truncated = value.floor("s").to_pydatetime()
+                return value + (get_past_or_future(offset, truncated) - truncated)
+
+            shifted = source.map(shift)
+
+        # Join on string values so that mismatched key dtypes (e.g. an empty
+        # offset series materializes its join keys as NaN floats) cannot break
+        # the merge.
+        df[column_name] = shifted.map(str)
+        offset_df[column_name] = _as_wall_clock(offset_df[temporal_join_key]).map(str)
+
+        remaining_keys = [key for key in join_keys if key != temporal_join_key]
+        return offset_df, [column_name, *remaining_keys]
+
     def _perform_join(
         self,
         df: pd.DataFrame,
@@ -2357,6 +2496,7 @@ def join_offset_dfs(
         time_grain: str | None,
         join_keys: list[str],
         full_range: bool = False,
+        x_axis_label: str | None = None,
     ) -> pd.DataFrame:
         """
         Join offset DataFrames with the main DataFrame.
@@ -2369,31 +2509,13 @@ def join_offset_dfs(
             time range instead of being truncated to the main series' range. This
             uses an outer join so offset-only rows (e.g. the rest of a prior day when
             the current day is still in progress) are preserved.
+        :param x_axis_label: The query's temporal x-axis label, used to pick the
+            temporal join key when no time grain is set.
         """
         join_column_producer = app.config["TIME_GRAIN_JOIN_COLUMN_PRODUCERS"].get(
             time_grain
         )
-
-        if not time_grain:
-            has_temporal_join_key = any(
-                pd.api.types.is_datetime64_any_dtype(df[key])
-                for key in join_keys
-                if key in df.columns
-            )
-            if has_temporal_join_key:
-                has_relative_offset = any(
-                    not (
-                        self.is_valid_date_range(offset)
-                        and feature_flag_manager.is_feature_enabled(
-                            "DATE_RANGE_TIMESHIFTS_ENABLED"
-                        )
-                    )
-                    for offset in offset_dfs
-                )
-                if has_relative_offset:
-                    raise QueryObjectValidationError(
-                        _("Time Grain must be specified when using Time Comparison.")
-                    )
+        original_columns = list(df.columns)
 
         for offset, offset_df in offset_dfs.items():
             is_date_range_offset = self.is_valid_date_range(
@@ -2410,6 +2532,7 @@ def join_offset_dfs(
                 join_keys,
                 is_date_range_offset,
                 join_column_producer,
+                x_axis_label,
             )
 
             # The full-range option is only meaningful for relative offsets aligned
@@ -2432,6 +2555,15 @@ def join_offset_dfs(
                 df, offset, time_grain, join_keys, is_date_range_offset
             )
 
+            if not time_grain and not is_date_range_offset:
+                # The grain-less join indexes on the synthetic key plus the
+                # non-temporal join keys, which reset_index moves to the front;
+                # restore the original column order.
+                df = df[
+                    [col for col in original_columns if col in df.columns]
+                    + [col for col in df.columns if col not in original_columns]
+                ]
+
         return df
 
     def _coalesce_offset_index(

superset/utils/date_parser.py

@@ -60,12 +60,33 @@
 
 logger = logging.getLogger(__name__)
 
+# Source times used by ``is_constant_human_timedelta`` to tell a delta from an
+# anchor. They share a date -- mid-month and mid-year, away from any month or
+# year boundary a shift could clamp against -- and differ only in the hour, so
+# that the sole thing the comparison can detect is sensitivity to time of day.
+# Neither hour is parsedatetime's 09:00 default, so an anchor cannot coincide
+# with a probe and masquerade as a zero shift.
+_SHIFT_PROBE_TIMES: tuple[datetime, datetime] = (
+    datetime(2024, 6, 15, 3, 0, 0),
+    datetime(2024, 6, 15, 21, 0, 0),
+)
+
 # Mapping of ordinal words to their numeric values for date expressions
 ORDINAL_MAP: dict[str, int] = {
     "first": 1,
     "1st": 1,
 }
 
+# parsedatetime does not understand "N quarters" (it leaves the source time
+# unchanged), so such phrases are rewritten to the equivalent number of months
+# before parsing. The lookbehind and the bounded repetition keep matching
+# linear on user-provided strings (every suffix of an unbounded digit run
+# would be re-scanned) and keep the int() conversion small; longer digit
+# runs fall through to parsedatetime like any other unparseable phrase.
+_QUARTERS_PATTERN: re.Pattern[str] = re.compile(
+    r"(?<![0-9])([0-9]{1,10})\s+quarters?\b", re.IGNORECASE
+)
+
 
 def parse_human_datetime(human_readable: str) -> datetime:
     """Returns ``datetime.datetime`` from human readable strings"""
@@ -95,9 +116,12 @@ def normalize_time_delta(human_readable: str) -> dict[str, int]:
     if not matched:
         raise TimeDeltaAmbiguousError(human_readable)
 
-    key = matched[2] + "s"
+    key = matched[2].lower() + "s"
     value = int(matched[1])
-    value = -value if matched[3] == "ago" else value
+    value = -value if (matched[3] or "").lower() == "ago" else value
+    if key == "quarters":
+        # pd.DateOffset does not accept a `quarters` argument
+        key, value = "months", value * 3
     return {key: value}
 
 
@@ -112,6 +136,13 @@ def dttm_from_timetuple(date_: struct_time) -> datetime:
     )
 
 
+def _rewrite_quarters_as_months(human_readable: str | None) -> str:
+    return _QUARTERS_PATTERN.sub(
+        lambda match: f"{int(match[1]) * 3} months",
+        human_readable or "",
+    )
+
+
 def get_past_or_future(
     human_readable: str | None,
     source_time: datetime | None = None,
@@ -120,7 +151,47 @@ def get_past_or_future(
     source_dttm = dttm_from_timetuple(
         source_time.timetuple() if source_time else datetime.now().timetuple()
     )
-    return dttm_from_timetuple(cal.parse(human_readable or "", source_dttm)[0])
+    human_readable = _rewrite_quarters_as_months(human_readable)
+    return dttm_from_timetuple(cal.parse(human_readable, source_dttm)[0])
+
+
+def is_parseable_human_timedelta(human_readable: str | None) -> bool:
+    """
+    Returns whether parsedatetime understands the phrase.
+
+    parsedatetime echoes the source time back for phrases it cannot parse,
+    so a zero ``parse_human_timedelta`` result cannot distinguish an
+    uninterpretable phrase from one that legitimately parses to no shift
+    (e.g. "0 days ago"). The parse flag makes that distinction: it is 0
+    only when nothing in the phrase was understood.
+    """
+    cal = parsedatetime.Calendar()
+    return cal.parse(_rewrite_quarters_as_months(human_readable))[1] != 0
+
+
+def is_constant_human_timedelta(human_readable: str | None) -> bool:
+    """
+    Returns whether the phrase shifts every source time by the same amount.
+
+    ``is_parseable_human_timedelta`` accepts anchors such as "yesterday" and
+    "last month" alongside true deltas such as "1 year ago", but the two
+    behave differently when applied per row: an anchor resolves to a single
+    timestamp (parsedatetime defaults to 09:00) no matter where the source
+    time sits within the day, so it shifts each row by a different amount.
+
+    Probing two source times within the same day separates the two. A delta
+    shifts both probes equally; an anchor maps both onto one timestamp, which
+    -- the probes being distinct -- necessarily yields differing shifts. Both
+    probes share a date, so calendar irregularities such as leap years and
+    month lengths apply to them identically and cannot skew the comparison.
+    """
+    if not is_parseable_human_timedelta(human_readable):
+        return False
+    deltas = {
+        get_past_or_future(human_readable, probe) - probe
+        for probe in _SHIFT_PROBE_TIMES
+    }
+    return len(deltas) == 1
 
 
 def parse_human_timedelta(