Table: Fixes sorting for number fields (#34722)

* Table: Fixes sorting for number fields

* Refactor: changes after PR comments

* Refactor: found a bug

packages/grafana-ui/src/components/Table/Table.tsx

@@ -14,7 +14,7 @@ import {
   useTable,
 } from 'react-table';
 import { FixedSizeList } from 'react-window';
-import { getColumns, sortCaseInsensitive } from './utils';
+import { getColumns, sortCaseInsensitive, sortNumber } from './utils';
 import {
   TableColumnResizeActionCallback,
   TableFilterActionCallback,
@@ -153,7 +153,8 @@ export const Table: FC<Props> = memo((props: Props) => {
       stateReducer: stateReducer,
       initialState: getInitialState(initialSortBy, memoizedColumns),
       sortTypes: {
-        'alphanumeric-insensitive': sortCaseInsensitive,
+        number: sortNumber, // should be replace with the builtin number when react-table is upgraded, see https://github.com/tannerlinsley/react-table/pull/3235
+        'alphanumeric-insensitive': sortCaseInsensitive, // should be replace with the builtin string when react-table is upgraded, see https://github.com/tannerlinsley/react-table/pull/3235
       },
     }),
     [initialSortBy, memoizedColumns, memoizedData, resizable, stateReducer]

packages/grafana-ui/src/components/Table/utils.test.ts

@@ -6,6 +6,7 @@ import {
   getFilteredOptions,
   getTextAlign,
   rowToFieldValue,
+  sortNumber,
   sortOptions,
   valuesToOptions,
 } from './utils';
@@ -414,4 +415,61 @@ describe('Table utils', () => {
       });
     });
   });
+
+  describe('sortNumber', () => {
+    it.each`
+      a                                         | b                                         | expected
+      ${{ values: [] }}                         | ${{ values: [] }}                         | ${0}
+      ${{ values: [undefined] }}                | ${{ values: [undefined] }}                | ${0}
+      ${{ values: [null] }}                     | ${{ values: [null] }}                     | ${0}
+      ${{ values: [Number.POSITIVE_INFINITY] }} | ${{ values: [Number.POSITIVE_INFINITY] }} | ${0}
+      ${{ values: [Number.NEGATIVE_INFINITY] }} | ${{ values: [Number.NEGATIVE_INFINITY] }} | ${0}
+      ${{ values: [Number.POSITIVE_INFINITY] }} | ${{ values: [Number.NEGATIVE_INFINITY] }} | ${1}
+      ${{ values: [Number.NEGATIVE_INFINITY] }} | ${{ values: [Number.POSITIVE_INFINITY] }} | ${-1}
+      ${{ values: ['infinIty'] }}               | ${{ values: ['infinIty'] }}               | ${0}
+      ${{ values: ['infinity'] }}               | ${{ values: ['not infinity'] }}           | ${0}
+      ${{ values: [1] }}                        | ${{ values: [1] }}                        | ${0}
+      ${{ values: [1.5] }}                      | ${{ values: [1.5] }}                      | ${0}
+      ${{ values: [2] }}                        | ${{ values: [1] }}                        | ${1}
+      ${{ values: [25] }}                       | ${{ values: [2.5] }}                      | ${1}
+      ${{ values: [2.5] }}                      | ${{ values: [1.5] }}                      | ${1}
+      ${{ values: [1] }}                        | ${{ values: [2] }}                        | ${-1}
+      ${{ values: [2.5] }}                      | ${{ values: [25] }}                       | ${-1}
+      ${{ values: [1.5] }}                      | ${{ values: [2.5] }}                      | ${-1}
+      ${{ values: [1] }}                        | ${{ values: [] }}                         | ${1}
+      ${{ values: [1] }}                        | ${{ values: [undefined] }}                | ${1}
+      ${{ values: [1] }}                        | ${{ values: [null] }}                     | ${1}
+      ${{ values: [1] }}                        | ${{ values: [Number.POSITIVE_INFINITY] }} | ${-1}
+      ${{ values: [1] }}                        | ${{ values: [Number.NEGATIVE_INFINITY] }} | ${1}
+      ${{ values: [1] }}                        | ${{ values: ['infinIty'] }}               | ${1}
+      ${{ values: [-1] }}                       | ${{ values: ['infinIty'] }}               | ${1}
+      ${{ values: [] }}                         | ${{ values: [1] }}                        | ${-1}
+      ${{ values: [undefined] }}                | ${{ values: [1] }}                        | ${-1}
+      ${{ values: [null] }}                     | ${{ values: [1] }}                        | ${-1}
+      ${{ values: [Number.POSITIVE_INFINITY] }} | ${{ values: [1] }}                        | ${1}
+      ${{ values: [Number.NEGATIVE_INFINITY] }} | ${{ values: [1] }}                        | ${-1}
+      ${{ values: ['infinIty'] }}               | ${{ values: [1] }}                        | ${-1}
+      ${{ values: ['infinIty'] }}               | ${{ values: [-1] }}                       | ${-1}
+    `("when called with a: '$a.toString', b: '$b.toString' then result should be '$expected'", ({ a, b, expected }) => {
+      expect(sortNumber(a, b, '0')).toEqual(expected);
+    });
+
+    it.skip('should have good performance', () => {
+      const ITERATIONS = 100000;
+      const a: any = { values: Array(ITERATIONS) };
+      const b: any = { values: Array(ITERATIONS) };
+      for (let i = 0; i < ITERATIONS; i++) {
+        a.values[i] = Math.random() * Date.now();
+        b.values[i] = Math.random() * Date.now();
+      }
+
+      const start = performance.now();
+      for (let i = 0; i < ITERATIONS; i++) {
+        sortNumber(a, b, i.toString(10));
+      }
+      const stop = performance.now();
+      const diff = stop - start;
+      expect(diff).toBeLessThanOrEqual(20);
+    });
+  });
 });

packages/grafana-ui/src/components/Table/utils.ts

@@ -60,6 +60,7 @@ export function getColumns(data: DataFrame, availableWidth: number, columnMinWid
     const selectSortType = (type: FieldType): string => {
       switch (type) {
         case FieldType.number:
+          return 'number';
         case FieldType.time:
           return 'basic';
         default:
@@ -208,3 +209,22 @@ export function getFilteredOptions(options: SelectableValue[], filterValues?: Se
 export function sortCaseInsensitive(a: Row<any>, b: Row<any>, id: string) {
   return String(a.values[id]).localeCompare(String(b.values[id]), undefined, { sensitivity: 'base' });
 }
+
+// sortNumber needs to have great performance as it is called a lot
+export function sortNumber(rowA: Row<any>, rowB: Row<any>, id: string) {
+  const a = toNumber(rowA.values[id]);
+  const b = toNumber(rowB.values[id]);
+  return a === b ? 0 : a > b ? 1 : -1;
+}
+
+function toNumber(value: any): number {
+  if (typeof value === 'number') {
+    return value;
+  }
+
+  if (value === null || value === undefined || value === '' || isNaN(value)) {
+    return Number.NEGATIVE_INFINITY;
+  }
+
+  return Number(value);
+}