You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/08/05 06:06:46 UTC
(superset) branch master updated: fix(filters): sort numeric filter values numerically, not lexicographically (#36775) (#42596)
This is an automated email from the ASF dual-hosted git repository.
rusackas 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 7324225c777 fix(filters): sort numeric filter values numerically, not lexicographically (#36775) (#42596)
7324225c777 is described below
commit 7324225c77717a2d9c26e9a3e92941d540ab3d50
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Tue Aug 4 23:06:39 2026 -0700
fix(filters): sort numeric filter values numerically, not lexicographically (#36775) (#42596)
Co-authored-by: Claude Code <no...@anthropic.com>
---
.../src/components/Select/utils.test.ts | 46 +++++++
.../src/components/Select/utils.tsx | 14 +-
.../components/Select/SelectFilterPlugin.test.tsx | 151 +++++++++++++++++++++
.../components/Select/SelectFilterPlugin.tsx | 16 ++-
4 files changed, 221 insertions(+), 6 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/components/Select/utils.test.ts b/superset-frontend/packages/superset-ui-core/src/components/Select/utils.test.ts
index 3443bb00017..3d4c932b9d1 100644
--- a/superset-frontend/packages/superset-ui-core/src/components/Select/utils.test.ts
+++ b/superset-frontend/packages/superset-ui-core/src/components/Select/utils.test.ts
@@ -16,10 +16,12 @@
* specific language governing permissions and limitations
* under the License.
*/
+import type { LabeledValue as AntdLabeledValue } from 'antd/es/select';
import {
splitWithQuoteEscaping,
stripSurroundingQuotes,
makeQuoteAwareTokenizer,
+ propertyComparator,
} from './utils';
test('stripSurroundingQuotes removes matching surrounding double quotes', () => {
@@ -138,3 +140,47 @@ test('makeQuoteAwareTokenizer detects any separator from the list outside quotes
expect(tokenize('a\nb')).toEqual(['a', 'b']);
expect(tokenize('"a\nb"')).toEqual(['"a\nb"']);
});
+
+test('propertyComparator sorts string properties lexicographically', () => {
+ const compare = propertyComparator('label');
+ const ten = { label: '10' } as AntdLabeledValue;
+ const two = { label: '2' } as AntdLabeledValue;
+ const hundred = { label: '100' } as AntdLabeledValue;
+ expect([ten, two, hundred].sort(compare)).toEqual([ten, hundred, two]);
+});
+
+test('propertyComparator sorts number properties numerically', () => {
+ const compare = propertyComparator('value');
+ const ten = { value: 10 } as unknown as AntdLabeledValue;
+ const two = { value: 2 } as unknown as AntdLabeledValue;
+ const hundred = { value: 100 } as unknown as AntdLabeledValue;
+ expect([ten, two, hundred].sort(compare)).toEqual([two, ten, hundred]);
+});
+
+test('propertyComparator sorts bigint properties numerically, not lexicographically', () => {
+ const compare = propertyComparator('value');
+ const ten = { value: 10n } as unknown as AntdLabeledValue;
+ const two = { value: 2n } as unknown as AntdLabeledValue;
+ const hundred = { value: 100n } as unknown as AntdLabeledValue;
+ expect([ten, two, hundred].sort(compare)).toEqual([two, ten, hundred]);
+});
+
+test('propertyComparator sorts diverging-digit-length bigint values numerically', () => {
+ const compare = propertyComparator('value');
+ const a = { value: 10000000000000002n } as unknown as AntdLabeledValue;
+ const b = { value: 9000000000000001n } as unknown as AntdLabeledValue;
+ const c = { value: 10000000000000001n } as unknown as AntdLabeledValue;
+ expect([a, b, c].sort(compare)).toEqual([b, c, a]);
+});
+
+test('propertyComparator sorts mixed bigint and number properties numerically', () => {
+ const compare = propertyComparator('value');
+ const bigTen = { value: 10n } as unknown as AntdLabeledValue;
+ const two = { value: 2 } as unknown as AntdLabeledValue;
+ const bigHundred = { value: 100n } as unknown as AntdLabeledValue;
+ expect([bigTen, two, bigHundred].sort(compare)).toEqual([
+ two,
+ bigTen,
+ bigHundred,
+ ]);
+});
diff --git a/superset-frontend/packages/superset-ui-core/src/components/Select/utils.tsx b/superset-frontend/packages/superset-ui-core/src/components/Select/utils.tsx
index 4ae9ef93a75..9a6739b6db7 100644
--- a/superset-frontend/packages/superset-ui-core/src/components/Select/utils.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/components/Select/utils.tsx
@@ -79,7 +79,7 @@ export function hasOption(
/**
* It creates a comparator to check for a specific property.
- * Can be used with string and number property values.
+ * Can be used with string, number, and bigint property values.
* */
export const propertyComparator =
(property: string) => (a: AntdLabeledValue, b: AntdLabeledValue) => {
@@ -91,6 +91,18 @@ export const propertyComparator =
if (typeof propertyA === 'number' && typeof propertyB === 'number') {
return propertyA - propertyB;
}
+ // BIGINT columns can decode to native `bigint` values (see json-bigint
+ // parsing of large numeric values). Compare numerically rather than
+ // falling through to the string fallback below, which would sort them
+ // lexicographically (e.g. "10", "100", "2").
+ if (
+ (typeof propertyA === 'bigint' || typeof propertyA === 'number') &&
+ (typeof propertyB === 'bigint' || typeof propertyB === 'number')
+ ) {
+ if (propertyA < propertyB) return -1;
+ if (propertyA > propertyB) return 1;
+ return 0;
+ }
return String(propertyA).localeCompare(String(propertyB)); // fallback to string comparison
};
diff --git a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx
index 1db796a0dce..933d90cd011 100644
--- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx
+++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx
@@ -720,6 +720,157 @@ describe('SelectFilterPlugin', () => {
expect(options[2]).toHaveTextContent('beta');
});
+ test('sorts numeric filter values numerically, not lexicographically, when no sortMetric is specified', () => {
+ // Regression for #36775: numeric filter values were sorted as strings
+ // (localeCompare on the formatted label), producing "1, 10, 100, 2"
+ // instead of the expected "1, 2, 10, 100".
+ const testData = [{ age: 10 }, { age: 2 }, { age: 100 }];
+
+ const testProps = {
+ ...selectMultipleProps,
+ formData: {
+ ...selectMultipleProps.formData,
+ groupby: ['age'],
+ sortMetric: undefined,
+ sortAscending: true,
+ },
+ queriesData: [
+ {
+ rowcount: 3,
+ colnames: ['age'],
+ coltypes: [0],
+ data: testData,
+ applied_filters: [{ column: 'age' }],
+ rejected_filters: [],
+ },
+ ],
+ filterState: {
+ value: [],
+ label: '',
+ excludeFilterValues: true,
+ },
+ };
+
+ render(
+ // @ts-expect-error
+ <SelectFilterPlugin
+ // @ts-expect-error
+ {...transformProps(testProps)}
+ setDataMask={jest.fn()}
+ showOverflow={false}
+ />,
+ {
+ useRedux: true,
+ initialState: {
+ nativeFilters: {
+ filters: {
+ 'test-filter': {
+ name: 'Test Filter',
+ },
+ },
+ },
+ dataMask: {
+ 'test-filter': {
+ extraFormData: {},
+ filterState: {
+ value: [],
+ label: '',
+ excludeFilterValues: true,
+ },
+ },
+ },
+ },
+ },
+ );
+
+ const filterSelect = screen.getAllByRole('combobox')[0];
+ userEvent.click(filterSelect);
+
+ // Options should appear in ascending numeric order (2, 10, 100), not
+ // ascending lexicographic order of their formatted labels (10, 100, 2).
+ const options = screen.getAllByRole('option');
+ expect(options[0]).toHaveTextContent('2');
+ expect(options[1]).toHaveTextContent('10');
+ expect(options[2]).toHaveTextContent('100');
+ });
+
+ test('sorts BIGINT filter values numerically when values decode to native bigint', () => {
+ // BIGINT columns with 16+ digit values decode to native `bigint` (see
+ // json-bigint parsing of the chart data response), not `number`. Those
+ // values must still sort numerically rather than falling back to
+ // lexicographic string comparison ("10", "100", "2").
+ const testData = [
+ { age: 10000000000000000n },
+ { age: 2000000000000000n },
+ { age: 100000000000000000n },
+ ];
+
+ const testProps = {
+ ...selectMultipleProps,
+ formData: {
+ ...selectMultipleProps.formData,
+ groupby: ['age'],
+ sortMetric: undefined,
+ sortAscending: true,
+ },
+ queriesData: [
+ {
+ rowcount: 3,
+ colnames: ['age'],
+ coltypes: [0],
+ data: testData,
+ applied_filters: [{ column: 'age' }],
+ rejected_filters: [],
+ },
+ ],
+ filterState: {
+ value: [],
+ label: '',
+ excludeFilterValues: true,
+ },
+ };
+
+ render(
+ // @ts-expect-error
+ <SelectFilterPlugin
+ // @ts-expect-error
+ {...transformProps(testProps)}
+ setDataMask={jest.fn()}
+ showOverflow={false}
+ />,
+ {
+ useRedux: true,
+ initialState: {
+ nativeFilters: {
+ filters: {
+ 'test-filter': {
+ name: 'Test Filter',
+ },
+ },
+ },
+ dataMask: {
+ 'test-filter': {
+ extraFormData: {},
+ filterState: {
+ value: [],
+ label: '',
+ excludeFilterValues: true,
+ },
+ },
+ },
+ },
+ },
+ );
+
+ const filterSelect = screen.getAllByRole('combobox')[0];
+ userEvent.click(filterSelect);
+
+ const options = screen.getAllByRole('option');
+ expect(options[0]).toHaveTextContent('2000000000000000');
+ expect(options[1]).toHaveTextContent('10000000000000000');
+ expect(options[2]).toHaveTextContent('100000000000000000');
+ });
+
test('shows create option for multi-select creatable filter when typing', async () => {
getWrapper({ creatable: true, multiSelect: true });
userEvent.type(screen.getByRole('combobox'), 'brand-new');
diff --git a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
index 8523adb2c6f..e1dc3cc3657 100644
--- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
+++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
@@ -357,14 +357,20 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
return 0; // Preserve the original order from the backend
}
- // Only apply alphabetical sorting when no sortMetric is specified
- const labelComparator = propertyComparator('label');
+ // Only apply sorting when no sortMetric is specified. `label` is always
+ // a formatted string (see getDataRecordFormatter), so comparing by it
+ // never reaches propertyComparator's numeric branch; numeric columns
+ // sort by the raw `value` instead so "2, 10, 100" doesn't collapse
+ // into lexicographic "10, 100, 2".
+ const comparator = propertyComparator(
+ datatype === GenericDataType.Numeric ? 'value' : 'label',
+ );
if (formData.sortAscending) {
- return labelComparator(a, b);
+ return comparator(a, b);
}
- return labelComparator(b, a);
+ return comparator(b, a);
},
- [formData.sortAscending, formData.sortMetric],
+ [formData.sortAscending, formData.sortMetric, datatype],
);
// Use effect for initialisation for filter plugin