From 6046c8b4ca528bd4b2bb914f6d398914b6b29299 Mon Sep 17 00:00:00 2001
From: Martin Packman <gzlist@googlemail.com>
Date: Mon, 2 Jul 2018 20:14:41 +0200
Subject: [PATCH] Make table sorting stable when null values exist (#12362)

Currently if a null appears in a table column, for instance in data
returned by postgres, sorting on that gives an arbitrary order. This
is due to null being neither greater or less than any string, which
makes the sort unstable.

Change the table sort function to compare on nullness first. Note
this is a slight behaviour change for numbers, which would otherwise
treat null and 0 as equivalent.

Signed-off-by: Martin Packman <gzlist@googlemail.com>
---
 public/app/core/specs/table_model.jest.ts | 35 +++++++++++++++++++++++
 public/app/core/table_model.ts            | 17 ++++-------
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/public/app/core/specs/table_model.jest.ts b/public/app/core/specs/table_model.jest.ts
index a2c1eb5e1af..3d4c526cfea 100644
--- a/public/app/core/specs/table_model.jest.ts
+++ b/public/app/core/specs/table_model.jest.ts
@@ -44,3 +44,38 @@ describe('when sorting table asc', () => {
     expect(table.rows[2][1]).toBe(15);
   });
 });
+
+describe('when sorting with nulls', () => {
+  var table;
+  var values;
+
+  beforeEach(() => {
+    table = new TableModel();
+    table.columns = [{}, {}];
+    table.rows = [[42, ''], [19, 'a'], [null, 'b'], [0, 'd'], [null, null], [2, 'c'], [0, null], [-8, '']];
+  });
+
+  it('numbers with nulls at end with asc sort', () => {
+    table.sort({ col: 0, desc: false });
+    values = table.rows.map(row => row[0]);
+    expect(values).toEqual([-8, 0, 0, 2, 19, 42, null, null]);
+  });
+
+  it('numbers with nulls at start with desc sort', () => {
+    table.sort({ col: 0, desc: true });
+    values = table.rows.map(row => row[0]);
+    expect(values).toEqual([null, null, 42, 19, 2, 0, 0, -8]);
+  });
+
+  it('strings with nulls at end with asc sort', () => {
+    table.sort({ col: 1, desc: false });
+    values = table.rows.map(row => row[1]);
+    expect(values).toEqual(['', '', 'a', 'b', 'c', 'd', null, null]);
+  });
+
+  it('strings with nulls at start with desc sort', () => {
+    table.sort({ col: 1, desc: true });
+    values = table.rows.map(row => row[1]);
+    expect(values).toEqual([null, null, 'd', 'c', 'b', 'a', '', '']);
+  });
+});
diff --git a/public/app/core/table_model.ts b/public/app/core/table_model.ts
index 5716aac2be6..04857eb806d 100644
--- a/public/app/core/table_model.ts
+++ b/public/app/core/table_model.ts
@@ -19,23 +19,16 @@ export default class TableModel {
     this.rows.sort(function(a, b) {
       a = a[options.col];
       b = b[options.col];
-      if (a < b) {
-        return -1;
-      }
-      if (a > b) {
-        return 1;
-      }
-      return 0;
+      // Sort null or undefined seperately from comparable values
+      return +(a == null) - +(b == null) || +(a > b) || -(a < b);
     });
 
-    this.columns[options.col].sort = true;
-
     if (options.desc) {
       this.rows.reverse();
-      this.columns[options.col].desc = true;
-    } else {
-      this.columns[options.col].desc = false;
     }
+
+    this.columns[options.col].sort = true;
+    this.columns[options.col].desc = options.desc;
   }
 
   addColumn(col) {