fix(time-series table): display null values in time-series table and …

…sortable (#19024)

* fix: display null values in time-series table and sortable

* add unit test

* fix unit test

* Add sortNumericValues with different nan treatment

Co-authored-by: Jesse Yang <jesse.yang@airbnb.com>

superset-frontend/src/utils/sortNumericValues.test.ts

@@ -0,0 +1,61 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import sortNumericValues from './sortNumericValues';
+
+test('should always sort null and NaNs to bottom', () => {
+  expect([null, 1, 2, '1', '5', NaN].sort(sortNumericValues)).toEqual([
+    1,
+    '1',
+    2,
+    '5',
+    NaN,
+    null,
+  ]);
+  expect(
+    [null, 1, 2, '1', '5', NaN].sort((a, b) =>
+      sortNumericValues(a, b, { descending: true }),
+    ),
+  ).toEqual(['5', 2, 1, '1', NaN, null]);
+});
+
+test('should treat null and NaN as smallest numbers', () => {
+  expect(
+    [null, 1, 2, '1', '5', NaN].sort((a, b) =>
+      sortNumericValues(a, b, { nanTreatment: 'asSmallest' }),
+    ),
+  ).toEqual([null, NaN, 1, '1', 2, '5']);
+  expect(
+    [null, 1, 2, '1', '5', NaN].sort((a, b) =>
+      sortNumericValues(a, b, { nanTreatment: 'asSmallest', descending: true }),
+    ),
+  ).toEqual(['5', 2, 1, '1', NaN, null]);
+});
+
+test('should treat null and NaN as largest numbers', () => {
+  expect(
+    [null, 1, 2, '1', '5', NaN].sort((a, b) =>
+      sortNumericValues(a, b, { nanTreatment: 'asLargest' }),
+    ),
+  ).toEqual([1, '1', 2, '5', NaN, null]);
+  expect(
+    [null, 1, 2, '1', '5', NaN].sort((a, b) =>
+      sortNumericValues(a, b, { nanTreatment: 'asLargest', descending: true }),
+    ),
+  ).toEqual([null, NaN, '5', 2, 1, '1']);
+});

superset-frontend/src/utils/sortNumericValues.ts

@@ -0,0 +1,52 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { JsonPrimitive } from '@superset-ui/core';
+
+export type NaNTreatment = 'alwaysLast' | 'asSmallest' | 'asLargest';
+
+/**
+ * Array.sort(...) comparator for potential numeric values with the ability to
+ * treat null and NaN as the smallest or largest values or always sort to bottom.
+ */
+export default function sortNumericValues(
+  valueA: JsonPrimitive,
+  valueB: JsonPrimitive,
+  {
+    descending = false,
+    nanTreatment = 'alwaysLast',
+  }: { descending?: boolean; nanTreatment?: NaNTreatment } = {},
+) {
+  let orderByIsNaN =
+    Number(valueA == null) - Number(valueB == null) ||
+    Number(Number.isNaN(Number(valueA))) - Number(Number.isNaN(Number(valueB)));
+
+  // if A is null or NaN and B is not, `orderByIsNaN` is 1,
+  // which will make A come after B in the sorted array,
+  // since we want to treat A as smallest number, we need to flip the sign
+  // when sorting in ascending order.
+  if (nanTreatment === 'asSmallest' && !descending) {
+    orderByIsNaN = -orderByIsNaN;
+  }
+  if (nanTreatment === 'asLargest' && descending) {
+    orderByIsNaN = -orderByIsNaN;
+  }
+  return (
+    orderByIsNaN || (Number(valueA) - Number(valueB)) * (descending ? -1 : 1)
+  );
+}

superset-frontend/src/visualizations/TimeTable/TimeTable.jsx

@@ -27,13 +27,23 @@ import {
   MetricOption,
 } from '@superset-ui/chart-controls';
 import moment from 'moment';
+import sortNumericValues from 'src/utils/sortNumericValues';
 
 import FormattedNumber from './FormattedNumber';
 import SparklineCell from './SparklineCell';
 import './TimeTable.less';
 
 const ACCESSIBLE_COLOR_BOUNDS = ['#ca0020', '#0571b0'];
 
+const sortNumberWithMixedTypes = (rowA, rowB, columnId, descending) =>
+  sortNumericValues(
+    rowA.values[columnId].props['data-value'],
+    rowB.values[columnId].props['data-value'],
+    { descending, nanTreatment: 'asSmallest' },
+  ) *
+  // react-table sort function always expects -1 for smaller number
+  (descending ? -1 : 1);
+
 function colorFromBounds(value, bounds, colorBounds = ACCESSIBLE_COLOR_BOUNDS) {
   if (bounds) {
     const [min, max] = bounds;
@@ -126,11 +136,7 @@ const TimeTable = ({
             )}
           </>
         ),
-        sortType: (rowA, rowB, columnId) => {
-          const rowAVal = rowA.values[columnId].props['data-value'];
-          const rowBVal = rowB.values[columnId].props['data-value'];
-          return rowAVal - rowBVal;
-        },
+        sortType: sortNumberWithMixedTypes,
       })),
     ],
     [columnConfigs],
@@ -192,14 +198,17 @@ const TimeTable = ({
         } else {
           v = reversedEntries[timeLag][valueField];
         }
-        if (column.comparisonType === 'diff') {
-          v = recent - v;
-        } else if (column.comparisonType === 'perc') {
-          v = recent / v;
-        } else if (column.comparisonType === 'perc_change') {
-          v = recent / v - 1;
+        if (typeof v === 'number' && typeof recent === 'number') {
+          if (column.comparisonType === 'diff') {
+            v = recent - v;
+          } else if (column.comparisonType === 'perc') {
+            v = recent / v;
+          } else if (column.comparisonType === 'perc_change') {
+            v = recent / v - 1;
+          }
+        } else {
+          v = null;
         }
-        v = v || 0;
       } else if (column.colType === 'contrib') {
         // contribution to column total
         v =