aminghadersohi commented on code in PR #45484:
URL: https://github.com/apache/superset/pull/45484#discussion_r4337322771
##########
superset/mcp_service/chart/sunburst.py:
##########
@@ -39,6 +39,93 @@ class SunburstResultRoles:
secondary_metric: str | None = None
+def grained_column_reference(column: Any) -> str | None:
+ """Return the column name of a grained hierarchy level, else None.
+
+ Recognizes only the BASE_AXIS column-reference shape written by
+ :func:`apply_sunburst_hierarchy_time_grain`; arbitrary Custom SQL columns
+ are not treated as physical references.
+ """
+ if (
+ isinstance(column, Mapping)
+ and column.get("columnType") == "BASE_AXIS"
+ and column.get("expressionType") == "SQL"
+ and column.get("isColumnReference") is True
+ and isinstance(name := column.get("sqlExpression"), str)
+ and name
+ and column.get("label") == name
+ ):
+ return name
+ return None
+
+
+def restore_sunburst_hierarchy_time_grain(form_data: dict[str, Any]) -> None:
+ """Recover the temporal pair from an Explore-saved columns control."""
+ if any(
+ key in form_data
+ for key in (
+ "temporal_column",
+ "granularity_sqla",
+ "time_grain",
+ "time_grain_sqla",
+ )
+ ):
+ return
+ columns = form_data.get("columns")
+ if not isinstance(columns, list):
+ return
+ levels = [
+ (name, column["timeGrain"])
+ for column in columns
+ if (name := grained_column_reference(column)) is not None
+ and column.get("timeGrain") not in (None, "")
+ ]
+ if len(levels) == 1:
+ form_data["granularity_sqla"], form_data["time_grain_sqla"] = levels[0]
+
+
+def apply_sunburst_hierarchy_time_grain(form_data: dict[str, Any]) ->
dict[str, Any]:
+ """Carry the Sunburst time grain on the temporal hierarchy level.
+
+ The Sunburst control panel has no ``granularity_sqla`` or
+ ``time_grain_sqla`` control, and Explore builds its chart-data request from
+ controls only, so those top-level keys never reach the query when a saved
+ chart is opened in Explore. A hierarchy level that is the temporal column
+ therefore carries the grain itself, in the BASE_AXIS column-reference shape
+ the frontend ``normalizeTimeColumn`` emits: the ``columns`` control keeps
it
+ and buildQuery forwards it unchanged. Without a grain, a grained level
+ reverts to the plain column name. The top-level keys are kept because
+ dashboards, validation and update merges read them.
+ """
+ columns = form_data.get("columns")
+ if not isinstance(columns, list):
+ return form_data
+ temporal_column = form_data.get("granularity_sqla")
+ grain = form_data.get("time_grain_sqla")
+ rewritten: list[Any] = []
+ for column in columns:
+ name = column if isinstance(column, str) else
grained_column_reference(column)
+ if name is None or (
+ "granularity_sqla" not in form_data and "time_grain_sqla" not in
form_data
+ ):
+ rewritten.append(column)
+ elif isinstance(grain, str) and grain and name == temporal_column:
+ rewritten.append(
+ {
+ "columnType": "BASE_AXIS",
+ "expressionType": "SQL",
+ "isColumnReference": True,
+ "label": name,
+ "sqlExpression": name,
+ "timeGrain": grain,
+ }
+ )
Review Comment:
Taking your split: the code this finding is about is out of this PR as of
commit 2629e1c3b6ed2abdd28fe591c88a9fc490801763, tracked separately. That
revert takes out the Sunburst BASE_AXIS column storage, its
chart_utils.py/chart_helpers.py/schemas.py/datasource_resolver.py wiring and
the semantic_layers/mapper.py per-column grain change, so those files match
master again.
Remaining: this finding is not fixed; it moves with the Sunburst change,
which will carry it.
##########
superset/semantic_layers/mapper.py:
##########
@@ -1320,6 +1318,42 @@ def _get_grain_time_axis_column(
return None
+def _get_grouping_grains(
+ query_object: ValidatedQueryObject,
+ all_dimensions: dict[str, Dimension],
+) -> dict[str, Grain | None]:
+ """Resolve legacy grain defaults and explicit per-column temporal grains.
+
+ BASE_AXIS references retain their own grains when Explore drops top-level
+ time controls. Like SQL datasource queries, column-local grains take
+ precedence over the legacy extras default and identify their own subjects.
+ """
+ axis = _get_grain_time_axis_column(query_object, all_dimensions)
+ default_grain = query_object.extras.get("time_grain_sqla")
+ requested_grains = (
+ {axis: default_grain} if axis is not None and default_grain else {}
+ )
+ dimension_names = set(all_dimensions)
+ for column in query_object.columns:
+ if (
+ isinstance(column, dict)
+ and column.get("columnType") == "BASE_AXIS"
+ and column.get("isColumnReference") is True
+ ):
+ name = _normalize_column(column, dimension_names)
+ requested_grains[name] = column.get("timeGrain")
Review Comment:
Taking your split: the code this finding is about is out of this PR as of
commit 2629e1c3b6ed2abdd28fe591c88a9fc490801763, tracked separately. That
revert takes out the Sunburst BASE_AXIS column storage, its
chart_utils.py/chart_helpers.py/schemas.py/datasource_resolver.py wiring and
the semantic_layers/mapper.py per-column grain change, so those files match
master again.
Remaining: this finding is not fixed; it moves with the Sunburst change,
which will carry it.
##########
superset/mcp_service/chart/sunburst.py:
##########
@@ -39,6 +39,93 @@ class SunburstResultRoles:
secondary_metric: str | None = None
+def grained_column_reference(column: Any) -> str | None:
+ """Return the column name of a grained hierarchy level, else None.
+
+ Recognizes only the BASE_AXIS column-reference shape written by
+ :func:`apply_sunburst_hierarchy_time_grain`; arbitrary Custom SQL columns
+ are not treated as physical references.
+ """
+ if (
+ isinstance(column, Mapping)
+ and column.get("columnType") == "BASE_AXIS"
+ and column.get("expressionType") == "SQL"
+ and column.get("isColumnReference") is True
+ and isinstance(name := column.get("sqlExpression"), str)
+ and name
+ and column.get("label") == name
+ ):
+ return name
+ return None
+
+
+def restore_sunburst_hierarchy_time_grain(form_data: dict[str, Any]) -> None:
+ """Recover the temporal pair from an Explore-saved columns control."""
+ if any(
+ key in form_data
+ for key in (
+ "temporal_column",
+ "granularity_sqla",
+ "time_grain",
+ "time_grain_sqla",
+ )
+ ):
+ return
+ columns = form_data.get("columns")
+ if not isinstance(columns, list):
+ return
+ levels = [
+ (name, column["timeGrain"])
+ for column in columns
+ if (name := grained_column_reference(column)) is not None
+ and column.get("timeGrain") not in (None, "")
+ ]
+ if len(levels) == 1:
+ form_data["granularity_sqla"], form_data["time_grain_sqla"] = levels[0]
+
+
+def apply_sunburst_hierarchy_time_grain(form_data: dict[str, Any]) ->
dict[str, Any]:
+ """Carry the Sunburst time grain on the temporal hierarchy level.
+
+ The Sunburst control panel has no ``granularity_sqla`` or
+ ``time_grain_sqla`` control, and Explore builds its chart-data request from
+ controls only, so those top-level keys never reach the query when a saved
+ chart is opened in Explore. A hierarchy level that is the temporal column
+ therefore carries the grain itself, in the BASE_AXIS column-reference shape
+ the frontend ``normalizeTimeColumn`` emits: the ``columns`` control keeps
it
+ and buildQuery forwards it unchanged. Without a grain, a grained level
+ reverts to the plain column name. The top-level keys are kept because
+ dashboards, validation and update merges read them.
+ """
+ columns = form_data.get("columns")
+ if not isinstance(columns, list):
+ return form_data
+ temporal_column = form_data.get("granularity_sqla")
+ grain = form_data.get("time_grain_sqla")
+ rewritten: list[Any] = []
+ for column in columns:
+ name = column if isinstance(column, str) else
grained_column_reference(column)
+ if name is None or (
+ "granularity_sqla" not in form_data and "time_grain_sqla" not in
form_data
+ ):
+ rewritten.append(column)
+ elif isinstance(grain, str) and grain and name == temporal_column:
Review Comment:
Taking your split: the code this finding is about is out of this PR as of
commit 2629e1c3b6ed2abdd28fe591c88a9fc490801763, tracked separately. That
revert takes out the Sunburst BASE_AXIS column storage, its
chart_utils.py/chart_helpers.py/schemas.py/datasource_resolver.py wiring and
the semantic_layers/mapper.py per-column grain change, so those files match
master again.
Remaining: this finding is not fixed; it moves with the Sunburst change,
which will carry it.
##########
superset/mcp_service/chart/sunburst.py:
##########
@@ -39,6 +39,93 @@ class SunburstResultRoles:
secondary_metric: str | None = None
+def grained_column_reference(column: Any) -> str | None:
+ """Return the column name of a grained hierarchy level, else None.
+
+ Recognizes only the BASE_AXIS column-reference shape written by
+ :func:`apply_sunburst_hierarchy_time_grain`; arbitrary Custom SQL columns
+ are not treated as physical references.
+ """
+ if (
+ isinstance(column, Mapping)
+ and column.get("columnType") == "BASE_AXIS"
+ and column.get("expressionType") == "SQL"
+ and column.get("isColumnReference") is True
+ and isinstance(name := column.get("sqlExpression"), str)
+ and name
+ and column.get("label") == name
+ ):
+ return name
+ return None
+
+
+def restore_sunburst_hierarchy_time_grain(form_data: dict[str, Any]) -> None:
+ """Recover the temporal pair from an Explore-saved columns control."""
+ if any(
+ key in form_data
+ for key in (
+ "temporal_column",
+ "granularity_sqla",
+ "time_grain",
+ "time_grain_sqla",
+ )
+ ):
+ return
+ columns = form_data.get("columns")
+ if not isinstance(columns, list):
+ return
+ levels = [
+ (name, column["timeGrain"])
+ for column in columns
+ if (name := grained_column_reference(column)) is not None
+ and column.get("timeGrain") not in (None, "")
+ ]
+ if len(levels) == 1:
+ form_data["granularity_sqla"], form_data["time_grain_sqla"] = levels[0]
+
+
+def apply_sunburst_hierarchy_time_grain(form_data: dict[str, Any]) ->
dict[str, Any]:
+ """Carry the Sunburst time grain on the temporal hierarchy level.
+
+ The Sunburst control panel has no ``granularity_sqla`` or
+ ``time_grain_sqla`` control, and Explore builds its chart-data request from
+ controls only, so those top-level keys never reach the query when a saved
+ chart is opened in Explore. A hierarchy level that is the temporal column
+ therefore carries the grain itself, in the BASE_AXIS column-reference shape
+ the frontend ``normalizeTimeColumn`` emits: the ``columns`` control keeps
it
+ and buildQuery forwards it unchanged. Without a grain, a grained level
+ reverts to the plain column name. The top-level keys are kept because
+ dashboards, validation and update merges read them.
+ """
+ columns = form_data.get("columns")
+ if not isinstance(columns, list):
+ return form_data
+ temporal_column = form_data.get("granularity_sqla")
+ grain = form_data.get("time_grain_sqla")
+ rewritten: list[Any] = []
+ for column in columns:
+ name = column if isinstance(column, str) else
grained_column_reference(column)
+ if name is None or (
+ "granularity_sqla" not in form_data and "time_grain_sqla" not in
form_data
+ ):
+ rewritten.append(column)
+ elif isinstance(grain, str) and grain and name == temporal_column:
+ rewritten.append(
+ {
+ "columnType": "BASE_AXIS",
+ "expressionType": "SQL",
+ "isColumnReference": True,
+ "label": name,
+ "sqlExpression": name,
+ "timeGrain": grain,
+ }
+ )
+ else:
+ rewritten.append(name)
+ form_data["columns"] = rewritten
Review Comment:
Taking your split: the code this finding is about is out of this PR as of
commit 2629e1c3b6ed2abdd28fe591c88a9fc490801763, tracked separately. That
revert takes out the Sunburst BASE_AXIS column storage, its
chart_utils.py/chart_helpers.py/schemas.py/datasource_resolver.py wiring and
the semantic_layers/mapper.py per-column grain change, so those files match
master again.
Remaining: this finding is not fixed; it moves with the Sunburst change,
which will carry it.
##########
superset/mcp_service/chart/sunburst.py:
##########
@@ -39,6 +39,93 @@ class SunburstResultRoles:
secondary_metric: str | None = None
+def grained_column_reference(column: Any) -> str | None:
+ """Return the column name of a grained hierarchy level, else None.
+
+ Recognizes only the BASE_AXIS column-reference shape written by
+ :func:`apply_sunburst_hierarchy_time_grain`; arbitrary Custom SQL columns
+ are not treated as physical references.
+ """
+ if (
+ isinstance(column, Mapping)
+ and column.get("columnType") == "BASE_AXIS"
+ and column.get("expressionType") == "SQL"
+ and column.get("isColumnReference") is True
+ and isinstance(name := column.get("sqlExpression"), str)
+ and name
+ and column.get("label") == name
Review Comment:
Taking your split: the code this finding is about is out of this PR as of
commit 2629e1c3b6ed2abdd28fe591c88a9fc490801763, tracked separately. That
revert takes out the Sunburst BASE_AXIS column storage, its
chart_utils.py/chart_helpers.py/schemas.py/datasource_resolver.py wiring and
the semantic_layers/mapper.py per-column grain change, so those files match
master again.
Remaining: this finding is not fixed; it moves with the Sunburst change,
which will carry it.
--
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]
