fix(big number with trendline): running 2 identical queries for no good reason (#34296)

This commit is contained in:
Maxime Beauchemin
2025-07-29 13:07:28 -07:00
committed by GitHub
parent 8de8f95a3c
commit 0964a8bb7a
11 changed files with 305 additions and 213 deletions

View File

@@ -49,38 +49,53 @@ describe('BigNumberWithTrendline buildQuery', () => {
aggregation: null,
};
it('creates raw metric query when aggregation is null', () => {
const queryContext = buildQuery({ ...baseFormData });
it('creates raw metric query when aggregation is "raw"', () => {
const queryContext = buildQuery({ ...baseFormData, aggregation: 'raw' });
const bigNumberQuery = queryContext.queries[1];
expect(bigNumberQuery.post_processing).toEqual([{ operation: 'pivot' }]);
expect(bigNumberQuery.is_timeseries).toBe(true);
expect(bigNumberQuery.post_processing).toEqual([]);
expect(bigNumberQuery.is_timeseries).toBe(false);
expect(bigNumberQuery.columns).toEqual([]);
});
it('adds aggregation operator when aggregation is "sum"', () => {
it('returns single query for aggregation methods that can be computed client-side', () => {
const queryContext = buildQuery({ ...baseFormData, aggregation: 'sum' });
const bigNumberQuery = queryContext.queries[1];
expect(bigNumberQuery.post_processing).toEqual([
expect(queryContext.queries.length).toBe(1);
expect(queryContext.queries[0].post_processing).toEqual([
{ operation: 'pivot' },
{ operation: 'aggregation', options: { operator: 'sum' } },
{ operation: 'rolling' },
{ operation: 'resample' },
{ operation: 'flatten' },
]);
expect(bigNumberQuery.is_timeseries).toBe(true);
});
it('skips aggregation when aggregation is LAST_VALUE', () => {
it('returns single query for LAST_VALUE aggregation', () => {
const queryContext = buildQuery({
...baseFormData,
aggregation: 'LAST_VALUE',
});
const bigNumberQuery = queryContext.queries[1];
expect(bigNumberQuery.post_processing).toEqual([{ operation: 'pivot' }]);
expect(bigNumberQuery.is_timeseries).toBe(true);
expect(queryContext.queries.length).toBe(1);
expect(queryContext.queries[0].post_processing).toEqual([
{ operation: 'pivot' },
{ operation: 'rolling' },
{ operation: 'resample' },
{ operation: 'flatten' },
]);
});
it('always returns two queries', () => {
const queryContext = buildQuery({ ...baseFormData });
it('returns two queries only for raw aggregation', () => {
const queryContext = buildQuery({ ...baseFormData, aggregation: 'raw' });
expect(queryContext.queries.length).toBe(2);
const queryContextLastValue = buildQuery({
...baseFormData,
aggregation: 'LAST_VALUE',
});
expect(queryContextLastValue.queries.length).toBe(1);
const queryContextSum = buildQuery({ ...baseFormData, aggregation: 'sum' });
expect(queryContextSum.queries.length).toBe(1);
});
});

View File

@@ -39,28 +39,37 @@ export default function buildQuery(formData: QueryFormData) {
? ensureIsArray(getXAxisColumn(formData))
: [];
return buildQueryContext(formData, baseQueryObject => [
{
...baseQueryObject,
columns: [...timeColumn],
...(timeColumn.length ? {} : { is_timeseries: true }),
post_processing: [
pivotOperator(formData, baseQueryObject),
rollingWindowOperator(formData, baseQueryObject),
resampleOperator(formData, baseQueryObject),
flattenOperator(formData, baseQueryObject),
],
},
{
...baseQueryObject,
columns: [...(isRawMetric ? [] : timeColumn)],
is_timeseries: !isRawMetric,
post_processing: isRawMetric
? []
: [
pivotOperator(formData, baseQueryObject),
aggregationOperator(formData, baseQueryObject),
],
},
]);
return buildQueryContext(formData, baseQueryObject => {
const queries = [
{
...baseQueryObject,
columns: [...timeColumn],
...(timeColumn.length ? {} : { is_timeseries: true }),
post_processing: [
pivotOperator(formData, baseQueryObject),
rollingWindowOperator(formData, baseQueryObject),
resampleOperator(formData, baseQueryObject),
flattenOperator(formData, baseQueryObject),
].filter(Boolean),
},
];
// Only add second query for raw metrics which need different query structure
// All other aggregations (sum, mean, min, max, median, LAST_VALUE) can be computed client-side from trendline data
if (formData.aggregation === 'raw') {
queries.push({
...baseQueryObject,
columns: [...(isRawMetric ? [] : timeColumn)],
is_timeseries: !isRawMetric,
post_processing: isRawMetric
? []
: ([
pivotOperator(formData, baseQueryObject),
aggregationOperator(formData, baseQueryObject),
].filter(Boolean) as any[]),
});
}
return queries;
});
}

View File

@@ -20,6 +20,41 @@ import { GenericDataType } from '@superset-ui/core';
import transformProps from './transformProps';
import { BigNumberWithTrendlineChartProps, BigNumberDatum } from '../types';
// Mock chart-controls to avoid styled-components issues in Jest
jest.mock('@superset-ui/chart-controls', () => ({
aggregationChoices: {
raw: {
label: 'Force server-side aggregation',
compute: (data: number[]) => data[0] ?? null,
},
LAST_VALUE: {
label: 'Last Value',
compute: (data: number[]) => data[0] ?? null,
},
sum: {
label: 'Total (Sum)',
compute: (data: number[]) => data.reduce((a, b) => a + b, 0),
},
mean: {
label: 'Average (Mean)',
compute: (data: number[]) =>
data.reduce((a, b) => a + b, 0) / data.length,
},
min: { label: 'Minimum', compute: (data: number[]) => Math.min(...data) },
max: { label: 'Maximum', compute: (data: number[]) => Math.max(...data) },
median: {
label: 'Median',
compute: (data: number[]) => {
const sorted = [...data].sort((a, b) => a - b);
const mid = Math.floor(sorted.length / 2);
return sorted.length % 2 === 0
? (sorted[mid - 1] + sorted[mid]) / 2
: sorted[mid];
},
},
},
}));
jest.mock('@superset-ui/core', () => ({
GenericDataType: { Temporal: 2, String: 1 },
extractTimegrain: jest.fn(() => 'P1D'),
@@ -218,7 +253,7 @@ describe('BigNumberWithTrendline transformProps', () => {
coltypes: ['NUMERIC'],
},
],
formData: { ...baseFormData, aggregation: 'SUM' },
formData: { ...baseFormData, aggregation: 'sum' },
rawFormData: baseRawFormData,
hooks: baseHooks,
datasource: baseDatasource,

View File

@@ -29,6 +29,7 @@ import {
tooltipHtml,
} from '@superset-ui/core';
import { EChartsCoreOption, graphic } from 'echarts/core';
import { aggregationChoices } from '@superset-ui/chart-controls';
import {
BigNumberVizProps,
BigNumberDatum,
@@ -43,6 +44,31 @@ const formatPercentChange = getNumberFormatter(
NumberFormats.PERCENT_SIGNED_1_POINT,
);
// Client-side aggregation function using shared aggregationChoices
function computeClientSideAggregation(
data: [number | null, number | null][],
aggregation: string | undefined | null,
): number | null {
if (!data.length) return null;
// Find the aggregation method, handling case variations
const methodKey = Object.keys(aggregationChoices).find(
key => key.toLowerCase() === (aggregation || '').toLowerCase(),
);
// Use the compute method from aggregationChoices, fallback to LAST_VALUE
const selectedMethod = methodKey
? aggregationChoices[methodKey as keyof typeof aggregationChoices]
: aggregationChoices.LAST_VALUE;
// Extract values from tuple array and filter out nulls
const values = data
.map(([, value]) => value)
.filter((v): v is number => v !== null);
return selectedMethod.compute(values);
}
export default function transformProps(
chartProps: BigNumberWithTrendlineChartProps,
): BigNumberVizProps {
@@ -126,27 +152,33 @@ export default function transformProps(
// sort in time descending order
.sort((a, b) => (a[0] !== null && b[0] !== null ? b[0] - a[0] : 0));
}
if (hasAggregatedData && aggregatedData) {
if (
aggregatedData[metricName] !== null &&
aggregatedData[metricName] !== undefined
) {
bigNumber = aggregatedData[metricName];
} else {
const metricKeys = Object.keys(aggregatedData).filter(
key =>
key !== xAxisLabel &&
aggregatedData[key] !== null &&
typeof aggregatedData[key] === 'number',
);
bigNumber = metricKeys.length > 0 ? aggregatedData[metricKeys[0]] : null;
}
timestamp = sortedData.length > 0 ? sortedData[0][0] : null;
} else if (sortedData.length > 0) {
bigNumber = sortedData[0][1];
if (sortedData.length > 0) {
timestamp = sortedData[0][0];
// Raw aggregation uses server-side data, all others use client-side
if (aggregation === 'raw' && hasAggregatedData && aggregatedData) {
// Use server-side aggregation for raw
if (
aggregatedData[metricName] !== null &&
aggregatedData[metricName] !== undefined
) {
bigNumber = aggregatedData[metricName];
} else {
const metricKeys = Object.keys(aggregatedData).filter(
key =>
key !== xAxisLabel &&
aggregatedData[key] !== null &&
typeof aggregatedData[key] === 'number',
);
bigNumber =
metricKeys.length > 0 ? aggregatedData[metricKeys[0]] : null;
}
} else {
// Use client-side aggregation for all other methods
bigNumber = computeClientSideAggregation(sortedData, aggregation);
}
// Handle null bigNumber case
if (bigNumber === null) {
bigNumberFallback = sortedData.find(d => d[1] !== null);
bigNumber = bigNumberFallback ? bigNumberFallback[1] : null;