From 0b74366425014bdce60519ac7e80fa86c6b16d07 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Sirois Date: Tue, 28 Jul 2026 20:00:56 -0300 Subject: [PATCH] feat: model CI queries at the repo's configured statistics scale Implements Query-Doctor/Site#3119. A repo can set a statistics scale in CI settings, and nothing read it. The analyzer now passes it to the stats mode it builds, so core plans every query against that multiple of the table and index sizes. The scale rides inside the statisticsMode already posted to the Site, which stamps it on the run. A scale of 1 is left off the mode, so a repo at the default size posts exactly what it posted before. Adopts @query-doctor/core ^0.18.0, the first version whose StatisticsMode carries scale. Co-Authored-By: Claude --- package-lock.json | 8 ++++---- package.json | 2 +- src/config.test.ts | 18 ++++++++++++++++- src/config.ts | 23 +++++++++++++++++++++ src/main.ts | 11 +++++++++- src/runner.test.ts | 50 ++++++++++++++++++++++++++++++++++++++++++++++ src/runner.ts | 27 ++++++++++++++++++------- 7 files changed, 125 insertions(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index 553f20b..37ef7f0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,7 +16,7 @@ "@libpg-query/parser": "^17.6.3", "@opentelemetry/api": "^1.9.0", "@pgsql/types": "^17.6.2", - "@query-doctor/core": "^0.17.0", + "@query-doctor/core": "^0.18.0", "async-sema": "^3.1.1", "capnweb": "^0.7.0", "dedent": "^1.7.1", @@ -1193,9 +1193,9 @@ "license": "BSD-3-Clause" }, "node_modules/@query-doctor/core": { - "version": "0.17.0", - "resolved": "https://registry.npmjs.org/@query-doctor/core/-/core-0.17.0.tgz", - "integrity": "sha512-WN0so5G6dqmOx0v7NGw58nTbTlIXdX/eTHRIyxSP/uBLPGfYPmTna3B+o4s2KixA7uVez5uaCM//1KhJ36r/Ww==", + "version": "0.18.0", + "resolved": "https://registry.npmjs.org/@query-doctor/core/-/core-0.18.0.tgz", + "integrity": "sha512-Z3ldmkUv92U2vxw9OI8J4eOn/MUxTMheqcRy+HJuCeMAcZGj2Sbv4XB60TzhtnLIbMt4EBCK/NysLeGxjlWm8w==", "dependencies": { "@pgsql/types": "^17.6.2", "capnweb": "^0.10.0", diff --git a/package.json b/package.json index 2bd0eb9..c788625 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "@libpg-query/parser": "^17.6.3", "@opentelemetry/api": "^1.9.0", "@pgsql/types": "^17.6.2", - "@query-doctor/core": "^0.17.0", + "@query-doctor/core": "^0.18.0", "async-sema": "^3.1.1", "capnweb": "^0.7.0", "dedent": "^1.7.1", diff --git a/src/config.test.ts b/src/config.test.ts index b57a888..b1c943a 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -1,5 +1,5 @@ import { test, expect, vi } from "vitest"; -import { DEFAULT_CONFIG } from "./config.ts"; +import { configuredStatisticsScale, DEFAULT_CONFIG } from "./config.ts"; import type { ServerApi } from "@query-doctor/core"; import type { RpcStub } from "capnweb"; @@ -55,3 +55,19 @@ test("passes repo and branch to getRepoConfig", async () => { await resolveConfig(api, "org/repo", "feat/my-branch"); expect(getRepoConfig).toHaveBeenCalledWith("org/repo", "feat/my-branch"); }); + +test("reads the scale the repo is configured at", () => { + expect(configuredStatisticsScale({ ...DEFAULT_CONFIG, statisticsScale: 10 })) + .toBe(10); +}); + +test("falls back to the current data size when the repo config has no scale", () => { + // A Site that predates the setting, or a run with no repo config at all. + expect(configuredStatisticsScale(DEFAULT_CONFIG)).toBe(1); + expect(configuredStatisticsScale(undefined)).toBe(1); +}); + +test("ignores a scale the Site could not have meant", () => { + expect(configuredStatisticsScale({ statisticsScale: 0 })).toBe(1); + expect(configuredStatisticsScale({ statisticsScale: "10" })).toBe(1); +}); diff --git a/src/config.ts b/src/config.ts index bfdc801..76a7959 100644 --- a/src/config.ts +++ b/src/config.ts @@ -13,6 +13,12 @@ export interface AnalyzerConfig { * it through `getRepoConfig`; the gate honours it the moment it arrives. */ conditionPolicies?: RepoPolicyConfig; + /** + * The multiple of the current data the repo asks its queries to be planned + * against (#3119). 1 is the current size, which is what a repo gets until + * someone raises it in CI settings. + */ + statisticsScale?: number; } export const DEFAULT_CONFIG: AnalyzerConfig = { @@ -20,4 +26,21 @@ export const DEFAULT_CONFIG: AnalyzerConfig = { regressionThreshold: 0, ignoredQueryHashes: [], acknowledgedQueryHashes: [], + statisticsScale: 1, }; + +/** + * The scale a repo is configured at, read off whatever the relay returned. + * + * The Site has sent `statisticsScale` on the repo config since #3115, but + * core's `RepoConfig` type does not declare it, so the read is defensive and + * lives in this one place. A missing, non-numeric, or sub-1 value means the + * current data size: an analyzer talking to an older Site keeps working, and a + * nonsense value never reaches the planner. Drop the cast once core ships the + * field on `RepoConfig`. + */ +export function configuredStatisticsScale(config: unknown): number { + const scale = (config as { statisticsScale?: unknown } | null | undefined) + ?.statisticsScale; + return typeof scale === "number" && scale >= 1 ? scale : 1; +} diff --git a/src/main.ts b/src/main.ts index 33b526e..4cf1f77 100644 --- a/src/main.ts +++ b/src/main.ts @@ -21,7 +21,7 @@ import { gateRegression } from "./gate/regression.ts"; import { gateNewQuery } from "./gate/new-query.ts"; import { gateSchemaChange } from "./gate/schema-change.ts"; import { resolveVerdict } from "./gate/policy.ts"; -import { DEFAULT_CONFIG } from "./config.ts"; +import { configuredStatisticsScale, DEFAULT_CONFIG } from "./config.ts"; import { ApiClient } from "./remote/api-client.ts"; import { Remote } from "./remote/remote.ts"; import { ConnectionManager } from "./sync/connection-manager.ts"; @@ -99,6 +99,14 @@ async function runInCI( ? new PgbadgerSource(logPath) : remoteDbManager.getConnectorFor(sourcePostgresUrl); + const statisticsScale = configuredStatisticsScale(config); + if (statisticsScale !== 1) { + log.info( + `Modeling queries at ${statisticsScale}x the current data size`, + "main", + ); + } + const runner = await Runner.build({ targetPostgresUrl, sourcePostgresUrl, @@ -107,6 +115,7 @@ async function runInCI( ignoredQueryHashes: config.ignoredQueryHashes, remote, productionStats: productionStats ?? undefined, + statisticsScale, }); let allResults: QueryProcessResult[]; let reportContext; diff --git a/src/runner.test.ts b/src/runner.test.ts index dacb7f0..28ffc5e 100644 --- a/src/runner.test.ts +++ b/src/runner.test.ts @@ -80,3 +80,53 @@ describe("Runner.determineStatsMode precedence", () => { expect(await Runner.determineStatsMode()).toEqual(syntheticMode); }); }); + +describe("Runner.determineStatsMode at a configured scale", () => { + const TABLE: ExportedStats = { + tableName: "users", + schemaName: "public", + relpages: 10, + reltuples: 166_000, + relallvisible: 8, + columns: [], + indexes: [], + }; + + /** CI always resolves a static mode; this narrows the union to read it. */ + async function staticMode( + productionStats?: ExportedStats[], + scale?: number, + ) { + const strategy = await Runner.determineStatsMode( + undefined, + productionStats, + scale, + ); + if (strategy.type !== "static") { + throw new Error(`expected a static stats mode, got ${strategy.type}`); + } + return strategy.stats; + } + + test("models production stats at the repo's scale", async () => { + const stats = await staticMode([TABLE], 10); + + expect(stats.kind).toBe("fromStatisticsExport"); + expect(stats.scale).toBe(10); + }); + + test("models the synthetic assumption at the repo's scale", async () => { + const stats = await staticMode(undefined, 1000); + + expect(stats.kind).toBe("fromAssumption"); + expect(stats.scale).toBe(1000); + }); + + test("leaves the mode unscaled when the repo is configured at 1x", async () => { + expect((await staticMode([TABLE], 1)).scale).toBeUndefined(); + }); + + test("leaves the mode unscaled when no scale is configured", async () => { + expect((await staticMode([TABLE])).scale).toBeUndefined(); + }); +}); diff --git a/src/runner.ts b/src/runner.ts index c69d8f2..61283fb 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -14,7 +14,7 @@ import { Connectable } from "./sync/connectable.ts"; import { Remote, StatisticsStrategy } from "./remote/remote.ts"; import { ConnectionManager } from "./sync/connection-manager.ts"; import type { OptimizedQuery } from "./sql/recent-query.ts"; -import { ExportedStats, Statistics } from "@query-doctor/core"; +import { ExportedStats, Statistics, type StatisticsMode } from "@query-doctor/core"; import { readFile } from "node:fs/promises"; import { buildQueries } from "./reporters/site-api.ts"; @@ -37,6 +37,10 @@ export class Runner { // Real production statistics pulled from the Site API. When present, queries // are costed against true prod cardinality instead of synthetic assumptions. productionStats?: ExportedStats[]; + // The repo's configured statistics scale. Queries are then planned against + // this multiple of the data, which answers whether they hold up at a size + // the database hasn't reached yet. + statisticsScale?: number; }) { const remote = options.remote ?? new Remote( options.targetPostgresUrl, @@ -45,7 +49,7 @@ export class Runner { { disableQueryLoader: true } ); await remote.syncFrom(options.sourcePostgresUrl, - await Runner.determineStatsMode(options.statisticsPath, options.productionStats) + await Runner.determineStatsMode(options.statisticsPath, options.productionStats, options.statisticsScale) ); await remote.optimizer.finish; return new Runner( @@ -59,14 +63,23 @@ export class Runner { // Stats-mode precedence for CI: real production stats pulled from the Site API // win, then an explicit stats file, then synthetic assumptions. CI never dumps // stats from the ephemeral target database itself. + // + // `scale` applies to whichever mode wins: core multiplies the planner's view + // of table and index size by it and leaves column statistics alone. A scale of + // 1 is left off the mode entirely, so a repo at the default size posts exactly + // the payload it did before. static async determineStatsMode( statsPath?: string, productionStats?: ExportedStats[], + scale?: number, ): Promise { + const atScale = (stats: T): T => + scale && scale !== 1 ? { ...stats, scale } : stats; + if (productionStats && productionStats.length > 0) { return { type: "static", - stats: Statistics.statsModeFromExport(productionStats), + stats: atScale(Statistics.statsModeFromExport(productionStats)), }; } if (statsPath) { @@ -75,20 +88,20 @@ export class Runner { const stats = ExportedStats.array().parse(rawStats); return { type: "static", - stats: { + stats: atScale({ kind: "fromStatisticsExport", source: { kind: "path", path: statsPath }, stats - } + }) } } return { type: "static", - stats: { + stats: atScale({ kind: "fromAssumption", reltuples: 10_000_000, - } + }) } }