Skip to content

Allow pagination for interpreted results #481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jcreedcmu merged 16 commits into from
Jul 6, 2020
Merged

Allow pagination for interpreted results #481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
d7732c4
Add interpreted results page size
jcreedcmu Jun 29, 2020
addddb0
Factor out truncation of results from interpretation itself
jcreedcmu Jun 29, 2020
b3642bd
Compute truncation functionally rather than imperative update
jcreedcmu Jun 29, 2020
4030ddb
Teach extension how to request display of alerts page
jcreedcmu Jun 29, 2020
fe02a58
Teach extension to accept ShowInterpretedPageMsg
jcreedcmu Jun 29, 2020
65aa692
Show number of pages in pagination interface
jcreedcmu Jun 29, 2020
0e3b7a8
Also show pagination interface for interpreted results
jcreedcmu Jun 29, 2020
11c538a
Teach webview to do pagination in interpreted results
jcreedcmu Jun 30, 2020
88c2761
Show number of results correctly
jcreedcmu Jun 30, 2020
96688e3
Fix listing of tables when in alerts view
jcreedcmu Jun 30, 2020
94d2303
Allow switching to alerts table
jcreedcmu Jun 30, 2020
ec45db3
Appease linter.
jcreedcmu Jun 30, 2020
e0a58a8
Inline now-trivial function paginationEnabled
jcreedcmu Jul 1, 2020
0b32961
Unbreak typing page number into field
jcreedcmu Jul 2, 2020
e345425
Fix wrong number of pages shown on initial display.
jcreedcmu Jul 2, 2020
e2d125a
Fix broken raw queries.
jcreedcmu Jul 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion extensions/ql-vscode/src/adapt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { DecodedBqrsChunk, ResultSetSchema, ColumnKind, Column, ColumnValue } from './bqrs-cli-types';
import { LocationValue, ResultSetSchema as AdaptedSchema, ColumnSchema, ColumnType, LocationStyle } from 'semmle-bqrs';
import { ResultSet } from './interface-types';

