diff --git a/src/sql/base/browser/ui/table/tableDataView.ts b/src/sql/base/browser/ui/table/tableDataView.ts index 4d97a69d71..f77899ae8a 100644 --- a/src/sql/base/browser/ui/table/tableDataView.ts +++ b/src/sql/base/browser/ui/table/tableDataView.ts @@ -24,26 +24,33 @@ function defaultCellValueGetter(data: any): any { return data; } -function defaultSort(args: Slick.OnSortEventArgs, data: Array, cellValueGetter: CellValueGetter = defaultCellValueGetter): Array { +export function defaultSort(args: Slick.OnSortEventArgs, data: Array, cellValueGetter: CellValueGetter = defaultCellValueGetter): Array { if (!args.sortCol || !args.sortCol.field || data.length === 0) { return data; } const field = args.sortCol.field; const sign = args.sortAsc ? 1 : -1; - let sampleData = data[0][field]; - sampleData = types.isObject(sampleData) ? cellValueGetter(sampleData) : sampleData; - let comparer: (a: T, b: T) => number; - if (!isNaN(Number(sampleData))) { - comparer = (a: T, b: T) => { - let anum = Number(cellValueGetter(a[field])); - let bnum = Number(cellValueGetter(b[field])); - return anum === bnum ? 0 : anum > bnum ? 1 : -1; - }; - } else { - comparer = (a: T, b: T) => { - return stringCompare(cellValueGetter(a[field]), cellValueGetter(b[field])); - }; - } + const comparer: (a: T, b: T) => number = (a: T, b: T) => { + const value1 = cellValueGetter(a[field]); + const value2 = cellValueGetter(b[field]); + const num1 = Number(value1); + const num2 = Number(value2); + const isValue1Number = !isNaN(num1); + const isValue2Number = !isNaN(num2); + // Order: undefined -> number -> string + if (value1 === undefined || value2 === undefined) { + return value1 === value2 ? 0 : (value1 === undefined ? -1 : 1); + } else if (isValue1Number || isValue2Number) { + if (isValue1Number && isValue2Number) { + return num1 === num2 ? 0 : num1 > num2 ? 1 : -1; + } else { + return isValue1Number ? -1 : 1; + } + } else { + return stringCompare(value1, value2); + } + }; + return data.sort((a, b) => comparer(a, b) * sign); } diff --git a/src/sql/base/test/browser/ui/table/tableDataView.test.ts b/src/sql/base/test/browser/ui/table/tableDataView.test.ts index b1f9f12e4c..ce79387b46 100644 --- a/src/sql/base/test/browser/ui/table/tableDataView.test.ts +++ b/src/sql/base/test/browser/ui/table/tableDataView.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import { TableDataView } from 'sql/base/browser/ui/table/tableDataView'; +import { defaultSort, TableDataView } from 'sql/base/browser/ui/table/tableDataView'; suite('TableDataView', () => { test('Data can be filtered and filter can be cleared', () => { @@ -173,6 +173,55 @@ suite('TableDataView', () => { findValue = await dataView.findNext(); assert.deepStrictEqual(findValue, { row: 2, col: 0 }); }); + + test('Default Sorter test', async () => { + const originalData: Slick.SlickData[] = [ + { fieldName: '10b' }, + { fieldName: undefined }, + { fieldName: '100.01' }, + { fieldName: undefined }, + { fieldName: '10.1' }, + { fieldName: 'abc' } + ]; + + const sortAscArg: Slick.OnSortEventArgs = { + sortAsc: true, + sortCol: { + field: 'fieldName' + }, + multiColumnSort: false, + grid: undefined + }; + + const sortDescArg: Slick.OnSortEventArgs = { + sortAsc: false, + sortCol: { + field: 'fieldName' + }, + multiColumnSort: false, + grid: undefined + }; + const expectedAsc: Slick.SlickData[] = [ + { fieldName: undefined }, + { fieldName: undefined }, + { fieldName: '10.1' }, + { fieldName: '100.01' }, + { fieldName: '10b' }, + { fieldName: 'abc' } + ]; + const sortedAsc = defaultSort(sortAscArg, originalData); + assert.deepStrictEqual(sortedAsc, expectedAsc, 'ascending sorting is not working as expected'); + const expectedDesc: Slick.SlickData[] = [ + { fieldName: 'abc' }, + { fieldName: '10b' }, + { fieldName: '100.01' }, + { fieldName: '10.1' }, + { fieldName: undefined }, + { fieldName: undefined }, + ]; + const sortedDesc = defaultSort(sortDescArg, originalData); + assert.deepStrictEqual(sortedDesc, expectedDesc, 'descending sorting is not working as expected'); + }); }); function populateData(row: number, column: number): any[] { diff --git a/src/sql/workbench/contrib/query/browser/gridPanel.ts b/src/sql/workbench/contrib/query/browser/gridPanel.ts index 6978cd3414..5f1c8f155b 100644 --- a/src/sql/workbench/contrib/query/browser/gridPanel.ts +++ b/src/sql/workbench/contrib/query/browser/gridPanel.ts @@ -491,7 +491,7 @@ export abstract class GridTableBase extends Disposable implements IView { (offset, count) => { return this.loadData(offset, count); }, undefined, undefined, - (data: ICellValue) => { return data?.displayValue; }, + (data: ICellValue) => { return data.isNull ? undefined : data?.displayValue; }, { inMemoryDataProcessing: this.options.inMemoryDataProcessing, inMemoryDataCountThreshold: this.options.inMemoryDataCountThreshold