Aleem89 commented on code in PR #45408:
URL: https://github.com/apache/superset/pull/45408#discussion_r4304052560
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/utilities.ts:
##########
@@ -36,13 +37,22 @@ const ADDITIVE_AGGREGATES = new Set(['SUM', 'COUNT', 'MIN',
'MAX']);
* Whether a metric's total can be derived additively from per-group results.
* Conservative: only SIMPLE metrics with a known additive aggregate qualify;
* saved-metric references (strings) and SQL/adhoc metrics are treated as
- * non-additive because their aggregate cannot be determined from form data.
+ * non-additive because their aggregate cannot be determined from form data,
+ * and MIN/MAX only qualify on a column known to be numeric (see below).
*/
export function isAdditiveMetric(metric: QueryFormMetric): boolean {
if (typeof metric === 'string' || metric.expressionType !== 'SIMPLE') {
return false;
}
- return !!metric.aggregate && ADDITIVE_AGGREGATES.has(metric.aggregate);
+ if (!metric.aggregate || !ADDITIVE_AGGREGATES.has(metric.aggregate)) {
+ return false;
+ }
+ // The fast-path reduces values with Number(), which would blank a MIN/MAX
+ // over a text or temporal column, so only numeric columns may take it.
+ if (metric.aggregate === 'MIN' || metric.aggregate === 'MAX') {
+ return metric.column?.type_generic === GenericDataType.Numeric;
+ }
+ return true;
Review Comment:
Good call, that makes sense to me. Renamed it to
`isClientAggregatableMetric`, and its companion `allMetricsAdditive` (the one
buildQuery/transformProps call) to `allMetricsClientAggregatable` so the pair
matches. Let me know if that looks good or if we need to adjust the scope
either way.
--
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]
