rusackas commented on code in PR #41184:
URL: https://github.com/apache/superset/pull/41184#discussion_r3565611254
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx:
##########
@@ -341,22 +296,45 @@ export default function PivotTableChart(props:
PivotTableProps) {
const unpivotedData = useMemo(
() =>
- data.reduce(
- (acc: Record<string, any>[], record: Record<string, any>) => [
- ...acc,
- ...metricNames
+ // `data` is now one entry per rollup level. Tag every record with the
+ // row/column dimension labels of the level that produced it (mirroring
+ // the METRIC_KEY injection used for the full rows/cols below) so
+ // PivotData can slot each pre-computed value without re-aggregating.
+ // buildGroupbyCombinations already applied transposePivot, so the
level's
+ // groupby is display-oriented and is not transposed again here.
+ data.flatMap((query: QueryData) => {
+ let levelRows = query.groupby.rows.map(getColumnLabel);
+ let levelCols = query.groupby.columns.map(getColumnLabel);
+ if (metricsLayout === MetricsLayoutEnum.ROWS) {
+ levelRows = combineMetric
+ ? [...levelRows, METRIC_KEY]
+ : [METRIC_KEY, ...levelRows];
+ } else {
+ levelCols = combineMetric
+ ? [...levelCols, METRIC_KEY]
+ : [METRIC_KEY, ...levelCols];
+ }
+ return query.data.flatMap((record: Record<string, any>) =>
+ metricNames
.map((name: string) => ({
...record,
[METRIC_KEY]: name,
value: record[name],
// Mark currency column for per-cell currency detection in
aggregators
__currencyColumn: currencyCodeColumn,
+ // The level this record belongs to (used by PivotData
placement).
+ // Namespaced with a `__` prefix (like `__metricKey` below) so it
+ // can't collide with a real dataset column named
`rows`/`columns`.
+ __rows: levelRows,
+ __columns: levelCols,
+ // Identify the metric pseudo-dimension so PivotData can feed the
+ // metric-collapsed totals (the opposite "Total" axis + corner).
+ __metricKey: METRIC_KEY,
}))
- .filter(record => record.value !== null),
- ],
- [],
- ),
- [data, metricNames, currencyCodeColumn],
+ .filter(r => r.value !== null),
Review Comment:
Good catch, this could drop a whole row instead of showing a blank cell when
a DB-computed total is null. Kept the record and taught the passthrough
formatter to render `null` as an empty cell instead of the literal `null`
string.
##########
superset/common/query_context_processor.py:
##########
@@ -252,9 +256,60 @@ def get_query_result(self, query_object: QueryObject) ->
QueryResult:
This method delegates to the datasource's get_query_result method,
which handles query execution, normalization, time offsets, and
post-processing.
+
+ When the query requests rollup ``grouping_sets`` but the engine does
not
+ support native ``GROUPING SETS``, fall back to one query per level and
+ concatenate the results with ``GROUPING()``-equivalent markers, so the
+ combined result matches the shape the native path produces (SIP.md,
+ phase 3b). Engines that support it run the single native query.
"""
+ if query_object.grouping_sets and not self._supports_grouping_sets():
+ return self._grouping_sets_fallback(query_object)
return self._qc_datasource.get_query_result(query_object)
+ def _supports_grouping_sets(self) -> bool:
+ engine_spec: BaseEngineSpec | None = getattr(
+ self._qc_datasource, "db_engine_spec", None
+ )
+ return bool(engine_spec and engine_spec.supports_grouping_sets)
+
+ def _grouping_sets_fallback(self, query_object: QueryObject) ->
QueryResult:
+ """
+ Emulate a GROUPING SETS query on engines without native support: run
one
+ query per rollup level and concatenate, tagging each level's rows with
+ the same per-column markers the native path emits.
+ """
+ levels: list[list[str]] = query_object.grouping_sets
+ # Use the same label derivation as the native path (physical column
name
+ # or adhoc column label) so both column kinds are represented and each
+ # label maps back to its own column, in the same order as the source
+ # list.
+ all_labels = [get_column_name(col) for col in query_object.columns]
+ label_to_column = dict(zip(all_labels, query_object.columns,
strict=True))
+
+ frames: list[pd.DataFrame] = []
+ result: QueryResult | None = None
+ for level in levels:
+ level_labels = set(level)
+ sub_query = copy.copy(query_object)
+ sub_query.grouping_sets = []
+ sub_query.columns = [
+ label_to_column[label] for label in all_labels if label in
level_labels
+ ]
Review Comment:
Good catch, fixed by clearing `row_limit` on each fallback level. The native
GROUPING SETS path skips it entirely (see the `use_grouping_sets` check in
`models/helpers.py`), so this brings the fallback in line with 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]