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>
This commit is contained in:
Martin Packman
2018-07-02 20:14:41 +02:00
committed by Torkel Ödegaard
parent 0d1f7c8782
commit 6046c8b4ca
2 changed files with 40 additions and 12 deletions

View File

@@ -44,3 +44,38 @@ describe('when sorting table asc', () => {
expect(table.rows[2][1]).toBe(15); 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', '', '']);
});
});

View File

@@ -19,23 +19,16 @@ export default class TableModel {
this.rows.sort(function(a, b) { this.rows.sort(function(a, b) {
a = a[options.col]; a = a[options.col];
b = b[options.col]; b = b[options.col];
if (a < b) { // Sort null or undefined seperately from comparable values
return -1; return +(a == null) - +(b == null) || +(a > b) || -(a < b);
}
if (a > b) {
return 1;
}
return 0;
}); });
this.columns[options.col].sort = true;
if (options.desc) { if (options.desc) {
this.rows.reverse(); 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) { addColumn(col) {