aminghadersohi commented on code in PR #45571:
URL: https://github.com/apache/superset/pull/45571#discussion_r4365979645
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/buildQuery.ts:
##########
@@ -16,14 +16,72 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { buildQueryContext, QueryFormData } from '@superset-ui/core';
+import {
+ AdhocColumn,
+ buildQueryContext,
+ ensureIsArray,
+ isPhysicalColumn,
+ QueryFormColumn,
+ QueryFormData,
+} from '@superset-ui/core';
+
+import { isGrainedLevel } from './timeGrain';
+
+// Wrap each temporal hierarchy level in the BASE_AXIS column reference that
+// carries the time grain, the shape ``normalizeTimeColumn`` uses for a
physical
+// x-axis. Non-temporal levels are left untouched.
+function applyTimeGrain(
+ columns: QueryFormColumn[],
+ formData: QueryFormData,
+ timeGrain: string | undefined,
+): { columns: QueryFormColumn[]; grained: boolean } {
+ let grained = false;
+ const mapped = columns.map(column => {
+ if (
+ timeGrain &&
+ isPhysicalColumn(column) &&
+ isGrainedLevel(
+ column,
+ formData.temporal_columns_lookup,
+ formData.extra_form_data?.granularity_sqla ??
formData.granularity_sqla,
+ )
+ ) {
+ grained = true;
+ return {
+ timeGrain,
+ columnType: 'BASE_AXIS',
+ sqlExpression: column,
+ label: column,
+ expressionType: 'SQL',
+ isColumnReference: true,
+ } as AdhocColumn;
+ }
+ return column;
+ });
+ return { columns: mapped, grained };
+}
export default function buildQuery(formData: QueryFormData) {
const { metric, sort_by_metric } = formData;
- return buildQueryContext(formData, baseQueryObject => [
- {
- ...baseQueryObject,
- ...(sort_by_metric && { orderby: [[metric, false]] }),
- },
- ]);
+ return buildQueryContext(formData, baseQueryObject => {
+ const { columns, grained } = applyTimeGrain(
+ ensureIsArray(baseQueryObject.columns),
+ formData,
+ baseQueryObject.extras?.time_grain_sqla,
+ );
+ const extras = { ...baseQueryObject.extras };
+ if (!grained && !baseQueryObject.granularity) {
+ // A grain with neither a temporal level nor a legacy granularity subject
+ // is dormant; semantic views reject a grain that has no time column.
Review Comment:
Fixed in ba1e5ab959efee70bda288c11dea823bd795fd92. Missing/null lookup
metadata retains the dashboard grain; an explicit empty lookup still drops a
dormant grain.
Regression:
`superset-frontend/plugins/plugin-chart-echarts/test/Sunburst/buildQuery.test.ts`,
“a legacy semantic Sunburst retains a dashboard grain unless metadata rules it
out: %p”: missing/null cases failed before and pass after (2 failures before;
all 150 branch Jest tests pass after). Backend parity and semantic mapping are
pinned by `test_legacy_sunburst_dashboard_grain_without_lookup` and
`test_legacy_sunburst_dashboard_grain_reaches_semantic_mapper` (8 failures
before; 916 Python tests pass after in Docker).
Siblings checked:
-
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/buildQuery.ts:73 —
fixed in ba1e5ab959efee70bda288c11dea823bd795fd92
- superset/common/form_data_query_context.py:647 — fixed in
ba1e5ab959efee70bda288c11dea823bd795fd92
- superset/mcp_service/chart/chart_helpers.py:1243 — fixed in
ba1e5ab959efee70bda288c11dea823bd795fd92
- superset/common/form_data_query_context.py:1773 — fixed in
ba1e5ab959efee70bda288c11dea823bd795fd92
- superset/semantic_layers/mapper.py:1351 — not affected: already infers a
single temporal dimension when the grain is retained; pinned by
test_legacy_sunburst_dashboard_grain_reaches_semantic_mapper.
- superset/common/form_data_query_context.py:580 — not affected: pivot
temporalization does not remove the extras grain.
- superset/common/form_data_query_context.py:660 — not affected: box-plot
temporalization does not remove the extras grain.
- superset/common/form_data_query_context.py:682 — not affected: table
temporalization does not use the Sunburst grain-removal branch.
- superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/timeGrain.ts —
not affected: drill/label selection does not remove query extras; inferring
semantic dimension types on the client is outside this compatibility change.
##########
superset/common/form_data_query_context.py:
##########
@@ -604,6 +607,50 @@ def _temporalized_columns(form_data: dict[str, Any],
columns: list[Any]) -> list
return result
+def _sunburst_temporal_grain(form_data: dict[str, Any], query: dict[str, Any])
-> None:
+ """Mirror Sunburst buildQuery: grain only its temporal hierarchy levels.
+
+ Each temporal level becomes the BASE_AXIS column reference
+ ``normalizeTimeColumn`` emits for a physical x-axis. A grain with neither a
+ temporal level nor a legacy ``granularity`` subject is dormant and is
+ dropped, since semantic views reject a grain that has no time column.
+ """
+ extras = dict(query.get("extras") or {})
+ time_grain = extras.get("time_grain_sqla")
+ temporal_lookup = form_data.get("temporal_columns_lookup")
+ if not isinstance(temporal_lookup, Mapping):
+ temporal_lookup = {}
+ subject = query.get("granularity")
+ grained = False
+ columns: list[Any] = []
+ for column in _as_list(query.get("columns")):
+ if (
+ time_grain
+ and isinstance(column, str)
+ and (subject == column or temporal_lookup.get(column))
+ ):
+ grained = True
+ columns.append(
+ {
+ "timeGrain": time_grain,
+ "columnType": "BASE_AXIS",
+ "sqlExpression": column,
+ "label": column,
+ "expressionType": "SQL",
+ "isColumnReference": True,
+ }
+ )
+ else:
+ columns.append(column)
+ query["columns"] = columns
+ if not grained and not query.get("granularity") and "time_grain_sqla" in
extras:
Review Comment:
Fixed in ba1e5ab959efee70bda288c11dea823bd795fd92. The mirror preserves the
grain when the saved lookup is absent or null, but still removes it for an
explicit empty lookup.
Regression:
`tests/unit_tests/common/test_form_data_query_context.py::test_legacy_sunburst_dashboard_grain_without_lookup`
exercises the shared builder, export fallback, and MCP dashboard override;
`tests/unit_tests/semantic_layers/mapper_test.py::test_legacy_sunburst_dashboard_grain_reaches_semantic_mapper`
asserts the mapped temporal dimension's grain. Missing/null cases failed on
the pre-fix head (8 failures); all 916 selected Python tests pass after in
Docker. The matching frontend regression also fails before and passes after
(150 branch Jest tests pass).
Siblings checked:
-
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/buildQuery.ts:73 —
fixed in ba1e5ab959efee70bda288c11dea823bd795fd92
- superset/common/form_data_query_context.py:647 — fixed in
ba1e5ab959efee70bda288c11dea823bd795fd92
- superset/mcp_service/chart/chart_helpers.py:1243 — fixed in
ba1e5ab959efee70bda288c11dea823bd795fd92
- superset/common/form_data_query_context.py:1773 — fixed in
ba1e5ab959efee70bda288c11dea823bd795fd92
- superset/semantic_layers/mapper.py:1351 — not affected: already infers a
single temporal dimension when the grain is retained; pinned by
test_legacy_sunburst_dashboard_grain_reaches_semantic_mapper.
- superset/common/form_data_query_context.py:580 — not affected: pivot
temporalization does not remove the extras grain.
- superset/common/form_data_query_context.py:660 — not affected: box-plot
temporalization does not remove the extras grain.
- superset/common/form_data_query_context.py:682 — not affected: table
temporalization does not use the Sunburst grain-removal branch.
- superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/timeGrain.ts —
not affected: drill/label selection does not remove query extras; inferring
semantic dimension types on the client is outside this compatibility change.
--
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]
