diff --git a/superset/models/helpers.py b/superset/models/helpers.py index d6cd79d537c..9b447661833 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -140,6 +140,8 @@ from superset.utils.core import ( ) 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 @@ OFFSET_JOIN_COLUMN_SUFFIX = "__offset_join_column_" 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 @@ class ExploreMixin: # pylint: disable=too-many-public-methods 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 @@ class ExploreMixin: # pylint: disable=too-many-public-methods 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 @@ class ExploreMixin: # pylint: disable=too-many-public-methods :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 @@ class ExploreMixin: # pylint: disable=too-many-public-methods ) 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 @@ class ExploreMixin: # pylint: disable=too-many-public-methods 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 @@ class ExploreMixin: # pylint: disable=too-many-public-methods 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 @@ class ExploreMixin: # pylint: disable=too-many-public-methods 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 @@ class ExploreMixin: # pylint: disable=too-many-public-methods 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 @@ class ExploreMixin: # pylint: disable=too-many-public-methods 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 @@ class ExploreMixin: # pylint: disable=too-many-public-methods 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( diff --git a/superset/utils/date_parser.py b/superset/utils/date_parser.py index 7405cba27a5..aa51aa630db 100644 --- a/superset/utils/date_parser.py +++ b/superset/utils/date_parser.py @@ -60,12 +60,33 @@ logging.getLogger("parsedatetime").setLevel(logging.WARNING) 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"(? 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( diff --git a/tests/integration_tests/query_context_tests.py b/tests/integration_tests/query_context_tests.py index e28c17ed010..3488cbacf64 100644 --- a/tests/integration_tests/query_context_tests.py +++ b/tests/integration_tests/query_context_tests.py @@ -1628,7 +1628,7 @@ def test_date_range_timeshift_multiple_periods(app_context, physical_dataset): @with_feature_flags(DATE_RANGE_TIMESHIFTS_ENABLED=True) def test_date_range_timeshift_invalid_format(app_context, physical_dataset): - """Test that invalid date range format raises appropriate error.""" + """Test that an uninterpretable offset fails with a validation error.""" qc = QueryContextFactory().create( datasource={ "type": physical_dataset.type, @@ -1665,11 +1665,12 @@ def test_date_range_timeshift_invalid_format(app_context, physical_dataset): force=True, ) - # Should raise an error for invalid date range format - from superset.commands.chart.exceptions import TimeDeltaAmbiguousError + # An uninterpretable offset fails the query cleanly instead of leaking + # an unhandled TimeDeltaAmbiguousError out of get_df_payload + query_payload = qc.get_df_payload(qc.queries[0]) - with pytest.raises(TimeDeltaAmbiguousError): - qc.get_df_payload(qc.queries[0]) + assert query_payload["status"] == QueryStatus.FAILED + assert "Unable to interpret the time offset" in query_payload["error"] @with_feature_flags(DATE_RANGE_TIMESHIFTS_ENABLED=True) diff --git a/tests/unit_tests/common/test_query_context_processor.py b/tests/unit_tests/common/test_query_context_processor.py index 0272658c2d6..a60228cd24f 100644 --- a/tests/unit_tests/common/test_query_context_processor.py +++ b/tests/unit_tests/common/test_query_context_processor.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. +from datetime import datetime, timedelta from typing import Any from unittest.mock import MagicMock, patch @@ -26,6 +27,7 @@ from superset.common.chart_data import ChartDataResultFormat, ChartDataResultTyp from superset.common.db_query_status import QueryStatus from superset.common.query_context_processor import QueryContextProcessor from superset.utils.core import GenericDataType +from superset.utils.date_parser import get_past_or_future @pytest.fixture @@ -389,8 +391,45 @@ def test_get_offset_custom_or_inherit_with_inherit(processor): "inherit", from_dttm, to_dttm ) - # Should return the difference in days - assert result == "9 days ago" + # Should shift back by the length of the range: 9 days, in seconds + assert result == "777600 seconds ago" + + +@pytest.mark.parametrize( + "hours", + [ + # Sub-day ranges truncated to "0 days ago" and compared the range + # against itself. + 1, + 12, + # Ranges that are not a whole number of days truncated downwards and + # overlapped the range they were compared against. + 36, + # Whole-day ranges were already correct and must stay so. + 24, + 48, + ], +) +def test_get_offset_custom_or_inherit_shifts_by_full_range( + processor, hours: int +) -> None: + """ + 'inherit' compares a range against the period immediately preceding it, + for any range length. The offset must not be truncated to whole days: it + has to reproduce the range's exact duration, so that the comparison window + neither overlaps the range nor leaves a gap before it. + """ + from_dttm = datetime(2024, 3, 10, 6, 0) + to_dttm = from_dttm + timedelta(hours=hours) + + offset = processor._qc_datasource.get_offset_custom_or_inherit( + "inherit", from_dttm, to_dttm + ) + + shifted_from = get_past_or_future(offset, from_dttm) + shifted_to = get_past_or_future(offset, to_dttm) + assert shifted_to == from_dttm + assert shifted_to - shifted_from == to_dttm - from_dttm def test_get_offset_custom_or_inherit_with_date(processor): @@ -1024,6 +1063,261 @@ def test_processing_time_offsets_updates_temporal_filter_with_adhoc_x_axis(proce assert "2025-06-01" in val, f"Expected shifted-to-dttm in val, got: {val!r}" +def test_processing_time_offsets_quarter_offset_shifts_query_window( + processor: QueryContextProcessor, +) -> None: + """A quarter offset must shift the offset query's window, not just the + join keys. parsedatetime does not understand "1 quarter ago", so without + rewriting quarters to months the offset subquery silently runs against + the current period and the comparison series joins to nulls. The fake + query below derives its rows from the requested window, so the join only + yields the expected values when the window was actually shifted. + """ + from superset.common.query_object import QueryObject + from superset.models.helpers import ExploreMixin + + # The fixture's datasource is a MagicMock, not a real Explorable + datasource: Any = processor._qc_datasource + + for method in ( + "processing_time_offsets", + "_align_offset_without_time_grain", + "_coalesce_offset_index", + ): + setattr( + datasource, + method, + getattr(ExploreMixin, method).__get__(datasource), + ) + + df = pd.DataFrame( + { + "__timestamp": pd.to_datetime(["2024-04-01", "2024-05-01", "2024-06-01"]), + "sum__num": [100, 200, 300], + } + ) + + query_object = QueryObject( + datasource=MagicMock(), + granularity="ds", + columns=[], + metrics=["sum__num"], + is_timeseries=True, + time_offsets=["1 quarter ago"], + filters=[ + { + "col": "ds", + "op": "TEMPORAL_RANGE", + "val": "2024-04-01 : 2024-07-01", + } + ], + ) + + captured: list[dict[str, Any]] = [] + + def fake_query(dct: dict[str, Any]) -> MagicMock: + captured.append(dct) + result = MagicMock() + result.df = pd.DataFrame( + { + "__timestamp": pd.date_range( + start=dct["from_dttm"], periods=3, freq="MS" + ), + "sum__num": [1.0, 2.0, 3.0], + } + ) + result.query = "SELECT 1" + return result + + datasource.query = fake_query + datasource.normalize_df = MagicMock( + side_effect=lambda offset_df, _query_object: offset_df + ) + + with ( + patch( + "superset.models.helpers.get_since_until_from_query_object", + return_value=(pd.Timestamp("2024-04-01"), pd.Timestamp("2024-07-01")), + ), + patch( + "superset.common.utils.query_cache_manager.QueryCacheManager" + ) as mock_cache_manager, + patch.object( + datasource, + "get_time_grain", + return_value=None, + ), + ): + mock_cache = MagicMock() + mock_cache.is_loaded = False + mock_cache_manager.get.return_value = mock_cache + + result = datasource.processing_time_offsets(df, query_object, None, None, False) + + assert len(captured) == 1 + assert captured[0]["from_dttm"] == pd.Timestamp("2024-01-01") + assert captured[0]["to_dttm"] == pd.Timestamp("2024-04-01") + assert result["df"]["sum__num__1 quarter ago"].tolist() == [1.0, 2.0, 3.0] + + +def test_processing_time_offsets_accepts_zero_shift_offset( + processor: QueryContextProcessor, +) -> None: + """An offset that legitimately parses to no shift (e.g. "0 days ago") + must render a self-comparison instead of being rejected as + uninterpretable: a zero delta is not a parse failure. + """ + from superset.common.query_object import QueryObject + from superset.models.helpers import ExploreMixin + + # The fixture's datasource is a MagicMock, not a real Explorable + datasource: Any = processor._qc_datasource + + for method in ( + "processing_time_offsets", + "_align_offset_without_time_grain", + "_coalesce_offset_index", + ): + setattr( + datasource, + method, + getattr(ExploreMixin, method).__get__(datasource), + ) + + df = pd.DataFrame( + { + "__timestamp": pd.to_datetime(["2024-04-01", "2024-05-01", "2024-06-01"]), + "sum__num": [100, 200, 300], + } + ) + + query_object = QueryObject( + datasource=MagicMock(), + granularity="ds", + columns=[], + metrics=["sum__num"], + is_timeseries=True, + time_offsets=["0 days ago"], + filters=[ + { + "col": "ds", + "op": "TEMPORAL_RANGE", + "val": "2024-04-01 : 2024-07-01", + } + ], + ) + + captured: list[dict[str, Any]] = [] + + def fake_query(dct: dict[str, Any]) -> MagicMock: + captured.append(dct) + result = MagicMock() + result.df = pd.DataFrame( + { + "__timestamp": pd.date_range( + start=dct["from_dttm"], periods=3, freq="MS" + ), + "sum__num": [1.0, 2.0, 3.0], + } + ) + result.query = "SELECT 1" + return result + + datasource.query = fake_query + datasource.normalize_df = MagicMock( + side_effect=lambda offset_df, _query_object: offset_df + ) + + with ( + patch( + "superset.models.helpers.get_since_until_from_query_object", + return_value=(pd.Timestamp("2024-04-01"), pd.Timestamp("2024-07-01")), + ), + patch( + "superset.common.utils.query_cache_manager.QueryCacheManager" + ) as mock_cache_manager, + patch.object( + datasource, + "get_time_grain", + return_value=None, + ), + ): + mock_cache = MagicMock() + mock_cache.is_loaded = False + mock_cache_manager.get.return_value = mock_cache + + result = datasource.processing_time_offsets(df, query_object, None, None, False) + + # The offset query runs against the unshifted window and its rows join + # back onto the main series one-to-one + assert len(captured) == 1 + assert captured[0]["from_dttm"] == pd.Timestamp("2024-04-01") + assert captured[0]["to_dttm"] == pd.Timestamp("2024-07-01") + assert result["df"]["sum__num__0 days ago"].tolist() == [1.0, 2.0, 3.0] + + +def test_processing_time_offsets_rejects_unparseable_offset( + processor: QueryContextProcessor, +) -> None: + """An offset no parser understands must fail with a validation error + instead of querying an unshifted window and presenting the current + period's rows as the comparison series. + """ + from superset.common.query_object import QueryObject + from superset.exceptions import QueryObjectValidationError + from superset.models.helpers import ExploreMixin + + # The fixture's datasource is a MagicMock, not a real Explorable + datasource: Any = processor._qc_datasource + + datasource.processing_time_offsets = ExploreMixin.processing_time_offsets.__get__( + datasource + ) + + df = pd.DataFrame( + { + "__timestamp": pd.to_datetime(["2024-04-01"]), + "sum__num": [100], + } + ) + + query_object = QueryObject( + datasource=MagicMock(), + granularity="ds", + columns=[], + metrics=["sum__num"], + is_timeseries=True, + time_offsets=["not a real offset"], + filters=[ + { + "col": "ds", + "op": "TEMPORAL_RANGE", + "val": "2024-04-01 : 2024-07-01", + } + ], + ) + + datasource.query = MagicMock() + + with ( + patch( + "superset.models.helpers.get_since_until_from_query_object", + return_value=(pd.Timestamp("2024-04-01"), pd.Timestamp("2024-07-01")), + ), + patch.object( + datasource, + "get_time_grain", + return_value=None, + ), + ): + with pytest.raises( + QueryObjectValidationError, match="Unable to interpret the time offset" + ): + datasource.processing_time_offsets(df, query_object, None, None, False) + + datasource.query.assert_not_called() + + def test_ensure_totals_available_updates_cache_values(): """ Test that ensure_totals_available() updates the query objects AND diff --git a/tests/unit_tests/common/test_time_shifts.py b/tests/unit_tests/common/test_time_shifts.py index a34076393f2..908ce4233b2 100644 --- a/tests/unit_tests/common/test_time_shifts.py +++ b/tests/unit_tests/common/test_time_shifts.py @@ -14,10 +14,9 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -import pytest from pandas import DataFrame, Series, Timestamp from pandas.testing import assert_frame_equal -from pytest import fixture, mark # noqa: PT013 +from pytest import fixture, mark, raises # noqa: PT013 from superset.common.chart_data import ChartDataResultFormat, ChartDataResultType from superset.common.query_context import QueryContext @@ -51,6 +50,9 @@ _datasource.is_valid_date_range = ExploreMixin.is_valid_date_range.__get__(_data _datasource._determine_join_keys = ExploreMixin._determine_join_keys.__get__( _datasource ) +_datasource._align_offset_without_time_grain = ( + ExploreMixin._align_offset_without_time_grain.__get__(_datasource) +) _datasource._perform_join = ExploreMixin._perform_join.__get__(_datasource) _datasource._apply_cleanup_logic = ExploreMixin._apply_cleanup_logic.__get__( _datasource @@ -334,20 +336,368 @@ def test_join_offset_dfs_totals_query_no_dimensions(): assert_frame_equal(expected, result) -def test_join_offset_dfs_raises_without_time_grain(): - """Time comparison with relative offsets requires a time grain.""" - df = DataFrame({"ds": [Timestamp("2021-01-01")], "D": [1]}) - offset_df = DataFrame({"ds": [Timestamp("2021-02-01")], "B": [5]}) +def test_join_offset_dfs_no_time_grain_aligns_relative_offset() -> None: + """ + Without a time grain, a relative offset joins on the exact shifted + timestamps instead of raising, so saved charts without a grain render + with a correctly aligned comparison series. + """ + df = DataFrame( + { + "ds": [Timestamp("2021-01-01"), Timestamp("2021-02-01")], + "D": [1, 2], + } + ) + offset_df = DataFrame( + { + "ds": [Timestamp("2020-01-01"), Timestamp("2020-02-01")], + "B": [5, 6], + } + ) offset_dfs = {"1 year ago": offset_df} - with pytest.raises( - QueryObjectValidationError, match="Time Grain must be specified" - ): + expected = DataFrame( + { + "ds": [Timestamp("2021-01-01"), Timestamp("2021-02-01")], + "D": [1, 2], + "B": [5, 6], + } + ) + + result = query_context_processor.join_offset_dfs( + df, offset_dfs, time_grain=None, join_keys=["ds"] + ) + + assert_frame_equal(expected, result) + + +def test_join_offset_dfs_no_time_grain_unmatched_timestamps_yield_nulls() -> None: + """ + Without a time grain, offset timestamps that have no exact shifted + counterpart in the main series produce nulls instead of raising. + """ + df = DataFrame({"ds": [Timestamp("2021-01-01")], "D": [1]}) + offset_df = DataFrame({"ds": [Timestamp("2020-06-15")], "B": [5]}) + offset_dfs = {"1 year ago": offset_df} + + result = query_context_processor.join_offset_dfs( + df, offset_dfs, time_grain=None, join_keys=["ds"] + ) + + assert "B" in result.columns + assert result["B"].isna().all() + + +def test_join_offset_dfs_no_time_grain_multiple_offsets() -> None: + """ + Multiple relative offsets without a time grain each align on their own + shifted timestamps, and no synthetic join columns leak into the result. + """ + df = DataFrame({"ds": [Timestamp("2021-01-29")], "D": [1]}) + offset_df1 = DataFrame({"ds": [Timestamp("2021-01-01")], "B": [5]}) + offset_df2 = DataFrame({"ds": [Timestamp("2020-01-29")], "C": [7]}) + offset_dfs = {"28 days ago": offset_df1, "1 year ago": offset_df2} + + expected = DataFrame( + { + "ds": [Timestamp("2021-01-29")], + "D": [1], + "B": [5], + "C": [7], + } + ) + + result = query_context_processor.join_offset_dfs( + df, offset_dfs, time_grain=None, join_keys=["ds"] + ) + + assert_frame_equal(expected, result) + + +def test_join_offset_dfs_no_time_grain_empty_offset_df() -> None: + """ + An empty offset series materializes its join keys as NaN floats; the + grain-less join must not crash on the dtype mismatch. + """ + df = DataFrame({"ds": [Timestamp("2021-01-01")], "D": [1]}) + offset_df = DataFrame({"ds": [float("nan")], "B": [float("nan")]}) + offset_dfs = {"1 year ago": offset_df} + + result = query_context_processor.join_offset_dfs( + df, offset_dfs, time_grain=None, join_keys=["ds"] + ) + + assert "B" in result.columns + assert result["B"].isna().all() + + +def test_join_offset_dfs_no_time_grain_quarter_offset() -> None: + """ + Quarter offsets align without a time grain (normalize_time_delta converts + quarters to months, since pd.DateOffset has no quarters argument). + """ + df = DataFrame({"ds": [Timestamp("2021-04-01")], "D": [1]}) + offset_df = DataFrame({"ds": [Timestamp("2021-01-01")], "B": [5]}) + offset_dfs = {"1 quarter ago": offset_df} + + expected = DataFrame({"ds": [Timestamp("2021-04-01")], "D": [1], "B": [5]}) + + result = query_context_processor.join_offset_dfs( + df, offset_dfs, time_grain=None, join_keys=["ds"] + ) + + assert_frame_equal(expected, result) + + +def test_join_offset_dfs_no_time_grain_free_form_offset() -> None: + """ + Offsets outside the normalize_time_delta grammar (e.g. "one year ago") + are aligned with the same parser that shifted the offset query's range. + """ + df = DataFrame({"ds": [Timestamp("2021-01-01")], "D": [1]}) + offset_df = DataFrame({"ds": [Timestamp("2020-01-01")], "B": [5]}) + offset_dfs = {"one year ago": offset_df} + + expected = DataFrame({"ds": [Timestamp("2021-01-01")], "D": [1], "B": [5]}) + + result = query_context_processor.join_offset_dfs( + df, offset_dfs, time_grain=None, join_keys=["ds"] + ) + + assert_frame_equal(expected, result) + + +def test_join_offset_dfs_no_time_grain_free_form_offset_tz_microseconds() -> None: + """ + Free-form offsets align timezone-aware, sub-second timestamps: each row's + shift is computed from a naive second-truncated copy and applied to the + original value, so the join keys keep their timezone and precision. + """ + df = DataFrame( + {"ds": [Timestamp("2021-03-04 05:06:07.890123", tz="UTC")], "D": [1]} + ) + offset_df = DataFrame( + {"ds": [Timestamp("2020-03-04 05:06:07.890123", tz="UTC")], "B": [5]} + ) + offset_dfs = {"one year ago": offset_df} + + result = query_context_processor.join_offset_dfs( + df, offset_dfs, time_grain=None, join_keys=["ds"] + ) + + assert result["B"].tolist() == [5] + + +def test_join_offset_dfs_no_time_grain_uninterpretable_offset() -> None: + """ + An offset that no parser can interpret cannot be aligned without a time + grain, and fails with the error that points at the missing grain rather + than silently producing an unshifted or empty comparison. + """ + df = DataFrame({"ds": [Timestamp("2021-01-01")], "D": [1]}) + offset_df = DataFrame({"ds": [Timestamp("2020-01-01")], "B": [5]}) + offset_dfs = {"not a real offset": offset_df} + + with raises(QueryObjectValidationError, match="Time Grain must be"): query_context_processor.join_offset_dfs( df, offset_dfs, time_grain=None, join_keys=["ds"] ) +def test_join_offset_dfs_no_time_grain_uninterpretable_offset_subsecond() -> None: + """ + Sub-second timestamps must not mask an uninterpretable offset: + unparseability is detected from the parser's parse flag, not by + comparing shifted values, so value precision cannot fake a + successful parse. + """ + df = DataFrame({"ds": [Timestamp("2021-01-01 00:00:00.123456")], "D": [1]}) + offset_df = DataFrame({"ds": [Timestamp("2020-01-01 00:00:00.123456")], "B": [5]}) + offset_dfs = {"not a real offset": offset_df} + + with raises(QueryObjectValidationError, match="Time Grain must be"): + query_context_processor.join_offset_dfs( + df, offset_dfs, time_grain=None, join_keys=["ds"] + ) + + +@mark.parametrize("offset", ["yesterday", "last month"]) +def test_join_offset_dfs_no_time_grain_anchor_offset(offset: str) -> None: + """ + Phrases that parsedatetime resolves to a fixed point rather than a shift + are rejected. Applied per row they would move each timestamp by a + different amount -- parsedatetime anchors both of these rows onto the same + 09:00 timestamp -- collapsing distinct buckets onto one join key. + """ + df = DataFrame( + { + "ds": [Timestamp("2021-06-15 03:00"), Timestamp("2021-06-15 21:00")], + "D": [1, 2], + } + ) + offset_df = DataFrame({"ds": [Timestamp("2021-06-14 03:00")], "B": [5]}) + + with raises(QueryObjectValidationError, match="Time Grain must be"): + query_context_processor.join_offset_dfs( + df, {offset: offset_df}, time_grain=None, join_keys=["ds"] + ) + + +def test_join_offset_dfs_no_time_grain_free_form_delta_offset() -> None: + """ + Free-form phrasing that does denote a fixed shift still aligns: it misses + the normalize_time_delta grammar but shifts every row equally, which is + all the join needs. + """ + df = DataFrame({"ds": [Timestamp("2021-06-15 03:00")], "D": [1]}) + offset_df = DataFrame({"ds": [Timestamp("2020-06-15 03:00")], "B": [5]}) + + result = query_context_processor.join_offset_dfs( + df, {"one year ago": offset_df}, time_grain=None, join_keys=["ds"] + ) + + assert result["B"].tolist() == [5] + + +def test_join_offset_dfs_no_time_grain_dst_nonexistent_hour() -> None: + """ + A shift landing on a local hour that DST skips must not raise: 02:30 never + occurs on 2021-03-14 in US/Eastern, so no row can carry that reading and + the comparison is simply empty. Shifting the tz-aware timestamp directly + raised NonExistentTimeError out of pandas instead. + """ + df = DataFrame({"ds": [Timestamp("2021-04-14 02:30", tz="US/Eastern")], "D": [1]}) + offset_df = DataFrame( + {"ds": [Timestamp("2021-03-15 02:30", tz="US/Eastern")], "B": [5]} + ) + + result = query_context_processor.join_offset_dfs( + df, {"1 month ago": offset_df}, time_grain=None, join_keys=["ds"] + ) + + assert result["B"].isna().all() + + +def test_join_offset_dfs_no_time_grain_dst_ambiguous_hour() -> None: + """ + A shift landing on a local hour that DST repeats aligns on the wall clock + the offset query returned. 01:30 occurs twice on 2021-11-07 in US/Eastern; + shifting the tz-aware timestamp directly raised AmbiguousTimeError out of + pandas rather than picking either reading. + """ + df = DataFrame({"ds": [Timestamp("2021-12-07 01:30", tz="US/Eastern")], "D": [1]}) + offset_df = DataFrame( + { + "ds": [ + Timestamp("2021-11-07 01:30").tz_localize("US/Eastern", ambiguous=True) + ], + "B": [5], + } + ) + + result = query_context_processor.join_offset_dfs( + df, {"1 month ago": offset_df}, time_grain=None, join_keys=["ds"] + ) + + assert result["B"].tolist() == [5] + + +@mark.parametrize( + "offset", + [ + # shifts via DateOffset + "2 weeks ago", + # misses the normalize_time_delta grammar, shifts via the parser + "two weeks ago", + ], +) +def test_join_offset_dfs_no_time_grain_tz_aware_preserves_wall_clock( + offset: str, +) -> None: + """ + A tz-aware shift crossing a DST boundary matches the same wall clock on + the other side, rather than drifting by the hour the UTC offset changed. + The offset query's own time range is shifted with naive arithmetic, so its + rows carry the source wall clock and only a wall-clock join finds them. + Both shift paths have to agree on that. + """ + df = DataFrame({"ds": [Timestamp("2021-03-20 12:00", tz="US/Eastern")], "D": [1]}) + # Two weeks before 2021-03-20 EDT is 2021-03-06 EST: same 12:00 wall + # clock, one hour further from UTC. + offset_df = DataFrame( + {"ds": [Timestamp("2021-03-06 12:00", tz="US/Eastern")], "B": [5]} + ) + + result = query_context_processor.join_offset_dfs( + df, {offset: offset_df}, time_grain=None, join_keys=["ds"] + ) + + assert result["B"].tolist() == [5] + + +def test_join_offset_dfs_no_time_grain_prefers_x_axis_label() -> None: + """ + With multiple datetime join keys, the query's x-axis is the one shifted + for alignment, not whichever datetime column happens to come first. + """ + df = DataFrame( + { + "birth_date": [Timestamp("1990-05-05")], + "ds": [Timestamp("2021-02-01")], + "D": [1], + } + ) + offset_df = DataFrame( + { + "birth_date": [Timestamp("1990-05-05")], + "ds": [Timestamp("2021-01-01")], + "B": [5], + } + ) + offset_dfs = {"1 month ago": offset_df} + + result = query_context_processor.join_offset_dfs( + df, + offset_dfs, + time_grain=None, + join_keys=["birth_date", "ds"], + x_axis_label="ds", + ) + + assert result["B"].tolist() == [5] + + +def test_join_offset_dfs_no_time_grain_preserves_column_order() -> None: + """ + A join key following the temporal key must not change the result's + column order (chart payloads and CSV exports derive order from columns). + """ + df = DataFrame( + { + "country": ["US"], + "ds": [Timestamp("2021-02-01")], + "category": ["a"], + "D": [1], + } + ) + offset_df = DataFrame( + { + "country": ["US"], + "ds": [Timestamp("2021-01-01")], + "category": ["a"], + "B": [5], + } + ) + offset_dfs = {"1 month ago": offset_df} + + result = query_context_processor.join_offset_dfs( + df, offset_dfs, time_grain=None, join_keys=["country", "ds", "category"] + ) + + assert result.columns.tolist() == ["country", "ds", "category", "D", "B"] + assert result["B"].tolist() == [5] + + def test_join_offset_dfs_allows_non_temporal_join_without_time_grain(): """Time comparison without time grain is valid when join keys are non-temporal.""" df = DataFrame({"country": ["US", "UK"], "metric": [10, 20]}) @@ -360,27 +710,38 @@ def test_join_offset_dfs_allows_non_temporal_join_without_time_grain(): assert "metric__1 year ago" in result.columns -def test_join_offset_dfs_raises_when_temporal_key_not_first(): - """Temporal join key detection works even when it's not the first key.""" +def test_join_offset_dfs_no_time_grain_temporal_key_not_first() -> None: + """ + The temporal join key is aligned by shifting even when it is not the + first join key; remaining keys still participate in the join. + """ df = DataFrame( { "country": ["US", "UK"], - "ds": [Timestamp("2021-01-01"), Timestamp("2021-02-01")], + "ds": [Timestamp("2021-02-01"), Timestamp("2021-02-01")], "D": [1, 2], } ) offset_df = DataFrame( { "country": ["US", "UK"], - "ds": [Timestamp("2021-03-01"), Timestamp("2021-04-01")], + "ds": [Timestamp("2021-01-01"), Timestamp("2021-01-01")], "B": [5, 6], } ) - offset_dfs = {"1 year ago": offset_df} + offset_dfs = {"1 month ago": offset_df} - with pytest.raises( - QueryObjectValidationError, match="Time Grain must be specified" - ): - query_context_processor.join_offset_dfs( - df, offset_dfs, time_grain=None, join_keys=["country", "ds"] - ) + expected = DataFrame( + { + "country": ["US", "UK"], + "ds": [Timestamp("2021-02-01"), Timestamp("2021-02-01")], + "D": [1, 2], + "B": [5, 6], + } + ) + + result = query_context_processor.join_offset_dfs( + df, offset_dfs, time_grain=None, join_keys=["country", "ds"] + ) + + assert_frame_equal(expected, result) diff --git a/tests/unit_tests/utils/date_parser_tests.py b/tests/unit_tests/utils/date_parser_tests.py index 5d275f2f271..a4a5a0b6c6c 100644 --- a/tests/unit_tests/utils/date_parser_tests.py +++ b/tests/unit_tests/utils/date_parser_tests.py @@ -32,9 +32,13 @@ from superset.utils.date_parser import ( datetime_eval, get_past_or_future, get_since_until, + is_constant_human_timedelta, + is_parseable_human_timedelta, + normalize_time_delta, parse_human_datetime, parse_human_timedelta, parse_past_timedelta, + TimeDeltaAmbiguousError, ) from tests.unit_tests.conftest import with_feature_flags @@ -561,6 +565,109 @@ def test_get_past_or_future() -> None: assert get_past_or_future("3 month", dttm) == datetime(2020, 5, 29) +def test_get_past_or_future_quarters() -> None: + # parsedatetime has no notion of quarters (it would leave the source time + # unchanged); quarter phrases are rewritten to months before parsing + dttm = datetime(2024, 5, 15, 10, 30, 45) + assert get_past_or_future("1 quarter ago", dttm) == datetime( + 2024, 2, 15, 10, 30, 45 + ) + assert get_past_or_future("2 quarters ago", dttm) == datetime( + 2023, 11, 15, 10, 30, 45 + ) + assert get_past_or_future("1 quarter later", dttm) == datetime( + 2024, 8, 15, 10, 30, 45 + ) + assert get_past_or_future("1 QUARTER ago", dttm) == datetime( + 2024, 2, 15, 10, 30, 45 + ) + # absurdly long digit runs are not rewritten (bounded to keep matching + # linear and the month count small); they parse like any other + # unintelligible phrase, i.e. the source time comes back unchanged + assert get_past_or_future("0" * 100_000 + " quarters ago", dttm) == dttm + + +def test_parse_human_timedelta_unparseable_is_zero() -> None: + # the parser leaves unparseable phrases as the source time, so their + # delta is zero; a zero delta alone cannot distinguish them from + # legitimate zero-shift phrases (see is_parseable_human_timedelta) + dttm = datetime(2024, 5, 15) + assert parse_human_timedelta("not a real offset", dttm) == timedelta(0) + + +def test_is_parseable_human_timedelta() -> None: + assert is_parseable_human_timedelta("1 week ago") + assert is_parseable_human_timedelta("one year ago") + assert is_parseable_human_timedelta("1 quarter ago") + # zero-shift phrases parse successfully even though their delta is zero + assert is_parseable_human_timedelta("0 days ago") + assert not is_parseable_human_timedelta("not a real offset") + assert not is_parseable_human_timedelta("invalid-date-range") + assert not is_parseable_human_timedelta("") + assert not is_parseable_human_timedelta(None) + # absurdly long digit runs are not rewritten to months; they are + # unparseable like any other unintelligible phrase + assert not is_parseable_human_timedelta("9" * 100_000 + " quarters ago") + + +def test_is_constant_human_timedelta() -> None: + # phrases that shift every source time by the same amount + assert is_constant_human_timedelta("1 week ago") + assert is_constant_human_timedelta("one year ago") + assert is_constant_human_timedelta("1 quarter ago") + assert is_constant_human_timedelta("2 days later") + # a zero shift is still a constant one + assert is_constant_human_timedelta("0 days ago") + # anchors resolve to a fixed point instead: how far they move a source + # time depends on where in the day that source time falls + assert not is_constant_human_timedelta("yesterday") + assert not is_constant_human_timedelta("last month") + assert not is_constant_human_timedelta("noon") + # a phrase nothing can parse is not a delta either + assert not is_constant_human_timedelta("not a real offset") + assert not is_constant_human_timedelta("") + assert not is_constant_human_timedelta(None) + + +def test_is_constant_human_timedelta_matches_applied_shift() -> None: + """ + The probes stand in for real source timestamps, so the verdict has to hold + for source times they never looked at: an accepted phrase shifts arbitrary + timestamps equally, a rejected one does not. + """ + sources = [ + datetime(2021, 3, 14, 2, 30), + datetime(2021, 11, 7, 1, 30), + datetime(2024, 2, 29, 23, 59, 59), + datetime(2020, 1, 1, 0, 0), + ] + + deltas = {get_past_or_future("1 week ago", src) - src for src in sources} + assert deltas == {timedelta(days=-7)} + + anchored = {get_past_or_future("yesterday", src) - src for src in sources} + assert len(anchored) > 1 + + +def test_normalize_time_delta() -> None: + assert normalize_time_delta("30 seconds ago") == {"seconds": -30} + assert normalize_time_delta("5 minutes later") == {"minutes": 5} + assert normalize_time_delta("12 hours ago") == {"hours": -12} + assert normalize_time_delta("2 weeks ago") == {"weeks": -2} + assert normalize_time_delta("28 days ago") == {"days": -28} + assert normalize_time_delta("2 months later") == {"months": 2} + assert normalize_time_delta("1 year ago") == {"years": -1} + # quarters are converted to months (pd.DateOffset has no quarters argument) + assert normalize_time_delta("1 quarter ago") == {"months": -3} + assert normalize_time_delta("2 quarters later") == {"months": 6} + # matching is case-insensitive and the result uses lowercase keys + assert normalize_time_delta("1 QUARTER ago") == {"months": -3} + assert normalize_time_delta("1 Year AGO") == {"years": -1} + + with pytest.raises(TimeDeltaAmbiguousError): + normalize_time_delta("one year ago") + + def test_parse_human_datetime() -> None: with pytest.raises(TimeRangeAmbiguousError): parse_human_datetime("2 days")