From 16bdc6578181b48e9a1ae3203eefc10dab536106 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Sirois Date: Wed, 15 Jul 2026 10:42:53 -0400 Subject: [PATCH] revert: keep pushing non-DML statements from pg_stat_statements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 80af5fb453e3a0df36b76dd36b7d9320d67cc070 (#179). #179 stopped the migration-day alert spam by dropping statementType === "other" at both push sites, so DDL, COPY and VACUUM never left the analyzer. It quiets the noise, but by discarding the events at capture: QD never ingests them, so nothing downstream can use them — an index dropped in prod, a schema-changing migration, correlating a later CI regression against a schema change, an MCP "recent events" feed. None of that is possible once the event is dropped before it reaches the catalog. Suppressing the alert does not require dropping the data at capture. That is an alerting-layer decision (scope and cadence), so ingestion should stay complete. This restores full ingestion; the migration-day alert spam returns until the alerting-side handling lands (Query-Doctor/Site#3581). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/remote/api-client.ts | 14 +------------- src/sql/recent-query.test.ts | 32 -------------------------------- src/sql/recent-query.ts | 4 +--- 3 files changed, 2 insertions(+), 48 deletions(-) diff --git a/src/remote/api-client.ts b/src/remote/api-client.ts index 9f048ea..6231910 100644 --- a/src/remote/api-client.ts +++ b/src/remote/api-client.ts @@ -33,24 +33,12 @@ export function hookUpApiReporter(api: RpcStub, remote: Remote): () = log.error(`Failed to push stats: ${err}`, "api-client"); }); }; - // pg_stat_statements retains everything the database ever executed — COPY - // from dumps/seeds, DDL from migrations, VACUUM. None of it is an app query: - // shipping it pollutes the project's query catalog and fires unfixable - // "no CI coverage" alerts (Query-Doctor/Site#3578). Only DML is pushed. - // `undefined` (analysis skipped or failed) is pushed too — fail open rather - // than silently losing real queries. Known gap: core classifies MergeStmt as - // "other", so MERGE statements are dropped until its map learns them. - const isAppQuery = (query: RecentQuery) => query.statementType !== "other"; - const onQueriesPolled = (queries: RecentQuery[]) => { - const appQueries = queries.filter(isAppQuery); - if (appQueries.length === 0) return; - api.pushQuery(JSON.parse(JSON.stringify(appQueries))).catch((err) => { + api.pushQuery(JSON.parse(JSON.stringify(queries))).catch((err) => { log.error(`Failed to push polled queries: ${err}`, "api-client"); }); }; const pushOptimizedQuery = (query: OptimizedQuery) => { - if (!isAppQuery(query)) return; const q = [query.toJSON()] api.pushQuery(q).catch((err) => { log.error(`Failed to push optimized query: ${err}`, "api-client"); diff --git a/src/sql/recent-query.test.ts b/src/sql/recent-query.test.ts index 96a431a..6a1b82e 100644 --- a/src/sql/recent-query.test.ts +++ b/src/sql/recent-query.test.ts @@ -259,38 +259,6 @@ test("analyze sets isSelectQuery=false for DELETE with EXISTS subquery", async ( expect(rq.isSelectQuery).toBe(false); }); -// --- statementType persistence (push sites drop "other" — see api-client) --- - -test("analyze keeps statementType on the instance for DML", async () => { - const data = makeRawQuery({ query: "SELECT * FROM users" }); - const rq = await RecentQuery.analyze(data, testHash, testNormalizedHash); - expect(rq.statementType).toBe("select"); -}); - -test("analyze classifies DDL as other", async () => { - const data = makeRawQuery({ - query: - 'CREATE INDEX "ci_runs_repo_created_at_idx" ON "ci_runs" USING btree ("repo", "created_at" DESC)', - }); - const rq = await RecentQuery.analyze(data, testHash, testNormalizedHash); - expect(rq.statementType).toBe("other"); -}); - -test("analyze classifies COPY as other", async () => { - const data = makeRawQuery({ - query: "COPY public.project_queries (id, hash) FROM stdin", - }); - const rq = await RecentQuery.analyze(data, testHash, testNormalizedHash); - expect(rq.statementType).toBe("other"); -}); - -test("oversized queries skip analysis and carry no statementType", async () => { - const data = makeRawQuery({ query: `SELECT ${"1,".repeat(30_000)} 1` }); - const rq = await RecentQuery.analyze(data, testHash, testNormalizedHash); - expect(rq.analysisSkipped).toBe(true); - expect(rq.statementType).toBeUndefined(); -}); - // --- displayQuery via analyze --- test("analyze populates displayQuery for wide SELECTs", async () => { diff --git a/src/sql/recent-query.ts b/src/sql/recent-query.ts index c36d22b..e67db76 100644 --- a/src/sql/recent-query.ts +++ b/src/sql/recent-query.ts @@ -49,9 +49,7 @@ export class RecentQuery { readonly hash: QueryHash, readonly normalizedHash: QueryHash, analysisSkipped = false, - // Kept on the instance so push sites can drop non-DML (see api-client); - // undefined when analysis was skipped (oversized) or failed. - readonly statementType?: StatementType, + statementType?: StatementType, ) { this.username = data.username; this.query = data.query;