Twiink commented on code in PR #45671:
URL: https://github.com/apache/superset/pull/45671#discussion_r4424814193
##########
superset/common/query_object.py:
##########
@@ -782,11 +784,54 @@ def exec_post_processing(self, df: DataFrame) ->
DataFrame:
func = extra_ops[operation]
options = post_process.get("options", {})
if operation == "resample":
- options = self._resolve_resample_options(options)
+ options = self._resolve_resample_options(options, df)
df = func(df, **options)
return df
- def _resolve_resample_options(self, options: dict[str, Any]) -> dict[str,
Any]:
+ def _resample_display_bound(
+ self, bound: datetime | None, df: DataFrame
+ ) -> datetime | None:
+ """Align explicit instants with a datasource's normalized display
frame."""
+ if (
+ bound is None
+ or bound.utcoffset() is None
+ or self.datasource is None
+ or getattr(df.index, "tz", None) is not None
+ ):
+ return bound
+ column_type = next(
+ (
+ col.type
+ for col in getattr(self.datasource, "columns", [])
+ if col.column_name == self.granularity
Review Comment:
Fixed in d9ed4f31. Calendar-bound detection now resolves the physical
base-axis column as well as legacy granularity, including an aliased x-axis.
Added regressions for DATE and date-formatted VARCHAR columns comparing the SQL
filter with fill_time_range boundaries. The reported May 1–June 1 range no
longer pads an April 30 row.
##########
superset/jinja_context.py:
##########
@@ -727,6 +728,37 @@ def _get_filters_from_query_context(
return filters
# pylint: disable=too-many-arguments
+ def _normalize_time_filter_bounds(
+ self,
+ start: datetime | None,
+ end: datetime | None,
+ column: str | None,
+ target_type: str | None,
+ ) -> tuple[datetime | None, datetime | None]:
+ """Use the same bound interpretation as the datasource's chart
filter."""
+ if self.table:
+ time_col = next(
+ (
+ col
+ for col in self.table.columns
+ if col.column_name == (column or self.table.main_dttm_col)
+ ),
+ None,
+ )
+ return self.table.adjust_time_bounds(
+ start, end, time_col, target_type=target_type
+ )
Review Comment:
Fixed in d9ed4f31 following your suggestion. The macro preserves each naive
endpoint's original wall-clock value and applies the datasource adjustment only
to explicit-offset endpoints. Added naive and mixed-range regressions with
Asia/Shanghai and Hour Offset 8; the naive May 1/June 1 values remain unchanged.
##########
superset/models/helpers.py:
##########
@@ -5881,48 +5894,51 @@ def adjust_time_bounds(
against the engine -- see exactly the instants the clause compares
against, rather than a copy of this logic that can drift.
"""
- dataset_timezone = self.get_dataset_timezone()
-
- if dataset_timezone and (start_dttm or end_dttm):
+ tz = None
+ if dataset_timezone := self.get_dataset_timezone():
try:
tz = ZoneInfo(dataset_timezone)
-
- # The datetimes from the UI are naive (no timezone info). We
- # interpret them as being in the dataset's configured timezone
and
- # convert them to UTC for comparison with UTC-stored data.
- if start_dttm is not None:
- start_dttm = (
- start_dttm.replace(tzinfo=tz)
- .astimezone(timezone.utc)
- .replace(tzinfo=None)
- )
- if end_dttm is not None:
- end_dttm = (
- end_dttm.replace(tzinfo=tz)
- .astimezone(timezone.utc)
- .replace(tzinfo=None)
- )
except (ZoneInfoNotFoundError, TypeError):
logger.warning(
"Invalid timezone %r in dataset extra; falling back to
Hour "
"Offset if configured",
dataset_timezone,
)
- dataset_timezone = None
-
- if not dataset_timezone and (offset_hours := getattr(self, "offset",
0) or 0):
+ column_type = target_type or (time_col.type if time_col is not None
else None)
+ sqla_type = (
+ self.db_engine_spec.get_sqla_column_type(column_type)
+ if column_type
+ else None
+ )
+ is_date = isinstance(sqla_type, sa.Date) and not isinstance(
Review Comment:
Fixed in d9ed4f31. Calendar detection follows the SQL literal
representation: date-only python_date_format values retain their calendar
boundaries, including the database-level format fallback. Native timestamp
conversion still takes precedence over a format hint. Added PostgreSQL
SQL-rendering regressions for %Y-%m-%d, %Y%m%d, timestamp strings, and a native
TIMESTAMP with a date-only hint.
--
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]
