fix sorting bug (#17769)

* fix sorting bug

* comments

src/sql/base/browser/ui/table/tableDataView.ts

@@ -24,26 +24,33 @@ function defaultCellValueGetter(data: any): any {
 	return data;
 }
 
-function defaultSort<T extends Slick.SlickData>(args: Slick.OnSortEventArgs<T>, data: Array<T>, cellValueGetter: CellValueGetter = defaultCellValueGetter): Array<T> {
+export function defaultSort<T extends Slick.SlickData>(args: Slick.OnSortEventArgs<T>, data: Array<T>, cellValueGetter: CellValueGetter = defaultCellValueGetter): Array<T> {
 	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];
+	const comparer: (a: T, b: T) => number = (a: T, b: T) => {
-	sampleData = types.isObject(sampleData) ? cellValueGetter(sampleData) : sampleData;
+		const value1 = cellValueGetter(a[field]);
-	let comparer: (a: T, b: T) => number;
+		const value2 = cellValueGetter(b[field]);
-	if (!isNaN(Number(sampleData))) {
+		const num1 = Number(value1);
-		comparer = (a: T, b: T) => {
+		const num2 = Number(value2);
-			let anum = Number(cellValueGetter(a[field]));
+		const isValue1Number = !isNaN(num1);
-			let bnum = Number(cellValueGetter(b[field]));
+		const isValue2Number = !isNaN(num2);
-			return anum === bnum ? 0 : anum > bnum ? 1 : -1;
+		// Order: undefined -> number -> string
-		};
+		if (value1 === undefined || value2 === undefined) {
-	} else {
+			return value1 === value2 ? 0 : (value1 === undefined ? -1 : 1);
-		comparer = (a: T, b: T) => {
+		} else if (isValue1Number || isValue2Number) {
-			return stringCompare(cellValueGetter(a[field]), cellValueGetter(b[field]));
+			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);
 }
 

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<Slick.SlickData> = {
+			sortAsc: true,
+			sortCol: {
+				field: 'fieldName'
+			},
+			multiColumnSort: false,
+			grid: undefined
+		};
+
+		const sortDescArg: Slick.OnSortEventArgs<Slick.SlickData> = {
+			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[] {

src/sql/workbench/contrib/query/browser/gridPanel.ts

@@ -491,7 +491,7 @@ export abstract class GridTableBase<T> 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