codeant-ai-for-open-source[bot] commented on code in PR #37014:
URL: https://github.com/apache/superset/pull/37014#discussion_r3741906919
##########
superset/models/helpers.py:
##########
@@ -1394,6 +1395,26 @@ def get_extra_cache_keys(self, query_obj:
QueryObjectDict) -> list[Hashable]:
def get_template_processor(self, **kwargs: Any) -> BaseTemplateProcessor:
raise NotImplementedError()
+ def get_dataset_timezone(self) -> str | None:
+ """
+ Get the timezone configured for this dataset from the extra JSON field.
+
+ Returns an IANA timezone name (e.g., "Europe/Berlin",
"America/New_York")
+ or None if not configured.
+
+ ``extra`` is arbitrary user-supplied JSON, so the ``timezone`` key
could
+ hold a non-string value (a number, object, list, ...). Only a string is
+ ever a valid IANA name, so anything else is treated as "not configured"
+ rather than propagating a bad value on to ``ZoneInfo``.
+
+ ``extra_dict`` is provided by concrete datasources (e.g. ``SqlaTable``)
+ rather than this mixin, so read it defensively: subclasses without it
+ simply have no configured timezone.
+ """
+ extra = getattr(self, "extra_dict", None) or {}
+ dataset_timezone = extra.get("timezone")
Review Comment:
**Suggestion:** `extra_dict` can contain any valid JSON value, not only an
object. For a non-empty list or string stored in the dataset's extra field,
`extra.get` raises `AttributeError` during normalization or time-filter
construction, causing the query to fail instead of treating the timezone as
unset. Verify that `extra` is a mapping before reading the key. [null pointer]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dataset query normalization raises `AttributeError`.
- ❌ Dashboard time-filter queries can fail before SQL execution.
- ⚠️ Invalid metadata disables affected dataset charts.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ab2b4358a9764404ae528b7be345d993&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=ab2b4358a9764404ae528b7be345d993&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/models/helpers.py
**Line:** 1414:1415
**Comment:**
*Null Pointer: `extra_dict` can contain any valid JSON value, not only
an object. For a non-empty list or string stored in the dataset's extra field,
`extra.get` raises `AttributeError` during normalization or time-filter
construction, causing the query to fail instead of treating the timezone as
unset. Verify that `extra` is a mapping before reading the key.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37014&comment_hash=5f691bd89685ceb7f47f9741926012a11226e069b85664e4c5648c6ac0699a85&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37014&comment_hash=5f691bd89685ceb7f47f9741926012a11226e069b85664e4c5648c6ac0699a85&reaction=dislike'>👎</a>
##########
superset/utils/core.py:
##########
@@ -2080,8 +2084,28 @@ def normalize_dttm_col(
_process_datetime_column(df, _col)
- if _col.offset:
+ if _col.timezone and isinstance(_col.timezone, str):
+ try:
+ tz = ZoneInfo(_col.timezone)
+ # Data is stored in UTC, convert to the dataset's configured
timezone
+ # First make the datetime UTC-aware, then convert to target
timezone
+ series = df[_col.col_label]
+ if not series.empty and series.notna().any():
+ # Convert UTC to target timezone
+ df[_col.col_label] = (
+ series.dt.tz_localize("UTC")
+ .dt.tz_convert(tz)
+ .dt.tz_localize(None) # Remove timezone info for
display
Review Comment:
**Suggestion:** The timezone conversion unconditionally calls
`tz_localize("UTC")` on the parsed series. If the database returns
timezone-aware timestamps, or the configured format parses an offset-bearing
value, the series is already timezone-aware and pandas raises `TypeError:
Already tz-aware, use tz_convert to convert`. Handle aware and naive series
separately: convert aware values directly and localize only naive values before
converting to the target timezone. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dashboard query results fail for timezone-aware temporal columns.
- ❌ Explore normalization aborts before returning chart data.
- ⚠️ Affects datasets using configured timezone plus aware database
timestamps.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=86b0b6a1527b4fabbe8c56e4961ffb4f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=86b0b6a1527b4fabbe8c56e4961ffb4f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/core.py
**Line:** 2095:2098
**Comment:**
*Type Error: The timezone conversion unconditionally calls
`tz_localize("UTC")` on the parsed series. If the database returns
timezone-aware timestamps, or the configured format parses an offset-bearing
value, the series is already timezone-aware and pandas raises `TypeError:
Already tz-aware, use tz_convert to convert`. Handle aware and naive series
separately: convert aware values directly and localize only naive values before
converting to the target timezone.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37014&comment_hash=d89cb59612d81d7a8cdcf8f4616ab3ad29f2c68fb1e98abf7847f9d86b8f33f1&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37014&comment_hash=d89cb59612d81d7a8cdcf8f4616ab3ad29f2c68fb1e98abf7847f9d86b8f33f1&reaction=dislike'>👎</a>
##########
superset/models/helpers.py:
##########
@@ -1959,11 +1980,18 @@ def normalize_df(self, df: pd.DataFrame, query_object:
QueryObject) -> pd.DataFr
"""
labels = self._collect_dttm_labels(query_object)
+ # ``get_dataset_timezone`` lives on ``ExploreMixin``; datasource
doubles
+ # that bind only a subset of mixin methods onto a plain object (as some
+ # unit tests do) won't have it, so fall back to "not configured" rather
+ # than raising.
+ get_dataset_timezone = getattr(self, "get_dataset_timezone", None)
+ dataset_timezone = get_dataset_timezone() if get_dataset_timezone else
None
dttm_cols = [
DateColumn(
timestamp_format=fmt,
offset=self.offset,
time_shift=query_object.time_shift,
+ timezone=dataset_timezone,
col_label=label,
Review Comment:
**Suggestion:** The timezone is propagated only to the columns collected by
`_collect_dttm_labels` and the legacy `__time` column. Native datetime columns
handled by `_offset_only_dttm_cols` still receive no `timezone`, and that
helper returns no columns when only a dataset timezone is configured. As a
result, secondary temporal columns remain in UTC while the selected time column
is converted to the dataset timezone. Pass `dataset_timezone` when constructing
those offset-only `DateColumn` instances and ensure the helper runs for
timezone-only configuration. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Secondary datetime columns remain displayed in UTC.
- ⚠️ Dashboard result tables show inconsistent temporal values.
- ⚠️ Timezone-only datasets skip native-column normalization.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c37572a36e994c2daadee8495cb61b4e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c37572a36e994c2daadee8495cb61b4e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/models/helpers.py
**Line:** 1994:1995
**Comment:**
*Incomplete Implementation: The timezone is propagated only to the
columns collected by `_collect_dttm_labels` and the legacy `__time` column.
Native datetime columns handled by `_offset_only_dttm_cols` still receive no
`timezone`, and that helper returns no columns when only a dataset timezone is
configured. As a result, secondary temporal columns remain in UTC while the
selected time column is converted to the dataset timezone. Pass
`dataset_timezone` when constructing those offset-only `DateColumn` instances
and ensure the helper runs for timezone-only configuration.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37014&comment_hash=206535a3e636814c263a8de7b183fe1dc5f9f2e6e2dae40eeae953650a297a84&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37014&comment_hash=206535a3e636814c263a8de7b183fe1dc5f9f2e6e2dae40eeae953650a297a84&reaction=dislike'>👎</a>
--
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]