// FIXME: This is a temporary bit of impedance matching to convert
// from the types provided by ./bqrs-cli-types, to the types used by
Expand Down Expand Up @@ -128,7 +129,8 @@ export interface ExtensionParsedResultSets {
t: 'ExtensionParsed';
pageNumber: number;
numPages: number;
numInterpretedPages: number;
selectedTable?: string; // when undefined, means 'show default table'
resultSetNames: string[];
resultSet: RawResultSet;
resultSet: ResultSet;
}
22 changes: 17 additions & 5 deletions extensions/ql-vscode/src/interface-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@ export type PathTableResultSet = {

export type ResultSet = RawTableResultSet | PathTableResultSet;

/**
* Only ever show this many results per run in interpreted results.
*/
export const INTERPRETED_RESULTS_PER_RUN_LIMIT = 100;

/**
* Only ever show this many rows in a raw result table.
*/
Expand All @@ -38,6 +33,11 @@ export const RAW_RESULTS_LIMIT = 10000;
*/
export const RAW_RESULTS_PAGE_SIZE = 100;

/**
* Show this many rows in an interpreted results table at a time.
*/
export const INTERPRETED_RESULTS_PAGE_SIZE = 100;

export interface DatabaseInfo {
name: string;
databaseUri: string;
Expand All @@ -61,6 +61,7 @@ export interface PreviousExecution {
export interface Interpretation {
sourceLocationPrefix: string;
numTruncatedResults: number;
numTotalResults: number;
/**
* sortState being undefined means don't sort, just present results in the order
* they appear in the sarif file.
Expand Down Expand Up @@ -113,6 +114,16 @@ export interface SetStateMsg {
parsedResultSets: ParsedResultSets;
}

export interface ShowInterpretedPageMsg {
t: 'showInterpretedPage';
interpretation: Interpretation;
database: DatabaseInfo;
metadata?: QueryMetadata;
pageNumber: number;
numPages: number;
resultSetNames: string[];
}

/** Advance to the next or previous path no in the path viewer */
export interface NavigatePathMsg {
t: 'navigatePath';
Expand All @@ -124,6 +135,7 @@ export interface NavigatePathMsg {
export type IntoResultsViewMsg =
| ResultsUpdatingMsg
| SetStateMsg
| ShowInterpretedPageMsg
| NavigatePathMsg;

export type FromResultsViewMsg =
Expand Down
160 changes: 110 additions & 50 deletions extensions/ql-vscode/src/interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { assertNever } from './helpers-pure';
import {
FromResultsViewMsg,
Interpretation,
INTERPRETED_RESULTS_PER_RUN_LIMIT,
IntoResultsViewMsg,
QueryMetadata,
ResultsPaths,
Expand All @@ -28,6 +27,8 @@ import {
InterpretedResultsSortState,
SortDirection,
RAW_RESULTS_PAGE_SIZE,
INTERPRETED_RESULTS_PAGE_SIZE,
ALERTS_TABLE_NAME,
} from './interface-types';
import { Logger } from './logging';
import * as messages from './messages';
Expand All @@ -51,6 +52,7 @@ import {
jumpToLocation,
} from './interface-utils';
import { getDefaultResultSetName } from './interface-types';
import { ResultSetSchema } from './bqrs-cli-types';

/**
* interface.ts
Expand Down Expand Up @@ -95,6 +97,10 @@ function numPagesOfResultSet(resultSet: RawResultSet): number {
return Math.ceil(resultSet.schema.tupleCount / RAW_RESULTS_PAGE_SIZE);
}

function numInterpretedPages(interpretation: Interpretation | undefined): number {
return Math.ceil((interpretation?.sarif.runs[0].results?.length || 0) / INTERPRETED_RESULTS_PAGE_SIZE);
}

export class InterfaceManager extends DisposableObject {
private _displayedQuery?: CompletedQuery;
private _interpretation?: Interpretation;
Expand Down Expand Up @@ -244,7 +250,12 @@ export class InterfaceManager extends DisposableObject {
);
break;
case 'changePage':
await this.showPageOfResults(msg.selectedTable, msg.pageNumber);
if (msg.selectedTable === ALERTS_TABLE_NAME) {
await this.showPageOfInterpretedResults(msg.pageNumber);
}
else {
await this.showPageOfRawResults(msg.selectedTable, msg.pageNumber);
}
break;
default:
assertNever(msg);
Expand Down Expand Up @@ -283,7 +294,8 @@ export class InterfaceManager extends DisposableObject {
return;
}

const interpretation = await this.interpretResultsInfo(
this._interpretation = undefined;
const interpretationPage = await this.interpretResultsInfo(
results.query,
results.interpretedResultsSortState
);
Expand All @@ -295,7 +307,6 @@ export class InterfaceManager extends DisposableObject {
);

this._displayedQuery = results;
this._interpretation = interpretation;

const panel = this.getPanel();
await this.waitForPanelLoaded();
Expand Down Expand Up @@ -325,19 +336,13 @@ export class InterfaceManager extends DisposableObject {

const getParsedResultSets = async (): Promise<ParsedResultSets> => {
if (EXPERIMENTAL_BQRS_SETTING.getValue()) {
const schemas = await this.cliServer.bqrsInfo(
results.query.resultsPaths.resultsPath,
RAW_RESULTS_PAGE_SIZE
);

const resultSetNames = schemas['result-sets'].map(
(resultSet) => resultSet.name
);
const resultSetSchemas = await this.getResultSetSchemas(results);
const resultSetNames = resultSetSchemas.map(schema => schema.name);

// This may not wind up being the page we actually show, if there are interpreted results,
// but speculatively send it anyway.
const selectedTable = getDefaultResultSetName(resultSetNames);
const schema = schemas['result-sets'].find(
const schema = resultSetSchemas.find(
(resultSet) => resultSet.name == selectedTable
)!;
if (schema === undefined) {
Expand All @@ -352,12 +357,12 @@ export class InterfaceManager extends DisposableObject {
);
const adaptedSchema = adaptSchema(schema);
const resultSet = adaptBqrs(adaptedSchema, chunk);

return {
t: 'ExtensionParsed',
pageNumber: 0,
numPages: numPagesOfResultSet(resultSet),
resultSet,
numInterpretedPages: numInterpretedPages(this._interpretation),
resultSet: { t: 'RawResultSet', ...resultSet },
Copy link
Copy Markdown
Contributor

@aeisenberg aeisenberg Jun 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has the type RawResultSet changed? In the current main branch, it looks like there is no t property.

Copy link
Copy Markdown
Contributor Author

@jcreedcmu jcreedcmu Jul 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To accomodate sarif results, I changed the type of the .resultSet field of ExtensionParsedResultSets from being always only a RawResultSet to being a ResultSet (which is either a RawResultSet or a SarifResultSet, tagged appropriately)

selectedTable: undefined,
resultSetNames,
};
Expand All @@ -368,7 +373,7 @@ export class InterfaceManager extends DisposableObject {

await this.postMessage({
t: 'setState',
interpretation,
interpretation: interpretationPage,
origResultsPaths: results.query.resultsPaths,
resultsPath: this.convertPathToWebviewUri(
results.query.resultsPaths.resultsPath
Expand All @@ -381,10 +386,48 @@ export class InterfaceManager extends DisposableObject {
});
}

/**
* Show a page of interpreted results
*/
public async showPageOfInterpretedResults(
pageNumber: number
): Promise<void> {
if (this._displayedQuery === undefined) {
throw new Error('Trying to show interpreted results but displayed query was undefined');
}
if (this._interpretation === undefined) {
throw new Error('Trying to show interpreted results but interpretation was undefined');
}
if (this._interpretation.sarif.runs[0].results === undefined) {
throw new Error('Trying to show interpreted results but results were undefined');
}

const resultSetSchemas = await this.getResultSetSchemas(this._displayedQuery);
const resultSetNames = resultSetSchemas.map(schema => schema.name);

await this.postMessage({
t: 'showInterpretedPage',
interpretation: this.getPageOfInterpretedResults(pageNumber),
database: this._displayedQuery.database,
metadata: this._displayedQuery.query.metadata,
pageNumber,
resultSetNames,
numPages: numInterpretedPages(this._interpretation),
});
}

private async getResultSetSchemas(results: CompletedQuery): Promise<ResultSetSchema[]> {
const schemas = await this.cliServer.bqrsInfo(
results.query.resultsPaths.resultsPath,
RAW_RESULTS_PAGE_SIZE
);
return schemas['result-sets'];
}

/**
* Show a page of raw results from the chosen table.
*/
public async showPageOfResults(
public async showPageOfRawResults(
selectedTable: string,
pageNumber: number
): Promise<void> {
Expand All @@ -399,16 +442,10 @@ export class InterfaceManager extends DisposableObject {
(sortedResultsMap[k] = this.convertPathPropertiesToWebviewUris(v))
);

const schemas = await this.cliServer.bqrsInfo(
results.query.resultsPaths.resultsPath,
RAW_RESULTS_PAGE_SIZE
);

const resultSetNames = schemas['result-sets'].map(
(resultSet) => resultSet.name
);
const resultSetSchemas = await this.getResultSetSchemas(results);
const resultSetNames = resultSetSchemas.map(schema => schema.name);

const schema = schemas['result-sets'].find(
const schema = resultSetSchemas.find(
(resultSet) => resultSet.name == selectedTable
)!;
if (schema === undefined)
Expand All @@ -426,8 +463,9 @@ export class InterfaceManager extends DisposableObject {
const parsedResultSets: ParsedResultSets = {
t: 'ExtensionParsed',
pageNumber,
resultSet,
resultSet: { t: 'RawResultSet', ...resultSet },
numPages: numPagesOfResultSet(resultSet),
numInterpretedPages: numInterpretedPages(this._interpretation),
selectedTable: selectedTable,
resultSetNames,
};
Expand All @@ -447,7 +485,7 @@ export class InterfaceManager extends DisposableObject {
});
}

private async getTruncatedResults(
private async _getInterpretedResults(
metadata: QueryMetadata | undefined,
resultsPaths: ResultsPaths,
sourceInfo: cli.SourceInfo | undefined,
Expand All @@ -460,37 +498,58 @@ export class InterfaceManager extends DisposableObject {
resultsPaths,
sourceInfo
);
// For performance reasons, limit the number of results we try
// to serialize and send to the webview. TODO: possibly also
// limit number of paths per result, number of steps per path,
// or throw an error if we are in aggregate trying to send
// massively too much data, as it can make the extension
// unresponsive.

let numTruncatedResults = 0;
sarif.runs.forEach((run) => {
if (run.results !== undefined) {
sarif.runs.forEach(run => {
if (run.results !== undefined)
sortInterpretedResults(run.results, sortState);
if (run.results.length > INTERPRETED_RESULTS_PER_RUN_LIMIT) {
numTruncatedResults +=
run.results.length - INTERPRETED_RESULTS_PER_RUN_LIMIT;
run.results = run.results.slice(0, INTERPRETED_RESULTS_PER_RUN_LIMIT);
}
}
});
return {

const numTotalResults = (() => {
if (sarif.runs.length === 0) return 0;
if (sarif.runs[0].results === undefined) return 0;
return sarif.runs[0].results.length;
})();

const interpretation: Interpretation = {
sarif,
sourceLocationPrefix,
numTruncatedResults,
numTruncatedResults: 0,
numTotalResults,
sortState,
};
this._interpretation = interpretation;
return interpretation;
}

private getPageOfInterpretedResults(
pageNumber: number
): Interpretation {

function getPageOfRun(run: Sarif.Run): Sarif.Run {
return {
...run, results: run.results?.slice(
INTERPRETED_RESULTS_PAGE_SIZE * pageNumber,
INTERPRETED_RESULTS_PAGE_SIZE * (pageNumber + 1)
)
};
}

if (this._interpretation === undefined) {
throw new Error('Tried to get interpreted results before interpretation finished');
}
if (this._interpretation.sarif.runs.length !== 1) {
this.logger.log(`Warning: SARIF file had ${this._interpretation.sarif.runs.length} runs, expected 1`);
}
const interp = this._interpretation;
return {
...interp,
sarif: { ...interp.sarif, runs: [getPageOfRun(interp.sarif.runs[0])] },
};
}

private async interpretResultsInfo(
query: QueryInfo,
sortState: InterpretedResultsSortState | undefined
): Promise<Interpretation | undefined> {
let interpretation: Interpretation | undefined = undefined;
if (
(await query.canHaveInterpretedResults()) &&
query.quickEvalPosition === undefined // never do results interpretation if quickEval
Expand All @@ -507,7 +566,7 @@ export class InterfaceManager extends DisposableObject {
sourceArchive: sourceArchiveUri.fsPath,
sourceLocationPrefix,
};
interpretation = await this.getTruncatedResults(
await this._getInterpretedResults(
query.metadata,
query.resultsPaths,
sourceInfo,
Expand All @@ -522,7 +581,7 @@ export class InterfaceManager extends DisposableObject {
);
}
}
return interpretation;
return this._interpretation && this.getPageOfInterpretedResults(0);
}

private async showResultsAsDiagnostics(
Expand All @@ -541,7 +600,8 @@ export class InterfaceManager extends DisposableObject {
sourceArchive: sourceArchiveUri.fsPath,
sourceLocationPrefix,
};
const interpretation = await this.getTruncatedResults(
// TODO: Performance-testing to determine whether this truncation is necessary.
const interpretation = await this._getInterpretedResults(
metadata,
resultsInfo,
sourceInfo,
Expand Down
Loading