Skip to content

Commit 9b0d4bd

Browse files
author
Dave Bartolomeo
committed
Make bad join order warning threshold configurable
The threshold at which the bad join order detection reports a warning was previously hard-coded to 50. Initial feedback from internal QL developers suggests that this is too high, and should be configurable in any case. I've made it configurable via the `codeQL.logInsights.joinOrderWarningThreshold` setting, leaving the default at 50. Once we get more feedback about what a better default value is, I'll update the default.
1 parent 7bdd452 commit 9b0d4bd

3 files changed

Lines changed: 16 additions & 3 deletions

File tree

extensions/ql-vscode/package.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,13 @@
290290
"pattern": "^$|^(?:[a-zA-Z0-9]+-)*[a-zA-Z0-9]+/[a-zA-Z0-9-_]+$",
291291
"patternErrorMessage": "Please enter a valid GitHub repository",
292292
"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>`)."
293+
},
294+
"codeQL.logInsights.joinOrderWarningThreshold": {
295+
"type": "number",
296+
"default": 50,
297+
"scope": "window",
298+
"minimum": 0,
299+
"description": "Report a warning for any join order whose metric exceeds this value."
293300
}
294301
}
295302
},

extensions/ql-vscode/src/config.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ const ROOT_SETTING = new Setting('codeQL');
4444
const TELEMETRY_SETTING = new Setting('telemetry', ROOT_SETTING);
4545
const AST_VIEWER_SETTING = new Setting('astViewer', ROOT_SETTING);
4646
const GLOBAL_TELEMETRY_SETTING = new Setting('telemetry');
47+
const LOG_INSIGHTS_SETTING = new Setting('logInsights', ROOT_SETTING);
4748

4849
export const LOG_TELEMETRY = new Setting('logTelemetry', TELEMETRY_SETTING);
4950
export const ENABLE_TELEMETRY = new Setting('enableTelemetry', TELEMETRY_SETTING);
@@ -327,6 +328,11 @@ export function allowCanaryQueryServer() {
327328
return !!CANARY_QUERY_SERVER.getValue<boolean>();
328329
}
329330

331+
export const JOIN_ORDER_WARNING_THRESHOLD = new Setting('joinOrderWarningThreshold', LOG_INSIGHTS_SETTING);
332+
333+
export function joinOrderWarningThreshold(): number {
334+
return JOIN_ORDER_WARNING_THRESHOLD.getValue<number>();
335+
}
330336

331337
/**
332338
* Avoids caching in the AST viewer if the user is also a canary user.

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import * as I from 'immutable';
2+
import { joinOrderWarningThreshold } from '../config';
23
import { EvaluationLogProblemReporter, EvaluationLogScanner, EvaluationLogScannerProvider } from './log-scanner';
34
import { InLayer, ComputeRecursive, SummaryEvent, PipelineRun, ComputeSimple } from './log-summary';
45

5-
const DEFAULT_WARNING_THRESHOLD = 50;
6-
76
/**
87
* Like `max`, but returns 0 if no meaningful maximum can be computed.
98
*/
@@ -455,6 +454,7 @@ class JoinOrderScanner implements EvaluationLogScanner {
455454

456455
export class JoinOrderScannerProvider implements EvaluationLogScannerProvider {
457456
public createScanner(problemReporter: EvaluationLogProblemReporter): EvaluationLogScanner {
458-
return new JoinOrderScanner(problemReporter, DEFAULT_WARNING_THRESHOLD);
457+
const threshold = joinOrderWarningThreshold();
458+
return new JoinOrderScanner(problemReporter, threshold);
459459
}
460460
}

0 commit comments

Comments
 (0)