sadpandajoe commented on code in PR #42054:
URL: https://github.com/apache/superset/pull/42054#discussion_r3598337819
##########
superset/models/helpers.py:
##########
@@ -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])
Review Comment:
Not changed — the suggested fix would introduce the very failure it
describes, and I could not find a reachable case for the original one.
Swapping in `is_datetime_series` makes the guard accept object-dtype dates,
but everything downstream still assumes `datetime64`, and the two sides are
treated asymmetrically: `df` gets shifted, `offset_df` is only stringified.
With an object-backed date axis on both sides (the realistic case — same
driver, same query shape), verified on pandas 2.2.3:
```
df side (shifted) : ['2021-01-01 00:00:00', '2021-01-02 00:00:00'] # date
+ DateOffset -> Timestamp
offset side (raw) : ['2021-01-01', '2021-01-02'] #
str(datetime.date)
-> zero overlap -> empty comparison
```
The free-form branch fares worse: `shift()` calls `value.floor("s")`, and
`datetime.date` has no `.floor` — `AttributeError` instead of a fallback.
Correctly supporting object-backed axes means coercing *both* sides with
`pd.to_datetime` before the shift, not broadening detection.
On reachability: `processing_time_offsets` runs on the output of
`normalize_df`, which sends every `is_dttm` base-axis label through
`normalize_dttm_col` -> `_process_datetime_column` -> `pd.to_datetime`, so the
temporal x-axis arrives as `datetime64`. The guard also matches the existing
convention in the same dispatch function — the grain-based branch has used
`is_datetime64_any_dtype` since a2722532438. `is_datetime_series` is used at
helpers.py:2011 to ask a different question (is this axis conceptually
temporal, for filter rewriting) rather than to gate datetime arithmetic. Where
the dtype genuinely is not `datetime64`, falling back to the raw join keys is
deliberate and non-fatal.
Happy to add the two-sided `pd.to_datetime` coercion if you have a concrete
query that lands an object-dtype temporal column in `processing_time_offsets`
after `normalize_df` — that is the piece I could not construct.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
