rebenitez1802 commented on code in PR #45571:
URL: https://github.com/apache/superset/pull/45571#discussion_r4365589026
##########
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:
🟡 Medium (non-blocking) — Dashboard Time Grain no longer applies to pre-PR
Sunbursts on semantic views
This drops `time_grain_sqla` whenever no level is grained. A Sunburst saved
before this PR has no `temporal_columns_lookup`, so none of its levels are
grained. On master, the semantic mapper's `_get_grain_time_axis_column` applied
`extras.time_grain_sqla` to the single temporal level. So a saved semantic-view
Sunburst with levels `['order_date', 'category']`, on a dashboard with a Time
Grain filter, shows months on master and raw dates here. That happens on the
dashboard and on the MCP form-data path (`_sunburst_temporal_grain`, see the
next comment).
It's behind the development-stage `SEMANTIC_LAYERS` flag, totals stay
correct, and re-saving in Explore fixes it, so I'm not blocking on it. Dropping
the grain only when a lookup is present keeps master's behaviour for charts
without one. `!= null` here and `is not None` in the backend agree for every
JSON value, including `{}`. All PR suites pass with both changes. A JS and a
Python test for a chart without a lookup keeping the grain would pin it.
```suggestion
if (
!grained &&
!baseQueryObject.granularity &&
formData.temporal_columns_lookup != null
) {
// A grain with neither a temporal level nor a legacy granularity
subject
// is dormant; semantic views reject a grain that has no time column.
// Without a lookup, temporal levels are unknown, so the grain is kept.
```
##########
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:
🟡 Medium (non-blocking) — same as the `buildQuery.ts` comment: the backend
mirror should keep the grain when the chart has no lookup.
```suggestion
if (
not grained
and not query.get("granularity")
# Without a lookup, temporal levels are unknown, so the grain is
kept.
and form_data.get("temporal_columns_lookup") is not None
and "time_grain_sqla" in extras
):
```
--
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]
