rusackas commented on code in PR #41184:
URL: https://github.com/apache/superset/pull/41184#discussion_r3565612015
##########
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: list[str] = [get_column_name(col) for col in
query_object.columns]
+ label_to_column = dict(zip(all_labels, query_object.columns,
strict=True))
Review Comment:
Added the type hint.
##########
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: list[str] = [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)
Review Comment:
Added the type hint.
##########
superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/transformProps.test.ts:
##########
@@ -64,36 +63,73 @@ const chartProps = new ChartProps<QueryFormData>({
theme: supersetTheme,
Review Comment:
Fixed, typed as `QueryData` instead.
##########
superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/transformProps.test.ts:
##########
@@ -64,36 +63,73 @@ const chartProps = new ChartProps<QueryFormData>({
theme: supersetTheme,
});
-test('should transform chart props for viz', () => {
- expect(transformProps(chartProps)).toEqual({
- width: 800,
- height: 600,
- groupbyRows: ['row1', 'row2'],
- groupbyColumns: ['col1', 'col2'],
- metrics: ['metric1', 'metric2'],
- tableRenderer: 'Table With Subtotal',
- colOrder: 'key_a_to_z',
- rowOrder: 'key_a_to_z',
- aggregateFunction: 'Sum',
- transposePivot: true,
- combineMetric: true,
- rowSubtotalPosition: true,
- colSubtotalPosition: true,
+test('should pass through formData props for viz', () => {
+ const result = transformProps(chartProps) as any;
+ expect(result.width).toBe(800);
+ expect(result.height).toBe(600);
+ expect(result.groupbyRows).toEqual(['row1', 'row2']);
+ expect(result.groupbyColumns).toEqual(['col1', 'col2']);
+ expect(result.metrics).toEqual(['metric1', 'metric2']);
+ expect(result.metricsLayout).toBe(MetricsLayoutEnum.COLUMNS);
+ expect(result.currencyFormat).toEqual({
Review Comment:
Fixed, typed as `QueryData` instead.
--
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]
