Merge pull request #481 from jcreedcmu/jcreed/interpreted-pagination

Allow pagination for interpreted results

extensions/ql-vscode/src/adapt.ts

@@ -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
@@ -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;
 }

extensions/ql-vscode/src/interface-types.ts

@@ -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.
  */
@@ -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;
@@ -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.
@@ -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';
@@ -124,6 +135,7 @@ export interface NavigatePathMsg {
 export type IntoResultsViewMsg =
   | ResultsUpdatingMsg
   | SetStateMsg
+  | ShowInterpretedPageMsg
   | NavigatePathMsg;
 
 export type FromResultsViewMsg =

extensions/ql-vscode/src/interface.ts

@@ -19,7 +19,6 @@ import { assertNever } from './helpers-pure';
 import {
   FromResultsViewMsg,
   Interpretation,
-  INTERPRETED_RESULTS_PER_RUN_LIMIT,
   IntoResultsViewMsg,
   QueryMetadata,
   ResultsPaths,
@@ -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';
@@ -51,6 +52,7 @@ import {
   jumpToLocation,
 } from './interface-utils';
 import { getDefaultResultSetName } from './interface-types';
+import { ResultSetSchema } from './bqrs-cli-types';
 
 /**
  * interface.ts
@@ -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;
@@ -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);
@@ -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
     );
@@ -295,7 +307,6 @@ export class InterfaceManager extends DisposableObject {
     );
 
     this._displayedQuery = results;
-    this._interpretation = interpretation;
 
     const panel = this.getPanel();
     await this.waitForPanelLoaded();
@@ -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(
+        const resultSetSchemas = await this.getResultSetSchemas(results);
-          results.query.resultsPaths.resultsPath,
+        const resultSetNames = resultSetSchemas.map(schema => schema.name);
-          RAW_RESULTS_PAGE_SIZE
-        );
-
-        const resultSetNames = schemas['result-sets'].map(
-          (resultSet) => resultSet.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) {
@@ -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 },
           selectedTable: undefined,
           resultSetNames,
         };
@@ -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
@@ -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> {
@@ -399,16 +442,10 @@ export class InterfaceManager extends DisposableObject {
         (sortedResultsMap[k] = this.convertPathPropertiesToWebviewUris(v))
     );
 
-    const schemas = await this.cliServer.bqrsInfo(
+    const resultSetSchemas = await this.getResultSetSchemas(results);
-      results.query.resultsPaths.resultsPath,
+    const resultSetNames = resultSetSchemas.map(schema => schema.name);
-      RAW_RESULTS_PAGE_SIZE
-    );
 
-    const resultSetNames = schemas['result-sets'].map(
+    const schema = resultSetSchemas.find(
-      (resultSet) => resultSet.name
-    );
-
-    const schema = schemas['result-sets'].find(
       (resultSet) => resultSet.name == selectedTable
     )!;
     if (schema === undefined)
@@ -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,
     };
@@ -447,7 +485,7 @@ export class InterfaceManager extends DisposableObject {
     });
   }
 
-  private async getTruncatedResults(
+  private async _getInterpretedResults(
     metadata: QueryMetadata | undefined,
     resultsPaths: ResultsPaths,
     sourceInfo: cli.SourceInfo | undefined,
@@ -460,37 +498,58 @@ export class InterfaceManager extends DisposableObject {
       resultsPaths,
       sourceInfo
     );
-    // For performance reasons, limit the number of results we try
+    sarif.runs.forEach(run => {
-    // to serialize and send to the webview. TODO: possibly also
+      if (run.results !== undefined)
-    // 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) {
         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
@@ -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,
@@ -522,7 +581,7 @@ export class InterfaceManager extends DisposableObject {
         );
       }
     }
-    return interpretation;
+    return this._interpretation && this.getPageOfInterpretedResults(0);
   }
 
   private async showResultsAsDiagnostics(
@@ -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,

extensions/ql-vscode/src/view/result-tables.tsx

@@ -50,9 +50,7 @@ function getResultCount(resultSet: ResultSet): number {
     case 'RawResultSet':
       return resultSet.schema.tupleCount;
     case 'SarifResultSet':
-      if (resultSet.sarif.runs.length === 0) return 0;
+      return resultSet.numTotalResults;
-      if (resultSet.sarif.runs[0].results === undefined) return 0;
-      return resultSet.sarif.runs[0].results.length + resultSet.numTruncatedResults;
   }
 }
 
@@ -110,22 +108,11 @@ export class ResultTables
     return this.props.parsedResultSets.t === 'ExtensionParsed';
   }
 
-  /**
-   * Holds if we actually should show pagination interface right now. This is
-   * still false for the time being when we're viewing alerts.
-   */
-  paginationEnabled(): boolean {
-    return this.paginationAllowed() &&
-      this.props.parsedResultSets.selectedTable !== ALERTS_TABLE_NAME &&
-      this.state.selectedTable !== ALERTS_TABLE_NAME;
-  }
-
   constructor(props: ResultTablesProps) {
     super(props);
-
     const selectedTable = props.parsedResultSets.selectedTable || getDefaultResultSet(this.getResultSets());
-
     let selectedPage: string;
+
     switch (props.parsedResultSets.t) {
       case 'ExtensionParsed':
         selectedPage = (props.parsedResultSets.pageNumber + 1) + '';
@@ -134,15 +121,13 @@ export class ResultTables
         selectedPage = '';
         break;
     }
-
     this.state = { selectedTable, selectedPage };
   }
 
   private onTableSelectionChange = (event: React.ChangeEvent<HTMLSelectElement>): void => {
     const selectedTable = event.target.value;
-    const fetchPageFromExtension = this.paginationAllowed() && selectedTable !== ALERTS_TABLE_NAME;
 
-    if (fetchPageFromExtension) {
+    if (this.paginationAllowed()) {
       vscode.postMessage({
         t: 'changePage',
         pageNumber: 0,
@@ -189,13 +174,22 @@ export class ResultTables
 
   renderPageButtons(resultSets: ExtensionParsedResultSets): JSX.Element {
     const selectedTable = this.state.selectedTable;
+
+    // FIXME: The extension, not the view, should be in charge of deciding whether to initially show
+    // a raw or alerts page. We have to conditionally recompute the number of pages here, because
+    // on initial load of query results, resultSets.numPages will have the number of *raw* pages available,
+    // not interpreted pages, because the extension doesn't know the view will default to showing alerts
+    // instead.
+    const numPages = selectedTable == ALERTS_TABLE_NAME ?
+      resultSets.numInterpretedPages : resultSets.numPages;
+
     const onChange = (e: React.ChangeEvent<HTMLInputElement>) => {
       this.setState({ selectedPage: e.target.value });
     };
     const choosePage = (input: string) => {
       const pageNumber = parseInt(input);
       if (pageNumber !== undefined && !isNaN(pageNumber)) {
-        const actualPageNumber = Math.max(0, Math.min(pageNumber - 1, resultSets.numPages - 1));
+        const actualPageNumber = Math.max(0, Math.min(pageNumber - 1, numPages - 1));
         vscode.postMessage({
           t: 'changePage',
           pageNumber: actualPageNumber,
@@ -214,22 +208,24 @@ export class ResultTables
     const nextPage = (e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => {
       vscode.postMessage({
         t: 'changePage',
-        pageNumber: Math.min(resultSets.pageNumber + 1, resultSets.numPages - 1),
+        pageNumber: Math.min(resultSets.pageNumber + 1, numPages - 1),
         selectedTable,
       });
     };
+
     return <span>
       <button onClick={prevPage} >&lt;</button>
-      <input value={this.state.selectedPage} onChange={onChange}
+      <input size={3} value={this.state.selectedPage} onChange={onChange}
         onBlur={e => choosePage(e.target.value)}
         onKeyDown={e => { if (e.keyCode === 13) choosePage((e.target as HTMLInputElement).value); }}
       />
+      / {numPages}
       <button value=">" onClick={nextPage} >&gt;</button>
     </span>;
   }
 
   renderButtons(): JSX.Element {
-    if (this.props.parsedResultSets.t === 'ExtensionParsed' && this.paginationEnabled())
+    if (this.props.parsedResultSets.t === 'ExtensionParsed' && this.paginationAllowed())
       return this.renderPageButtons(this.props.parsedResultSets);
     else
       return <span />;

extensions/ql-vscode/src/view/results.tsx

@@ -17,6 +17,7 @@ import {
   NavigatePathMsg,
   QueryMetadata,
   ResultsPaths,
+  ALERTS_TABLE_NAME,
 } from '../interface-types';
 import { EventHandlers as EventHandlerList } from './event-handler-list';
 import { ResultTables } from './result-tables';
@@ -193,6 +194,32 @@ class App extends React.Component<{}, ResultsViewState> {
           metadata: msg.metadata,
         });
 
+        this.loadResults();
+        break;
+      case 'showInterpretedPage':
+        this.updateStateWithNewResultsInfo({
+          resultsPath: '', // FIXME: Not used for interpreted, refactor so this is not needed
+          parsedResultSets: {
+            t: 'ExtensionParsed',
+            numPages: msg.numPages,
+            numInterpretedPages: msg.numPages,
+            resultSetNames: msg.resultSetNames,
+            pageNumber: msg.pageNumber,
+            resultSet: {
+              t: 'SarifResultSet',
+              name: ALERTS_TABLE_NAME,
+              schema: { name: ALERTS_TABLE_NAME, version: 0, columns: [], tupleCount: 1 },
+              ...msg.interpretation,
+            },
+            selectedTable: ALERTS_TABLE_NAME,
+          },
+          origResultsPaths: undefined as any, // FIXME: Not used for interpreted, refactor so this is not needed
+          sortedResultsMap: new Map(), // FIXME: Not used for interpreted, refactor so this is not needed
+          database: msg.database,
+          interpretation: msg.interpretation,
+          shouldKeepOldResultsWhileRendering: true,
+          metadata: msg.metadata,
+        });
         this.loadResults();
         break;
       case 'resultsUpdating':
@@ -342,8 +369,10 @@ class App extends React.Component<{}, ResultsViewState> {
       displayedResults.resultsInfo !== null
     ) {
       const parsedResultSets = displayedResults.resultsInfo.parsedResultSets;
+      const key = (parsedResultSets.t === 'ExtensionParsed' ? (parsedResultSets.selectedTable || '') + parsedResultSets.pageNumber : '');
       return (
         <ResultTables
+          key={key}
           parsedResultSets={parsedResultSets}
           rawResultSets={displayedResults.results.resultSets}
           interpretation={