Bug Fix for _fnSort

awelch

I have noticed a problem involving table sorting and found some older posts that seem to be related as well (e.g. this discussion). Due to the current implementation of _fnSort certain situations will make sorting on a column seem non-deterministic. In particular, if you sort a column that has duplicate data values in it (so the data in each row isn't unique) the sorting can be influenced by previous sorting on the table. By this I mean, if you sort by a column, then sort by a column with duplicate data values you will see a different order than if you had only sorted by the column with duplicate data values. You can find an example of this here: live.datatables.net/dehifodu/1/edit. In this example you can see that although both tables are sorted by the Location column the sorting is substantially different because the bottom table was sorted by Name before being sorted by location. The effect is that you get multi-column sorting without transparency to the user. The larger problem with this is that if you clear the table (_fnClearTable) through some means (e.g. ajax.reload() or clear()) the sorting from the second column will be removed. This means upon refreshing a table the sorting can change unexpectedly.

The solution I am currently using for this is a small change to the _fnSort function. The original code is as follows:

/**
* Change the order of the table
*  @param {object} oSettings dataTables settings object
*  @memberof DataTable#oApi
*  @todo This really needs split up!
*/
function _fnSort ( oSettings )
{

…

/* No sorting required if server-side or no sorting array */
if ( _fnDataSource( oSettings ) != 'ssp' && aSort.length !== 0 )
{

    // Create a value - key array of the current row positions such that we can use their
    // current position during the sort, if values match, in order to perform stable sorting
    for ( i=0, iLen=displayMaster.length ; i<iLen ; i++ ) {
        aiOrig[ displayMaster[i] ] = i;
    }

…

}

My proposed solution is to change the above for loop to the following:

for ( i=0, iLen=displayMaster.length ; i<iLen ; i++ ) {
    aiOrig[i] = i;
}

In doing this aiOrig will not inherit the sorting currently in place on aiDisplayMaster. From what I can tell aiDisplayMaster seems to be pretty much read-only with the exception of clearing a table or performing sorting. I don't see a reason to read from the property rather than using an array with no previous sorting performed (i.e. [0, 1, 2, 3, …, n]).

I worried that this may affect the RowReorder plugin but after digging into to it a bit RowReorder seems to manipulate the data source directly rather than aiDisplayMaster.

There are a number of other ways to approach this such as calling _fnClearTable or manually resetting aiDisplayMaster in _fnSortListener before running _fnSort but the proposed solution seemed the cleanest to me. After a fair amount of testing I haven't seen any adverse effects from its implementation.

allan

This is excellent analysis thank you! A concur with your conclusion as well. I think what I'll do is put this change into v2 rather than into 1.10.x since it is effectively a change in behaviour.

Regards,
Allan

allan

4 years later... It has been committed

Allan