Merge pull request #1604 from github/dbartol/join-order-threshold

Make bad join order warning threshold configurable

Author: Dave Bartolomeo

extensions/ql-vscode/package.json

@@ -290,6 +290,13 @@
           "pattern": "^$|^(?:[a-zA-Z0-9]+-)*[a-zA-Z0-9]+/[a-zA-Z0-9-_]+$",
           "patternErrorMessage": "Please enter a valid GitHub repository",
           "markdownDescription": "[For internal use only] The name of the GitHub repository in which the GitHub Actions workflow is run when using the \"Run Variant Analysis\" command. The repository should be of the form `<owner>/<repo>`)."
+        },
+        "codeQL.logInsights.joinOrderWarningThreshold": {
+          "type": "number",
+          "default": 50,
+          "scope": "window",
+          "minimum": 0,
+          "description": "Report a warning for any join order whose metric exceeds this value."
         }
       }
     },

extensions/ql-vscode/src/config.ts

@@ -59,6 +59,7 @@ const ROOT_SETTING = new Setting('codeQL');
 const TELEMETRY_SETTING = new Setting('telemetry', ROOT_SETTING);
 const AST_VIEWER_SETTING = new Setting('astViewer', ROOT_SETTING);
 const GLOBAL_TELEMETRY_SETTING = new Setting('telemetry');
+const LOG_INSIGHTS_SETTING = new Setting('logInsights', ROOT_SETTING);
 
 export const LOG_TELEMETRY = new Setting('logTelemetry', TELEMETRY_SETTING);
 export const ENABLE_TELEMETRY = new Setting('enableTelemetry', TELEMETRY_SETTING);
@@ -342,6 +343,11 @@ export function allowCanaryQueryServer() {
   return !!CANARY_QUERY_SERVER.getValue<boolean>();
 }
 
+export const JOIN_ORDER_WARNING_THRESHOLD = new Setting('joinOrderWarningThreshold', LOG_INSIGHTS_SETTING);
+
+export function joinOrderWarningThreshold(): number {
+  return JOIN_ORDER_WARNING_THRESHOLD.getValue<number>();
+}
 
 /**
  * Avoids caching in the AST viewer if the user is also a canary user.

extensions/ql-vscode/src/extension.ts

@@ -33,6 +33,7 @@ import {
   CliConfigListener,
   DistributionConfigListener,
   isCanary,
+  joinOrderWarningThreshold,
   MAX_QUERIES,
   QueryHistoryConfigListener,
   QueryServerConfigListener
@@ -518,7 +519,7 @@ async function activateWithInstalledDistribution(
   void logger.log('Initializing evaluation log scanners.');
   const logScannerService = new LogScannerService(qhm);
   ctx.subscriptions.push(logScannerService);
-  ctx.subscriptions.push(logScannerService.scanners.registerLogScannerProvider(new JoinOrderScannerProvider()));
+  ctx.subscriptions.push(logScannerService.scanners.registerLogScannerProvider(new JoinOrderScannerProvider(() => joinOrderWarningThreshold())));
 
   void logger.log('Reading query history');
   await qhm.readQueryHistory();

extensions/ql-vscode/src/log-insights/join-order.ts

@@ -2,8 +2,6 @@ import * as I from 'immutable';
 import { EvaluationLogProblemReporter, EvaluationLogScanner, EvaluationLogScannerProvider } from './log-scanner';
 import { InLayer, ComputeRecursive, SummaryEvent, PipelineRun, ComputeSimple } from './log-summary';
 
-const DEFAULT_WARNING_THRESHOLD = 50;
-
 /**
  * Like `max`, but returns 0 if no meaningful maximum can be computed.
  */
@@ -454,7 +452,11 @@ class JoinOrderScanner implements EvaluationLogScanner {
 }
 
 export class JoinOrderScannerProvider implements EvaluationLogScannerProvider {
+  constructor(private readonly getThreshdold: () => number) {
+  }
+
   public createScanner(problemReporter: EvaluationLogProblemReporter): EvaluationLogScanner {
-    return new JoinOrderScanner(problemReporter, DEFAULT_WARNING_THRESHOLD);
+    const threshold = this.getThreshdold();
+    return new JoinOrderScanner(problemReporter, threshold);
   }
 }

extensions/ql-vscode/test/pure-tests/log-scanner.test.ts

@@ -31,7 +31,7 @@ class TestProblemReporter implements EvaluationLogProblemReporter {
 describe('log scanners', function() {
   it('should detect bad join orders', async function() {
     const scanners = new EvaluationLogScannerSet();
-    scanners.registerLogScannerProvider(new JoinOrderScannerProvider());
+    scanners.registerLogScannerProvider(new JoinOrderScannerProvider(() => 50));
     const summaryPath = path.join(__dirname, 'evaluator-log-summaries/bad-join-order.jsonl');
     const problemReporter = new TestProblemReporter();
     await scanners.scanLog(summaryPath, problemReporter);