fix: SelectControl default sort numeric choices by value

superset-frontend/src/explore/components/controls/SelectControl.jsx

@@ -87,6 +87,44 @@ const defaultProps = {
   valueKey: 'value',
 };
 
+const numberComparator = (a, b) => a.value - b.value;
+
+export const areAllValuesNumbers = (items, valueKey = 'value') => {
+  if (!items || items.length === 0) {
+    return false;
+  }
+  return items.every(item => {
+    if (Array.isArray(item)) {
+      const [value] = item;
+      return typeof value === 'number';
+    }
+    if (typeof item === 'object' && item !== null) {
+      return typeof item[valueKey] === 'number';
+    }
+    return typeof item === 'number';
+  });
+};
+
+export const getSortComparator = (
+  choices,
+  options,
+  valueKey,
+  explicitComparator,
+) => {
+  if (explicitComparator) {
+    return explicitComparator;
+  }
+
+  if (
+    (options && areAllValuesNumbers(options, valueKey)) ||
+    (choices && areAllValuesNumbers(choices, valueKey))
+  ) {
+    return numberComparator;
+  }
+
+  return undefined;
+};
+
 export const innerGetOptions = props => {
   const { choices, optionRenderer, valueKey } = props;
   let options = [];
@@ -252,7 +290,12 @@ export default class SelectControl extends PureComponent {
       onDeselect,
       options: this.state.options,
       placeholder,
-      sortComparator: this.props.sortComparator,
+      sortComparator: getSortComparator(
+        this.props.choices,
+        this.props.options,
+        this.props.valueKey,
+        this.props.sortComparator,
+      ),
       value: getValue(),
       tokenSeparators,
       notFoundContent,

superset-frontend/src/explore/components/controls/SelectControl.test.jsx

@@ -27,6 +27,8 @@ import {
 } from 'spec/helpers/testing-library';
 import SelectControl, {
   innerGetOptions,
+  areAllValuesNumbers,
+  getSortComparator,
 } from 'src/explore/components/controls/SelectControl';
 
 const defaultProps = {
@@ -235,4 +237,210 @@ describe('SelectControl', () => {
       expect(innerGetOptions(defaultProps)).toEqual(options);
     });
   });
+
+  describe('areAllValuesNumbers', () => {
+    it('returns true when all values are numbers (array format)', () => {
+      const items = [
+        [1, 'One'],
+        [2, 'Two'],
+        [3, 'Three'],
+      ];
+      expect(areAllValuesNumbers(items)).toBe(true);
+    });
+
+    it('returns false when some values are not numbers (array format)', () => {
+      const items = [
+        [1, 'One'],
+        ['two', 'Two'],
+        [3, 'Three'],
+      ];
+      expect(areAllValuesNumbers(items)).toBe(false);
+    });
+
+    it('returns true when all values are numbers (object format)', () => {
+      const items = [
+        { value: 1, label: 'One' },
+        { value: 2, label: 'Two' },
+        { value: 3, label: 'Three' },
+      ];
+      expect(areAllValuesNumbers(items)).toBe(true);
+    });
+
+    it('returns false when some values are not numbers (object format)', () => {
+      const items = [
+        { value: 1, label: 'One' },
+        { value: 'two', label: 'Two' },
+        { value: 3, label: 'Three' },
+      ];
+      expect(areAllValuesNumbers(items)).toBe(false);
+    });
+
+    it('returns true when all values are numbers (primitive format)', () => {
+      const items = [1, 2, 3];
+      expect(areAllValuesNumbers(items)).toBe(true);
+    });
+
+    it('returns false when some values are not numbers (primitive format)', () => {
+      const items = [1, 'two', 3];
+      expect(areAllValuesNumbers(items)).toBe(false);
+    });
+
+    it('works with custom valueKey', () => {
+      const items = [
+        { id: 1, label: 'One' },
+        { id: 2, label: 'Two' },
+        { id: 3, label: 'Three' },
+      ];
+      expect(areAllValuesNumbers(items, 'id')).toBe(true);
+    });
+
+    it('returns false for empty items', () => {
+      expect(areAllValuesNumbers([])).toBe(false);
+      expect(areAllValuesNumbers(null)).toBe(false);
+      expect(areAllValuesNumbers(undefined)).toBe(false);
+    });
+  });
+
+  describe('getSortComparator', () => {
+    const mockExplicitComparator = (a, b) => a.label.localeCompare(b.label);
+
+    it('returns explicit comparator when provided', () => {
+      const choices = [
+        [1, 'One'],
+        [2, 'Two'],
+      ];
+      const result = getSortComparator(
+        choices,
+        null,
+        'value',
+        mockExplicitComparator,
+      );
+      expect(result).toBe(mockExplicitComparator);
+    });
+
+    it('returns number comparator for numeric choices', () => {
+      const choices = [
+        [1, 'One'],
+        [2, 'Two'],
+      ];
+      const result = getSortComparator(choices, null, 'value', null);
+      expect(typeof result).toBe('function');
+      expect(result).not.toBe(mockExplicitComparator);
+    });
+
+    it('returns number comparator for numeric options', () => {
+      const options = [
+        { value: 1, label: 'One' },
+        { value: 2, label: 'Two' },
+      ];
+      const result = getSortComparator(null, options, 'value', null);
+      expect(typeof result).toBe('function');
+      expect(result).not.toBe(mockExplicitComparator);
+    });
+
+    it('prioritizes options over choices when both are numeric', () => {
+      const choices = [
+        [1, 'One'],
+        [2, 'Two'],
+      ];
+      const options = [
+        { value: 3, label: 'Three' },
+        { value: 4, label: 'Four' },
+      ];
+      const result = getSortComparator(choices, options, 'value', null);
+      expect(typeof result).toBe('function');
+    });
+
+    it('returns undefined for non-numeric choices', () => {
+      const choices = [
+        ['one', 'One'],
+        ['two', 'Two'],
+      ];
+      const result = getSortComparator(choices, null, 'value', null);
+      expect(result).toBeUndefined();
+    });
+
+    it('returns undefined for non-numeric options', () => {
+      const options = [
+        { value: 'one', label: 'One' },
+        { value: 'two', label: 'Two' },
+      ];
+      const result = getSortComparator(null, options, 'value', null);
+      expect(result).toBeUndefined();
+    });
+
+    it('returns undefined when no choices or options provided', () => {
+      const result = getSortComparator(null, null, 'value', null);
+      expect(result).toBeUndefined();
+    });
+  });
+
+  describe('numeric sorting integration', () => {
+    it('applies numeric sorting to choices automatically', () => {
+      const numericChoices = [
+        [3, 'Three'],
+        [1, 'One'],
+        [2, 'Two'],
+      ];
+      renderSelectControl({ choices: numericChoices });
+
+      // The SelectControl should receive a sortComparator for numeric values
+      // This is tested by verifying the component renders without errors
+      const selectorWrapper = screen.getByLabelText('Row Limit', {
+        selector: 'div',
+      });
+      expect(selectorWrapper).toBeInTheDocument();
+    });
+
+    it('applies numeric sorting to options automatically', () => {
+      const numericOptions = [
+        { value: 3, label: 'Three' },
+        { value: 1, label: 'One' },
+        { value: 2, label: 'Two' },
+      ];
+      renderSelectControl({ options: numericOptions, choices: undefined });
+
+      // The SelectControl should receive a sortComparator for numeric values
+      const selectorWrapper = screen.getByLabelText('Row Limit', {
+        selector: 'div',
+      });
+      expect(selectorWrapper).toBeInTheDocument();
+    });
+
+    it('does not apply numeric sorting to mixed-type choices', () => {
+      const mixedChoices = [
+        [1, 'One'],
+        ['two', 'Two'],
+        [3, 'Three'],
+      ];
+      renderSelectControl({ choices: mixedChoices });
+
+      // Should render without errors and not apply numeric sorting
+      const selectorWrapper = screen.getByLabelText('Row Limit', {
+        selector: 'div',
+      });
+      expect(selectorWrapper).toBeInTheDocument();
+    });
+
+    it('respects explicit sortComparator over automatic numeric sorting', () => {
+      const numericChoices = [
+        [3, 'Three'],
+        [1, 'One'],
+        [2, 'Two'],
+      ];
+      const explicitComparator = jest.fn((a, b) =>
+        a.label.localeCompare(b.label),
+      );
+
+      renderSelectControl({
+        choices: numericChoices,
+        sortComparator: explicitComparator,
+      });
+
+      const selectorWrapper = screen.getByLabelText('Row Limit', {
+        selector: 'div',
+      });
+      expect(selectorWrapper).toBeInTheDocument();
+    });
+  });
 });