This is an automated email from the ASF dual-hosted git repository.
asoare pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 2b411b32ba0 fix(scatter): Fix ad-hoc metric for pointsize (#37669)
2b411b32ba0 is described below
commit 2b411b32ba07f8dd4b11ee23623de81e064ee1fb
Author: Alexandru Soare <[email protected]>
AuthorDate: Mon Feb 9 11:13:06 2026 +0200
fix(scatter): Fix ad-hoc metric for pointsize (#37669)
---
.../src/layers/Scatter/buildQuery.test.ts | 245 ++++++++++++++++++++-
.../src/layers/Scatter/buildQuery.ts | 44 ++--
.../src/layers/buildQueryUtils.ts | 4 -
3 files changed, 270 insertions(+), 23 deletions(-)
diff --git
a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.test.ts
b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.test.ts
index 830d2c26b58..007867764cc 100644
---
a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.test.ts
+++
b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.test.ts
@@ -212,20 +212,27 @@ test('Scatter buildQuery should handle js_columns', () =>
{
expect(query.columns).toContain('custom_col2');
});
-test('Scatter buildQuery should convert numeric metric value to string', () =>
{
+test('Scatter buildQuery should handle adhoc SQL metric for
point_radius_fixed', () => {
+ const adhocMetric = {
+ label: 'count(*) * 1.1',
+ expressionType: 'SQL' as const,
+ sqlExpression: 'count(*) * 1.1',
+ };
const formData: DeckScatterFormData = {
...baseFormData,
point_radius_fixed: {
type: 'metric',
- value: 123, // numeric metric (edge case)
+ value: adhocMetric,
},
};
const queryContext = buildQuery(formData);
const [query] = queryContext.queries;
- expect(query.metrics).toContain('123');
- expect(query.orderby).toEqual([['123', false]]);
+ // Should preserve full adhoc metric object (not just the label string)
+ expect(query.metrics).toContainEqual(adhocMetric);
+ // orderby should use the label string
+ expect(query.orderby).toEqual([['count(*) * 1.1', false]]);
});
test('Scatter buildQuery should set is_timeseries to false', () => {
@@ -310,3 +317,233 @@ test('Scatter buildQuery should deduplicate metrics when
radius metric already e
expect(query.metrics).toEqual(['COUNT(*)', 'AVG(price)']);
expect(query.metrics).toHaveLength(2);
});
+
+// Comprehensive point_radius_fixed tests to prevent regressions
+test('Scatter buildQuery should handle adhoc SIMPLE metric for
point_radius_fixed', () => {
+ const adhocMetric = {
+ label: 'AVG(population)',
+ expressionType: 'SIMPLE' as const,
+ column: { column_name: 'population' },
+ aggregate: 'AVG' as const,
+ };
+ const formData: DeckScatterFormData = {
+ ...baseFormData,
+ point_radius_fixed: {
+ type: 'metric',
+ value: adhocMetric,
+ },
+ };
+
+ const queryContext = buildQuery(formData);
+ const [query] = queryContext.queries;
+
+ // Should preserve full adhoc metric object
+ expect(query.metrics).toContainEqual(adhocMetric);
+ expect(query.orderby).toEqual([['AVG(population)', false]]);
+});
+
+test('Scatter buildQuery should deduplicate adhoc metrics with same label', ()
=> {
+ const adhocMetric = {
+ label: 'custom_count',
+ expressionType: 'SQL' as const,
+ sqlExpression: 'count(*) * 2',
+ };
+ const formData: DeckScatterFormData = {
+ ...baseFormData,
+ metrics: [adhocMetric], // Already has this metric
+ point_radius_fixed: {
+ type: 'metric',
+ value: adhocMetric, // Same metric for radius
+ },
+ };
+
+ const queryContext = buildQuery(formData);
+ const [query] = queryContext.queries;
+
+ // Should not duplicate the metric
+ expect(query.metrics).toHaveLength(1);
+ expect(query.metrics).toContainEqual(adhocMetric);
+});
+
+test('Scatter buildQuery should handle fixed type with string value
correctly', () => {
+ const formData: DeckScatterFormData = {
+ ...baseFormData,
+ point_radius_fixed: {
+ type: 'fix',
+ value: '2500',
+ },
+ };
+
+ const queryContext = buildQuery(formData);
+ const [query] = queryContext.queries;
+
+ // Fixed values should NOT be added to metrics
+ expect(query.metrics).toEqual([]);
+ expect(query.orderby).toEqual([]);
+});
+
+test('Scatter buildQuery should handle undefined value in metric type
gracefully', () => {
+ const formData: DeckScatterFormData = {
+ ...baseFormData,
+ point_radius_fixed: {
+ type: 'metric',
+ value: undefined,
+ },
+ };
+
+ const queryContext = buildQuery(formData);
+ const [query] = queryContext.queries;
+
+ // Should not add anything when value is undefined
+ expect(query.metrics).toEqual([]);
+ expect(query.orderby).toEqual([]);
+});
+
+test('Scatter buildQuery should preserve adhoc metric with custom label', ()
=> {
+ const adhocMetric = {
+ label: 'My Custom Metric',
+ expressionType: 'SQL' as const,
+ sqlExpression: 'SUM(revenue) / COUNT(*)',
+ hasCustomLabel: true,
+ };
+ const formData: DeckScatterFormData = {
+ ...baseFormData,
+ point_radius_fixed: {
+ type: 'metric',
+ value: adhocMetric,
+ },
+ };
+
+ const queryContext = buildQuery(formData);
+ const [query] = queryContext.queries;
+
+ // Should preserve full metric including hasCustomLabel
+ expect(query.metrics).toContainEqual(adhocMetric);
+ expect(query.orderby).toEqual([['My Custom Metric', false]]);
+});
+
+test('Scatter buildQuery should handle point_radius_fixed with undefined
type', () => {
+ const formData: DeckScatterFormData = {
+ ...baseFormData,
+ point_radius_fixed: {
+ type: undefined,
+ value: '1000',
+ },
+ };
+
+ const queryContext = buildQuery(formData);
+ const [query] = queryContext.queries;
+
+ // Without explicit type, should not be treated as metric
+ expect(query.metrics).toEqual([]);
+ expect(query.orderby).toEqual([]);
+});
+
+test('Scatter buildQuery should handle empty point_radius_fixed object', () =>
{
+ const formData: DeckScatterFormData = {
+ ...baseFormData,
+ point_radius_fixed: {},
+ };
+
+ const queryContext = buildQuery(formData);
+ const [query] = queryContext.queries;
+
+ // Empty object should not add any metrics
+ expect(query.metrics).toEqual([]);
+ expect(query.orderby).toEqual([]);
+});
+
+test('Scatter buildQuery should handle fix type with undefined value', () => {
+ const formData: DeckScatterFormData = {
+ ...baseFormData,
+ point_radius_fixed: {
+ type: 'fix',
+ value: undefined,
+ },
+ };
+
+ const queryContext = buildQuery(formData);
+ const [query] = queryContext.queries;
+
+ // Fix type with no value should not affect metrics
+ expect(query.metrics).toEqual([]);
+ expect(query.orderby).toEqual([]);
+});
+
+test('Scatter buildQuery should add saved metric radius to existing adhoc
metrics', () => {
+ const existingAdhocMetric = {
+ label: 'SUM(sales)',
+ expressionType: 'SQL' as const,
+ sqlExpression: 'SUM(sales)',
+ };
+ const formData: DeckScatterFormData = {
+ ...baseFormData,
+ metrics: [existingAdhocMetric],
+ point_radius_fixed: {
+ type: 'metric',
+ value: 'AVG(radius)',
+ },
+ };
+
+ const queryContext = buildQuery(formData);
+ const [query] = queryContext.queries;
+
+ // Should have both the existing adhoc metric and the new saved metric
+ expect(query.metrics).toContainEqual(existingAdhocMetric);
+ expect(query.metrics).toContain('AVG(radius)');
+ expect(query.metrics).toHaveLength(2);
+});
+
+test('Scatter buildQuery should add adhoc metric radius to existing saved
metrics', () => {
+ const radiusAdhocMetric = {
+ label: 'Custom Radius',
+ expressionType: 'SQL' as const,
+ sqlExpression: 'AVG(size) * 100',
+ };
+ const formData: DeckScatterFormData = {
+ ...baseFormData,
+ metrics: ['COUNT(*)', 'SUM(value)'],
+ point_radius_fixed: {
+ type: 'metric',
+ value: radiusAdhocMetric,
+ },
+ };
+
+ const queryContext = buildQuery(formData);
+ const [query] = queryContext.queries;
+
+ // Should have existing saved metrics plus the new adhoc metric
+ expect(query.metrics).toContain('COUNT(*)');
+ expect(query.metrics).toContain('SUM(value)');
+ expect(query.metrics).toContainEqual(radiusAdhocMetric);
+ expect(query.metrics).toHaveLength(3);
+});
+
+test('Scatter buildQuery should handle legacy string format for
point_radius_fixed', () => {
+ const formData: DeckScatterFormData = {
+ ...baseFormData,
+ point_radius_fixed: 'COUNT(*)',
+ };
+
+ const queryContext = buildQuery(formData);
+ const [query] = queryContext.queries;
+
+ // Legacy string format should be treated as a metric
+ expect(query.metrics).toContain('COUNT(*)');
+ expect(query.orderby).toEqual([['COUNT(*)', false]]);
+});
+
+test('Scatter buildQuery should deduplicate legacy string metric with existing
metrics', () => {
+ const formData: DeckScatterFormData = {
+ ...baseFormData,
+ metrics: ['COUNT(*)', 'SUM(value)'],
+ point_radius_fixed: 'COUNT(*)',
+ };
+
+ const queryContext = buildQuery(formData);
+ const [query] = queryContext.queries;
+
+ // Should not duplicate COUNT(*)
+ expect(query.metrics).toEqual(['COUNT(*)', 'SUM(value)']);
+ expect(query.metrics).toHaveLength(2);
+});
diff --git
a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.ts
b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.ts
index e1976c6544f..231e2bf1f79 100644
---
a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.ts
+++
b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.ts
@@ -19,6 +19,8 @@
import {
buildQueryContext,
ensureIsArray,
+ getMetricLabel,
+ QueryFormMetric,
QueryFormOrderBy,
SqlaFormData,
QueryFormColumn,
@@ -31,17 +33,19 @@ import {
} from '../spatialUtils';
import {
addJsColumnsToColumns,
- processMetricsArray,
addTooltipColumnsToQuery,
} from '../buildQueryUtils';
-import { isMetricValue, extractMetricKey } from '../utils/metricUtils';
+import { isMetricValue } from '../utils/metricUtils';
export interface DeckScatterFormData
extends Omit<SpatialFormData, 'color_picker'>, SqlaFormData {
- point_radius_fixed?: {
- type?: 'fix' | 'metric';
- value?: string | number;
- };
+ // Can be a string (legacy format) or an object with type and value
+ point_radius_fixed?:
+ | string // Legacy format: metric name directly
+ | {
+ type?: 'fix' | 'metric';
+ value?: QueryFormMetric | number;
+ };
multiplier?: number;
point_unit?: string;
min_radius?: number;
@@ -82,26 +86,36 @@ export default function buildQuery(formData:
DeckScatterFormData) {
// Only add metric if point_radius_fixed is a metric type
const isMetric = isMetricValue(point_radius_fixed);
- const metricValue = isMetric
- ? extractMetricKey(point_radius_fixed?.value)
- : null;
+ // Extract metric value: legacy string format or object with metric value
+ const rawValue =
+ typeof point_radius_fixed === 'string'
+ ? point_radius_fixed
+ : point_radius_fixed?.value;
+ const metricValue: QueryFormMetric | null =
+ isMetric && rawValue !== undefined && typeof rawValue !== 'number'
+ ? (rawValue as QueryFormMetric)
+ : null;
// Preserve existing metrics and only add radius metric if it's
metric-based
const existingMetrics = baseQueryObject.metrics || [];
- const radiusMetrics = processMetricsArray(
- metricValue ? [metricValue] : [],
+ // Deduplicate metrics using getMetricLabel for comparison
+ const existingLabels = new Set(
+ existingMetrics.map(m => getMetricLabel(m)),
);
- // Deduplicate metrics to avoid adding the same metric twice
- const metricsSet = new Set([...existingMetrics, ...radiusMetrics]);
- const metrics = Array.from(metricsSet);
+ const metrics: QueryFormMetric[] =
+ metricValue && !existingLabels.has(getMetricLabel(metricValue))
+ ? [...existingMetrics, metricValue]
+ : existingMetrics;
+
const filters = addSpatialNullFilters(
spatial,
ensureIsArray(baseQueryObject.filters || []),
);
+ // orderby needs string label, not the full metric object
const orderby =
isMetric && metricValue
- ? ([[metricValue, false]] as QueryFormOrderBy[])
+ ? ([[getMetricLabel(metricValue), false]] as QueryFormOrderBy[])
: (baseQueryObject.orderby as QueryFormOrderBy[]) || [];
return [
diff --git
a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/buildQueryUtils.ts
b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/buildQueryUtils.ts
index f61a71ede7c..7a3c20d2422 100644
---
a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/buildQueryUtils.ts
+++
b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/buildQueryUtils.ts
@@ -92,10 +92,6 @@ export function addColumnsIfNotExists(
return result;
}
-export function processMetricsArray(metrics: (string | undefined)[]): string[]
{
- return metrics.filter((metric): metric is string => Boolean(metric));
-}
-
export function extractTooltipColumns(tooltipContents?: unknown[]): string[] {
if (!Array.isArray(tooltipContents) || !tooltipContents.length) {
return [];
