rusackas commented on code in PR #41184:
URL: https://github.com/apache/superset/pull/41184#discussion_r3557744328


##########
superset/common/query_object.py:
##########
@@ -157,6 +158,7 @@ def __init__(  # pylint: disable=too-many-locals, 
too-many-arguments
         self.series_limit = series_limit
         self.series_limit_metric = series_limit_metric
         self.group_others_when_limit_reached = group_others_when_limit_reached
+        self.grouping_sets = grouping_sets or []

Review Comment:
   Good call, added `grouping_sets: list[list[str]]` to the class attrs 
alongside the others.



##########
superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/buildQuery.test.ts:
##########
@@ -56,9 +55,41 @@ const formData: PivotTableQueryFormData = {
   currencyFormat: { symbol: 'USD', symbolPosition: 'prefix' },
 };
 
-test('should build groupby with series in form data', () => {
+test('additive metrics use the fast-path: a single full-detail query', () => {
+  const { queries } = buildQuery({
+    ...formData,
+    metrics: [
+      {
+        expressionType: 'SIMPLE',
+        aggregate: 'SUM',
+        column: { column_name: 'num' },
+        label: 'sum_num',
+      },
+    ] as any,
+  });
+  expect(queries).toHaveLength(1);
+  // The single leaf query carries all dimensions (2 rows + 2 cols).
+  expect(queries[0].columns).toHaveLength(4);
+});
+
+test('non-additive metrics emit a single GROUPING SETS query with all levels', 
() => {
+  // 2 row dims x 2 col dims -> (2+1) x (2+1) = 9 rollup levels, carried as
+  // grouping_sets on a single query (saved-metric strings are non-additive).
   const queryContext = buildQuery(formData);
+  expect(queryContext.queries).toHaveLength(1);
   const [query] = queryContext.queries;
+  // The single query selects the full set of dimensions ...
+  expect(query.columns).toHaveLength(4);
+  // ... and requests every rollup level via grouping_sets (grand total = []).
+  expect((query as any).grouping_sets).toHaveLength(9);
+  expect((query as any).grouping_sets[0]).toEqual([]);
+  expect((query as any).grouping_sets[8]).toHaveLength(4);
+});
+
+test('should build groupby with series in form data', () => {
+  const queryContext = buildQuery(formData);
+  // Multi-query rollup: the full-detail level is the last query (grand total 
is first).
+  const query = queryContext.queries[queryContext.queries.length - 1];

Review Comment:
   Yep, stale wording from an earlier multi-query iteration. Updated the 
comment to reflect the single GROUPING SETS query.



-- 
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]

Reply via email to