Fixes bug around sorting equal and unequal properties (#20702)

* Fixes bug around sorting equal and unequal properties

* Uses deepClone to deepCopy objects

* Code review changes

Co-authored-by: Charles Gagnon <chgagnon@microsoft.com>
This commit is contained in:
Lewis Sanchez
2022-10-03 23:10:45 -07:00
committed by GitHub
parent 16721cac00
commit 5e48cb99e1

View File

@@ -19,6 +19,7 @@ import { IContextMenuService, IContextViewService } from 'vs/platform/contextvie
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { ITextResourcePropertiesService } from 'vs/editor/common/services/textResourceConfigurationService'; import { ITextResourcePropertiesService } from 'vs/editor/common/services/textResourceConfigurationService';
import { Codicon } from 'vs/base/common/codicons'; import { Codicon } from 'vs/base/common/codicons';
import { deepClone } from 'vs/base/common/objects';
export enum ExecutionPlanCompareOrientation { export enum ExecutionPlanCompareOrientation {
Horizontal = 'horizontal', Horizontal = 'horizontal',
@@ -173,7 +174,8 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
secondaryProps = this._model.secondaryElement.properties; secondaryProps = this._model.secondaryElement.properties;
} }
const tableRows = this.convertPropertiesToTableRows(primaryProps, secondaryProps); let tableRows = this.convertPropertiesToTableRows(primaryProps, secondaryProps);
tableRows = this.sortPropertiesByDisplayValueEquivalency(tableRows);
this.setSummaryElement(this.getExpensivePropertySummary(tableRows)); this.setSummaryElement(this.getExpensivePropertySummary(tableRows));
this.populateTable(columns, tableRows); this.populateTable(columns, tableRows);
} }
@@ -189,14 +191,14 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
* @param tableRows The table rows that will appear in the properties table. * @param tableRows The table rows that will appear in the properties table.
* @returns The string array containing the segments of the summary. * @returns The string array containing the segments of the summary.
*/ */
private getExpensivePropertySummary(tableRows: { [key: string]: string }[]): string[] { private getExpensivePropertySummary(tableRows: TableRow[]): string[] {
let summary: string[] = []; let summary: string[] = [];
tableRows.forEach(row => { tableRows.forEach(row => {
const rowName = row.name['text']; const rowName = (<RowContent>row.name).text;
if (row.primary && row.secondary) { if (row.primary && row.secondary) {
const primaryText = row.primary['text'].split(' '); const primaryText = row.primary.text.split(' ');
const secondaryTitle = row.secondary['title'].split(' '); const secondaryTitle = row.secondary.title.split(' ');
if (primaryText.length === secondaryTitle.length && primaryText.length <= 2 && secondaryTitle.length <= 2) { if (primaryText.length === secondaryTitle.length && primaryText.length <= 2 && secondaryTitle.length <= 2) {
const MAX_PROPERTY_SUMMARY_LENGTH = 3; const MAX_PROPERTY_SUMMARY_LENGTH = 3;
@@ -310,46 +312,71 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
* *
* Name Value (Top plan) Value (Bottom Plan) * Name Value (Top plan) Value (Bottom Plan)
* ------------------------------------------------------------------------- * -------------------------------------------------------------------------
* Compile Time 38 37 <diff> * Compile Time 38 37 <unequal>
* CompileCpu 38 37 <diff> * CompileCpu 38 37 <unequal>
* CompileMemory 5816 6424 <diff> * CompileMemory 5816 6424 <unequal>
* Estimated Number of Rows 1000 1000 <same> * Estimated Number of Rows 1000 1000 <equal>
* Optimization Level FULL FULL <same> * Optimization Level FULL FULL <equal>
* RetrievedFromCache false false <same> * RetrievedFromCache false false <equal>
* *
* @param props Map of properties that will be organized. * @param rows An array of TableRows that contains all the properties that will be organized.
* @returns A new map with different values appearing at the top and similar values appearing at the bottom. * @returns A new array of TableRows with unequal values appearing at the top and equal values appearing at the bottom.
*/ */
public sortPropertiesByDisplayValueEquivalency(props: Map<string, TablePropertiesMapEntry>, sortProperties: (props: Map<string, TablePropertiesMapEntry>) => Map<string, TablePropertiesMapEntry>): Map<string, TablePropertiesMapEntry> { public sortPropertiesByDisplayValueEquivalency(rows: TableRow[]): TableRow[] {
let unequalProperties: Map<string, TablePropertiesMapEntry> = new Map(); const [unequalPropertyRows, equalPropertyRows] = this.splitEqualFromUnequalProperties(rows);
let equalProperties: Map<string, TablePropertiesMapEntry> = new Map();
[...props.entries()].forEach(prop => { const organizedPropertyRows: TableRow[] = [...unequalPropertyRows];
const [rowKey, rowEntry] = prop;
const primaryProp = rowEntry.primaryProp;
const secondaryProp = rowEntry.secondaryProp;
if (primaryProp?.displayValue.localeCompare(secondaryProp?.displayValue) === 0) { if (equalPropertyRows.length > 0) {
equalProperties.set(rowKey, rowEntry); const equivalentPropertiesRow: TableRow = new Object() as TableRow;
equivalentPropertiesRow.name = equivalentPropertiesRowHeader;
equivalentPropertiesRow.expanded = false;
equivalentPropertiesRow.treeGridChildren = equalPropertyRows;
organizedPropertyRows.push(equivalentPropertiesRow);
}
return organizedPropertyRows;
}
private splitEqualFromUnequalProperties(rows: TableRow[]): [TableRow[], TableRow[]] {
const unequalRows: TableRow[] = [];
const equalRows: TableRow[] = [];
for (let row of rows) {
const treeGridChildren = row.treeGridChildren;
if (treeGridChildren?.length > 0) {
let [unequalSubRows, equalSubRows] = this.splitEqualFromUnequalProperties(treeGridChildren);
if (unequalSubRows.length > 0) {
let currentRow = deepClone(row);
currentRow.treeGridChildren = unequalSubRows;
unequalRows.push(currentRow);
}
if (equalSubRows.length > 0) {
let currentRow = deepClone(row);
currentRow.treeGridChildren = equalSubRows;
equalRows.push(currentRow);
}
} }
else { else {
unequalProperties.set(rowKey, rowEntry); const primary = row.primary;
const secondary = row.secondary;
if (primary && secondary && primary.text === secondary.title) {
equalRows.push(row);
}
else {
unequalRows.push(row);
}
} }
}); }
unequalProperties = sortProperties(unequalProperties); return [unequalRows, equalRows];
equalProperties = sortProperties(equalProperties);
let map: Map<string, TablePropertiesMapEntry> = new Map();
unequalProperties.forEach((v, k) => {
map.set(k, v);
});
equalProperties.forEach((v, k) => {
map.set(k, v);
});
return map;
} }
public sortPropertiesReverseAlphabetically(props: Map<string, TablePropertiesMapEntry>): Map<string, TablePropertiesMapEntry> { public sortPropertiesReverseAlphabetically(props: Map<string, TablePropertiesMapEntry>): Map<string, TablePropertiesMapEntry> {
@@ -366,8 +393,8 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
})); }));
} }
private convertPropertiesToTableRows(primaryNode: azdata.executionPlan.ExecutionPlanGraphElementProperty[], secondaryNode: azdata.executionPlan.ExecutionPlanGraphElementProperty[]): { [key: string]: string }[] { private convertPropertiesToTableRows(primaryNode: azdata.executionPlan.ExecutionPlanGraphElementProperty[], secondaryNode: azdata.executionPlan.ExecutionPlanGraphElementProperty[]): TableRow[] {
const rows: { [key: string]: string }[] = []; const rows: TableRow[] = [];
let propertiesMap: Map<string, TablePropertiesMapEntry> = new Map(); let propertiesMap: Map<string, TablePropertiesMapEntry> = new Map();
if (primaryNode) { if (primaryNode) {
@@ -398,28 +425,27 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
switch (this.sortType) { switch (this.sortType) {
case PropertiesSortType.DisplayOrder: case PropertiesSortType.DisplayOrder:
propertiesMap = this.sortPropertiesByDisplayValueEquivalency(propertiesMap, this.sortPropertiesByImportance); propertiesMap = this.sortPropertiesByImportance(propertiesMap);
break; break;
case PropertiesSortType.Alphabetical: case PropertiesSortType.Alphabetical:
propertiesMap = this.sortPropertiesByDisplayValueEquivalency(propertiesMap, this.sortPropertiesAlphabetically); propertiesMap = this.sortPropertiesAlphabetically(propertiesMap);
break; break;
case PropertiesSortType.ReverseAlphabetical: case PropertiesSortType.ReverseAlphabetical:
propertiesMap = this.sortPropertiesByDisplayValueEquivalency(propertiesMap, this.sortPropertiesReverseAlphabetically); propertiesMap = this.sortPropertiesReverseAlphabetically(propertiesMap);
break; break;
} }
propertiesMap.forEach((v, k) => { propertiesMap.forEach((v, k) => {
let row = {}; let row: TableRow = new Object() as TableRow;
row['name'] = { row.name = {
text: k text: k
}; };
const primaryProp = v.primaryProp; const primaryProp = v.primaryProp;
const secondaryProp = v.secondaryProp; const secondaryProp = v.secondaryProp;
let diffIconClass = '';
if (primaryProp && secondaryProp) { if (primaryProp && secondaryProp) {
row['displayOrder'] = v.primaryProp.displayOrder; row.displayOrder = v.primaryProp.displayOrder;
let diffIcon = new Object() as DiffIcon; let diffIcon = new Object() as DiffIcon;
if (v.primaryProp.displayValue !== v.secondaryProp.displayValue) { if (v.primaryProp.displayValue !== v.secondaryProp.displayValue) {
@@ -445,80 +471,62 @@ export class ExecutionPlanComparisonPropertiesView extends ExecutionPlanProperti
} }
} }
row['primary'] = { row.primary = {
text: removeLineBreaks(v.primaryProp.displayValue, ' ') text: removeLineBreaks(v.primaryProp.displayValue, ' ')
}; };
row['icon'] = { row.icon = {
iconCssClass: diffIcon.iconClass, iconCssClass: diffIcon.iconClass ?? '',
title: diffIcon.title title: diffIcon.title ?? ''
}; };
row['secondary'] = { row.secondary = {
title: removeLineBreaks(v.secondaryProp.displayValue, ' ') title: removeLineBreaks(v.secondaryProp.displayValue, ' '),
}; };
if ((primaryProp && !isString(primaryProp.value)) || (secondaryProp && !isString(secondaryProp.value))) { if ((primaryProp && !isString(primaryProp.value)) || (secondaryProp && !isString(secondaryProp.value))) {
row['name'].iconCssClass += ` parent-row-styling`; const parentRowStyling = ' parent-row-styling';
row['primary'].iconCssClass += ` parent-row-styling`;
row['icon'].iconCssClass += 'parent-row-styling'; row.name.iconCssClass = !row.name.iconCssClass ? parentRowStyling : row.name.iconCssClass + parentRowStyling;
row['secondary'].iconCssClass += ` parent-row-styling`; row.primary.iconCssClass = !row.primary.iconCssClass ? parentRowStyling : row.primary.iconCssClass + parentRowStyling;
row.icon.iconCssClass = !row.icon.iconCssClass ? parentRowStyling : row.icon.iconCssClass + parentRowStyling;
row.secondary.iconCssClass = !row.secondary.iconCssClass ? parentRowStyling : row.secondary.iconCssClass + parentRowStyling;
} }
rows.push(row); rows.push(row);
const topPropValue = isString(primaryProp.value) ? undefined : primaryProp.value; const topPropValue = isString(primaryProp.value) ? undefined : primaryProp.value;
const bottomPropValue = isString(secondaryProp.value) ? undefined : secondaryProp.value; const bottomPropValue = isString(secondaryProp.value) ? undefined : secondaryProp.value;
row['treeGridChildren'] = this.convertPropertiesToTableRows(topPropValue, bottomPropValue); row.treeGridChildren = this.convertPropertiesToTableRows(topPropValue, bottomPropValue);
} else if (primaryProp && !secondaryProp) { } else if (primaryProp && !secondaryProp) {
row['displayOrder'] = v.primaryProp.displayOrder; row.displayOrder = v.primaryProp.displayOrder;
row['primary'] = { row.primary = {
text: v.primaryProp.displayValue text: v.primaryProp.displayValue
}; };
rows.push(row); rows.push(row);
if (!isString(primaryProp.value)) { if (!isString(primaryProp.value)) {
row['name'].iconCssClass += ` parent-row-styling`; row.name.iconCssClass += ` parent-row-styling`;
row['primary'].iconCssClass += ` parent-row-styling`; row.primary.iconCssClass += ` parent-row-styling`;
row['treeGridChildren'] = this.convertPropertiesToTableRows(primaryProp.value, undefined); row.treeGridChildren = this.convertPropertiesToTableRows(primaryProp.value, undefined);
} }
} else if (!primaryProp && secondaryProp) { } else if (!primaryProp && secondaryProp) {
row['displayOrder'] = v.secondaryProp.displayOrder; row.displayOrder = v.secondaryProp.displayOrder;
row['secondary'] = { row.secondary = {
title: v.secondaryProp.displayValue, title: v.secondaryProp.displayValue,
iconCssClass: diffIconClass iconCssClass: ''
}; };
rows.push(row); rows.push(row);
if (!isString(secondaryProp.value)) { if (!isString(secondaryProp.value)) {
row['name'].iconCssClass += ` parent-row-styling`; row.name.iconCssClass += ` parent-row-styling`;
row['secondary'].iconCssClass += ` parent-row-styling`; row.secondary.iconCssClass += ` parent-row-styling`;
row['treeGridChildren'] = this.convertPropertiesToTableRows(undefined, secondaryProp.value); row.treeGridChildren = this.convertPropertiesToTableRows(undefined, secondaryProp.value);
} }
} }
}); });
let formattedRows: { [key: string]: string }[] = []; return rows;
let equalRows: { [key: string]: string }[] = [];
for (const [_, row] of Object.entries(rows)) {
if (row.primary && row.secondary && row.primary['text'] === row.secondary['title']) {
equalRows.push(row);
}
else {
formattedRows.push(row);
}
}
if (equalRows.length > 0) {
let equalRow = {};
equalRow['name'] = equivalentPropertiesRowHeader;
equalRow['expanded'] = false;
equalRow['treeGridChildren'] = equalRows;
formattedRows.push(equalRow);
}
return formattedRows;
} }
set orientation(value: ExecutionPlanCompareOrientation) { set orientation(value: ExecutionPlanCompareOrientation) {
@@ -564,3 +572,19 @@ interface DiffIcon {
iconClass: string; iconClass: string;
title: string; title: string;
} }
interface TableRow extends Slick.SlickData {
displayOrder: number;
icon: RowContent;
name: RowContent | string;
primary: RowContent;
secondary: RowContent;
expanded: boolean;
treeGridChildren: TableRow[];
}
interface RowContent {
iconCssClass?: string;
text?: string;
title?: string;
}