rusackas commented on code in PR #44657:
URL: https://github.com/apache/superset/pull/44657#discussion_r4143709960
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts:
##########
@@ -1223,7 +1474,137 @@ class PivotData {
return this.rowKeys;
}
+ /**
+ * Result aggregation (see resultAggregation.ts): unlike `processRecord`
+ * below, which places each DB-precomputed record into exactly one rollup
+ * slot, every leaf record here is fed directly to every scope it
+ * contributes to -- one cell, one subtotal per enabled row/column depth,
+ * and the grand total -- so each aggregator reduces real original results,
+ * never another aggregator's already-computed value.
+ */
+ processResultRecord(record: PivotRecord): void {
+ const rows = this.props.rows as string[];
+ const cols = this.props.cols as string[];
+ const rowKey = rows.map(key =>
+ String(key in record ? record[key] : 'null'),
+ );
+ const colKey = cols.map(key =>
+ String(key in record ? record[key] : 'null'),
+ );
+ // Depth 0 is the fully collapsed (grand total/opposite-axis) scope;
+ // depth === length is the leaf; anything between is a subtotal, included
+ // only when that axis's subtotals are enabled. Computed before the
+ // metric-scope block below so
`rowGroupMetricTotals`/`colGroupMetricTotals`
+ // can be recorded at every depth a subtotal denominator might need, not
+ // just the leaf.
+ const rowDepths = [
+ 0,
+ ...rows
+ .map((_, i) => i + 1)
+ .filter(depth => depth === rows.length || this.subtotals.rowEnabled),
+ ];
+ const colDepths = [
+ 0,
+ ...cols
+ .map((_, i) => i + 1)
+ .filter(depth => depth === cols.length || this.subtotals.colEnabled),
+ ];
+ // A per-metric total (see `rowMetricTotals`/`colMetricTotals`), needed by
+ // "... as Fraction of ..." result aggregations independently of whether
+ // the corresponding subtotal is enabled, and of where the Metric
+ // pseudo-dimension sits in `rows`/`cols` (`combineMetric` can place it
+ // first or last): keyed purely by the metric's own value, not by depth,
+ // so it doesn't matter which position it collapses from.
+ const metricDim = record.__metricKey as unknown as string | undefined;
+ if (metricDim) {
+ const colMetricIndex = cols.indexOf(metricDim);
+ if (colMetricIndex !== -1) {
+ const metricValue = colKey[colMetricIndex];
+ this.colMetricTotals[metricValue] ??= this.getFormattedAggregator(
+ record,
+ )(this, [], [metricValue]);
+ this.colMetricTotals[metricValue].push(record);
+
+ // Row+metric scope: this record's own row, just this metric, across
+ // every column that shares it -- the 'row' fraction type's
+ // denominator when Metric sits on columns. Independent of
+ // `subtotals.colEnabled`, unlike the depth-gated tree. Recorded at
+ // every enabled row depth (not just the leaf) so a row subtotal's
+ // own (shorter) prefix key finds a denominator too, instead of only
+ // the full leaf-level row ever getting an entry.
+ rowDepths.forEach(ri => {
+ const rowPrefix = rowKey.slice(0, ri);
+ const flatRk = flatKey(rowPrefix);
+ this.rowGroupMetricTotals[flatRk] ??= Object.create(null);
+ this.rowGroupMetricTotals[flatRk][metricValue] ??=
+ this.getFormattedAggregator(record)(this, rowPrefix,
[metricValue]);
+ this.rowGroupMetricTotals[flatRk][metricValue].push(record);
+ });
+ }
+ const rowMetricIndex = rows.indexOf(metricDim);
+ if (rowMetricIndex !== -1) {
+ const metricValue = rowKey[rowMetricIndex];
+ this.rowMetricTotals[metricValue] ??= this.getFormattedAggregator(
+ record,
+ )(this, [metricValue], []);
+ this.rowMetricTotals[metricValue].push(record);
+
+ // Col+metric scope: the mirror of the above for the 'col' fraction
+ // type when Metric sits on rows instead, recorded at every enabled
+ // column depth for the same subtotal-prefix reason.
+ colDepths.forEach(ci => {
+ const colPrefix = colKey.slice(0, ci);
+ const flatCk = flatKey(colPrefix);
+ this.colGroupMetricTotals[flatCk] ??= Object.create(null);
+ this.colGroupMetricTotals[flatCk][metricValue] ??=
+ this.getFormattedAggregator(record)(this, [metricValue],
colPrefix);
+ this.colGroupMetricTotals[flatCk][metricValue].push(record);
+ });
+ }
+ }
+ rowDepths.forEach(ri =>
+ colDepths.forEach(ci => {
+ if (ri === 0 && ci === 0) {
+ this.allTotal.push(record);
+ return;
+ }
+ const r = rowKey.slice(0, ri);
+ const c = colKey.slice(0, ci);
+ const rk = flatKey(r);
+ const ck = flatKey(c);
+ let target: Record<string, Aggregator>;
+ let key: string;
+ if (ci === 0) {
+ target = this.rowTotals;
+ key = rk;
+ if (!target[key]) this.rowKeys.push(r);
+ } else if (ri === 0) {
+ target = this.colTotals;
+ key = ck;
+ if (!target[key]) this.colKeys.push(c);
+ } else {
+ this.tree[rk] ??= {};
+ target = this.tree[rk];
+ key = ck;
+ }
+ target[key] ??= this.getFormattedAggregator(
Review Comment:
Good catch. `processResultRecord` was feeding every scope through the
reducer, leaves included, since it never distinguished a leaf from a rollup.
Leaf cells now keep the DB value verbatim and only subtotals/totals reduce.
Pushed, plus a regression test.
--
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]
