SimpleQueryEngine doesn't correctly sort data with null [PATCH, CLA]

[Stefan Bird]

The sorting code in SimpleQueryEngine? assumes that all of the values it encounters are directly comparable, however strings and nulls are not: null < "hello"

null > "hello" == false. This means that sorting on an attribute that contains both strings and nulls will result in the rows being in an undefined order.

Note there are other situations where this can occur but the patch doesn't deal with them for performance reasons. The situation most likely to occur is sorting numbers with NaN values, eg [{a: 5}, {a: NaN}]. Maybe a flag could be added to the sort parameter to say that NaN values may be encountered (eg {attribute: "MyNumber?", mayHaveNaN: true}).

The code in the patch considers null to be larger than any string value; this is consistent with Array.sort()'s behaviour ([null, "hello"].sort() == ["hello", null])

[dante]

fwiw, I have an override in dojo/data/util/sorter that exposes the "direction" of the sort for this reason. Currently, default sort treats nulls as "higher than any value", so they always appear at the top (or bottom if sorted descending...). I needed a behavior where nulls were always treated as "incomplete", and _always_ at the bottom regardless of sort direction. (when ascending sort, treat null as "lower than any value", descending, treat as "higher") ... slightly related to this ticket is all. Would be nice to expose the direction of the sort to the sorting function so that it could more easily handle custom considerations like this.

[Kris Zyp]

In [29085]:

Properly handle nulls in sorting and allow sorting functions, fixes #15534, fixes #15515 !strict

[bill]

[29085] breaks util/doh/runner.html?testModule=dojo.tests.store.Memory on IE8:

    _AssertFailure: assertEqual() failed: 	expected
		four
	but got
		two

     ERROR IN: 		 function testQueryWithSort(t){
				t.is(store.query({prime: true}, {sort:[{attribute:"name"}]}).length, 3);
				t.is(store.query({even: true}, {sort:[{attribute:"name"}]})[1].name, "two");
				t.is(store.query({even: true}, {sort:function(a, b){
						return a > b ? -1 : 1;
					}})[1].name, "two");
				t.is(store.query(null, {sort:[{attribute:"mappedTo"}]})[4].name, "four");
			} FAILED test: testQueryWithSort 0 ms

[Kris Zyp]

In [29243]:

Fix custom sorting function test to use an actual property value, fixes #15515 !strict

[Kris Zyp]

#16065 is a duplicate of this ticket.