From f733454e6542da474910d887cba1019840cd902a Mon Sep 17 00:00:00 2001 From: Jean-Philippe Sirois Date: Thu, 27 Aug 2026 21:16:15 -0300 Subject: [PATCH 1/3] fix(sync): install pg_stat_statements into a named schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `installPgStatStatements()` issued a bare `CREATE EXTENSION`, which lands in `public`. The extension owns views there that a migration tool reconciling `public` cannot drop, so the tool aborts half-applied with SQLSTATE 2BP01 and the failure surfaces far from the extension. The install now takes a schema, defaulting to `query_doctor`, and the verify probe resolves the schema through `getQuerySource()` instead of reading an unqualified name through the `search_path` — which returns 42P01 for exactly the placement the install now produces. Co-Authored-By: Claude Opus 5 (1M context) --- src/remote/remote.dto.ts | 23 ++++++ src/server/http.ts | 13 +++- src/sync/pg-connector.test.ts | 143 +++++++++++++++++++++++++++++++++- src/sync/pg-connector.ts | 59 +++++++++++++- 4 files changed, 230 insertions(+), 8 deletions(-) diff --git a/src/remote/remote.dto.ts b/src/remote/remote.dto.ts index f87675f5..5661d014 100644 --- a/src/remote/remote.dto.ts +++ b/src/remote/remote.dto.ts @@ -17,6 +17,29 @@ export const RemoteSyncRequest = z.codec( }, ); +/** + * `schema` is where the extension gets created. Omitting it takes the + * connector's default, which is deliberately not `public`. + */ +export const InstallPgStatStatementsRequest = z.codec( + z.string(), + z.object({ + db: z.custom(), + schema: z.string().optional(), + }), + { + encode: (value) => + JSON.stringify({ db: value.db.toString(), schema: value.schema }), + decode: (value) => { + const parsed = JSON.parse(value); + return { + db: Connectable.fromString(parsed.db), + schema: typeof parsed.schema === "string" ? parsed.schema : undefined, + }; + }, + }, +); + export const RemoteSyncFullSchemaResponse = z.discriminatedUnion("type", [ z.object({ type: z.literal("ok"), value: z.custom() }), z.object({ type: z.literal("error"), error: z.string() }), diff --git a/src/server/http.ts b/src/server/http.ts index 3aa36e3c..9d08a8e8 100644 --- a/src/server/http.ts +++ b/src/server/http.ts @@ -12,7 +12,10 @@ import { SyncResult } from "../sync/syncer.ts"; import * as errors from "../sync/errors.ts"; import { RemoteController } from "../remote/remote-controller.ts"; import { Connectable } from "../sync/connectable.ts"; -import { RemoteSyncRequest } from "../remote/remote.dto.ts"; +import { + InstallPgStatStatementsRequest, + RemoteSyncRequest, +} from "../remote/remote.dto.ts"; import { ConnectionManager } from "../sync/connection-manager.ts"; import { Remote } from "../remote/remote.ts"; @@ -170,14 +173,18 @@ export async function createServer( fastify.post("/postgres/extensions/pg_stat_statements", async (request, reply) => { log.info(`[POST] /postgres/extensions/pg_stat_statements`, "http"); - const body = RemoteSyncRequest.safeDecode(JSON.stringify(request.body)); + const body = InstallPgStatStatementsRequest.safeDecode( + JSON.stringify(request.body), + ); if (!body.success) { return reply.status(400).send(body.error); } try { const db = await body.data.db.resolveDockerHost(); const connector = sourceConnectionManager.getConnectorFor(db); - const result = await connector.installPgStatStatements(); + const result = await connector.installPgStatStatements({ + schema: body.data.schema, + }); return reply.status(200).send({ success: true, ...result }); } catch (error) { return reply.status(500).send(makeUnexpectedErrorResult(error).body); diff --git a/src/sync/pg-connector.test.ts b/src/sync/pg-connector.test.ts index d5ed21b0..cb41121d 100644 --- a/src/sync/pg-connector.test.ts +++ b/src/sync/pg-connector.test.ts @@ -1,4 +1,4 @@ -import { test, expect } from "vitest"; +import { test, expect, vi } from "vitest"; import { PostgreSqlContainer } from "@testcontainers/postgresql"; import { ConnectionManager } from "./connection-manager.ts"; import { Connectable } from "./connectable.ts"; @@ -312,3 +312,144 @@ test("getTotalRowCount does not double-count when a table appears twice in the i await pg.stop(); } }); + +// Installing into `public` breaks any project whose migration tool reconciles +// that schema: the extension owns `pg_stat_statements_info`, and a reconciler +// that tries to drop it aborts half-applied with SQLSTATE 2BP01. The one path +// where we choose the placement must not choose `public`. +test("installPgStatStatements creates the extension outside public", async () => { + const pg = await new PostgreSqlContainer("postgres:17") + .withCommand(["-c", "shared_preload_libraries=pg_stat_statements"]) + .start(); + + const manager = ConnectionManager.forLocalDatabase(); + const conn = Connectable.fromString(pg.getConnectionUri()); + const db = manager.getOrCreateConnection(conn); + const connector = manager.getConnectorFor(db); + + try { + const result = await connector.installPgStatStatements(); + + expect(result.schema).toBe("query_doctor"); + + const [placement] = await db.exec<{ schema: string }>(` + SELECT n.nspname AS schema + FROM pg_extension e + JOIN pg_namespace n ON n.oid = e.extnamespace + WHERE e.extname = 'pg_stat_statements' + `); + expect(placement?.schema).toBe("query_doctor"); + + const [leftInPublic] = await db.exec<{ count: string }>(` + SELECT count(*) AS count + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = 'public' AND c.relname LIKE 'pg_stat_statements%' + `); + expect(leftInPublic?.count).toBe("0"); + } finally { + await manager.closeAll(); + await pg.stop(); + } +}); + +test("installPgStatStatements installs into a caller-supplied schema", async () => { + const pg = await new PostgreSqlContainer("postgres:17") + .withCommand(["-c", "shared_preload_libraries=pg_stat_statements"]) + .start(); + + const manager = ConnectionManager.forLocalDatabase(); + const conn = Connectable.fromString(pg.getConnectionUri()); + const db = manager.getOrCreateConnection(conn); + const connector = manager.getConnectorFor(db); + + try { + const result = await connector.installPgStatStatements({ schema: "ext" }); + + expect(result.schema).toBe("ext"); + + const [placement] = await db.exec<{ schema: string }>(` + SELECT n.nspname AS schema + FROM pg_extension e + JOIN pg_namespace n ON n.oid = e.extnamespace + WHERE e.extname = 'pg_stat_statements' + `); + expect(placement?.schema).toBe("ext"); + } finally { + await manager.closeAll(); + await pg.stop(); + } +}); + +// The verify step used to probe an unqualified `pg_stat_statements`, which +// resolves through the search_path and so returns 42P01 for exactly the +// placement the install step now produces: a working install reported as a +// failure. +test("installPgStatStatements verifies an extension that sits off the search_path", async () => { + const pg = await new PostgreSqlContainer("postgres:17") + .withCopyContentToContainer([ + { + content: ` + CREATE SCHEMA monitoring; + CREATE EXTENSION pg_stat_statements SCHEMA monitoring; + `, + target: "/docker-entrypoint-initdb.d/init.sql", + }, + ]) + .withCommand(["-c", "shared_preload_libraries=pg_stat_statements"]) + .start(); + + const manager = ConnectionManager.forLocalDatabase(); + const conn = Connectable.fromString(pg.getConnectionUri()); + const connector = manager.getConnectorFor(conn); + + try { + const result = await connector.installPgStatStatements(); + + expect(result.schema).toBe("monitoring"); + } finally { + await manager.closeAll(); + await pg.stop(); + } +}); + +// Today the user finds out about a `public` install through unrelated migration +// failures. The resolver already knows, so say it where we read the extension. +test("getRecentQueries warns when the extension sits in public", async () => { + const pg = await new PostgreSqlContainer("postgres:17") + .withCopyContentToContainer([ + { + content: ` + CREATE EXTENSION pg_stat_statements; + CREATE TABLE users(id int, name text); + SELECT * FROM users WHERE id = 1; + `, + target: "/docker-entrypoint-initdb.d/init.sql", + }, + ]) + .withCommand(["-c", "shared_preload_libraries=pg_stat_statements"]) + .start(); + + const manager = ConnectionManager.forLocalDatabase(); + const conn = Connectable.fromString(pg.getConnectionUri()); + const connector = manager.getConnectorFor(conn); + const logged = vi.spyOn(console, "log").mockImplementation(() => {}); + + try { + await connector.getRecentQueries(); + + const messages = logged.mock.calls.map((call) => String(call[0])); + expect( + messages.some((message) => + message.includes("pg_stat_statements") && + message.includes("public") && + message.includes("SET SCHEMA") + ), + `Expected a warning naming the fix. Logged:\n${messages.join("\n")}`, + ).toBe(true); + } finally { + logged.mockRestore(); + await manager.closeAll(); + await pg.stop(); + } +}); diff --git a/src/sync/pg-connector.ts b/src/sync/pg-connector.ts index b38da958..1689c2ae 100644 --- a/src/sync/pg-connector.ts +++ b/src/sync/pg-connector.ts @@ -83,8 +83,15 @@ export type ResetPgStatStatementsResult = */ export class PostgresConnector implements DatabaseConnector, RecentQuerySource { private static readonly QUERY_DOCTOR_USER = "query_doctor_db_link"; + /** + * Where {@link installPgStatStatements} puts the extension when the caller + * doesn't say. Never `public`: the extension owns views a migration tool + * reconciling `public` will try to drop, and fail on. + */ + public static readonly EXTENSION_SCHEMA = "query_doctor"; private readonly tupleEstimates = new Map(); private querySource: QuerySourceExtension | null = null; + private warnedAboutPublicSchema = false; private static extensionNotInstalledError = new ExtensionNotInstalledError([ "pg_stat_statements", "pg_stat_monitor" @@ -503,9 +510,27 @@ ORDER BY extensionName: PgIdentifier.fromString(firstResult.extension), schema: PgIdentifier.fromString(firstResult.schema) }; + this.warnIfExtensionIsInPublic(this.querySource); return this.querySource; } + /** + * An extension in `public` is a landmine for any migration tool that + * reconciles that schema, and nothing at the point of failure names it. We + * resolve the schema on every read, so this is the one place that knows. + */ + private warnIfExtensionIsInPublic(source: QuerySourceExtension): void { + if (source.schema.unquoted() !== "public" || this.warnedAboutPublicSchema) { + return; + } + this.warnedAboutPublicSchema = true; + const extension = source.extensionName.unquoted(); + log.warn( + `${extension} is installed in the public schema. A migration tool that reconciles public cannot drop its extension-owned views and will abort mid-run (SQLSTATE 2BP01). Move it with: CREATE SCHEMA IF NOT EXISTS ${PostgresConnector.EXTENSION_SCHEMA}; ALTER EXTENSION ${extension} SET SCHEMA ${PostgresConnector.EXTENSION_SCHEMA};`, + "postgres", + ); + } + /** * Get the latest queries using pg_stat_statements * @throws {ExtensionNotInstalledError} - pg_stat_statements is not installed @@ -578,8 +603,25 @@ ORDER BY } } - public async installPgStatStatements(): Promise<{ preloadUpdated: boolean }> { + /** + * Installs `pg_stat_statements` into {@link options.schema}, defaulting to + * {@link PostgresConnector.EXTENSION_SCHEMA}. + * + * An extension already installed elsewhere is left where it is — relocating + * it needs an ownership we may not hold, and moving a schema object out from + * under whoever put it there is not ours to decide. The resolver warns about + * a `public` placement on every read instead. + * + * @throws {PostgresError} + * @throws {ExtensionNotInstalledError} - the install ran but left nothing behind + */ + public async installPgStatStatements( + options: { schema?: string } = {}, + ): Promise<{ preloadUpdated: boolean; schema: string }> { let preloadUpdated = false; + const targetSchema = PgIdentifier.fromString( + options.schema ?? PostgresConnector.EXTENSION_SCHEMA, + ); const [preload] = await this.db.exec<{ setting: string }>(` SELECT setting FROM pg_settings WHERE name = 'shared_preload_libraries'; -- @qd_introspection @@ -603,19 +645,28 @@ ORDER BY `); if (!result?.exists) { try { - await this.db.exec(`CREATE EXTENSION pg_stat_statements;`); + await this.db.exec(`CREATE SCHEMA IF NOT EXISTS ${targetSchema};`); + await this.db.exec( + `CREATE EXTENSION pg_stat_statements SCHEMA ${targetSchema};`, + ); } catch (err) { throw new PostgresError(err instanceof Error ? err.message : String(err)); } } + // Resolve the schema rather than assume it. The extension may predate this + // call, and an unqualified probe reads through the search_path — which + // reports 42P01 for the very placement the branch above just produced. + const source = await this.getQuerySource(); try { - await this.db.exec(`SELECT 1 FROM pg_stat_statements LIMIT 1; -- @qd_introspection`); + await this.db.exec( + `SELECT 1 FROM ${source.schema}.${source.extensionName} LIMIT 1; -- @qd_introspection`, + ); } catch (err) { throw new PostgresError(err instanceof Error ? err.message : String(err)); } - return { preloadUpdated }; + return { preloadUpdated, schema: source.schema.unquoted() }; } public async checkPrivilege(): Promise<{ From 7f855d3ee0bde5c28dde5930b41f7aac4914f82b Mon Sep 17 00:00:00 2001 From: Jean-Philippe Sirois Date: Thu, 27 Aug 2026 21:31:08 -0300 Subject: [PATCH 2/3] fix(sync): probe the installed extension and warn once per connection The verify step read `getQuerySource()`, which resolves either `pg_stat_statements` or `pg_stat_monitor` with no ordering, so with both installed it could verify an extension the install never touched. It now resolves the one extension it installed. The public-schema warning was guarded by an instance flag, but `ConnectionManager.getConnectorFor` builds a new connector on every poll, so it fired every ten seconds. The guard is now keyed by connection: once per database, and still once per database when several are attached. `POST /postgres/extensions/pg_stat_statements` takes the schema through the websocket controller as well, and rejects an empty one. Co-Authored-By: Claude Opus 5 (1M context) --- src/remote/remote-controller.ts | 21 +++++++++--- src/remote/remote.dto.ts | 2 +- src/remote/remote.ts | 7 ++-- src/sync/pg-connector.test.ts | 7 ++++ src/sync/pg-connector.ts | 58 +++++++++++++++++++++++++++++---- 5 files changed, 81 insertions(+), 14 deletions(-) diff --git a/src/remote/remote-controller.ts b/src/remote/remote-controller.ts index 1981a83d..57fef7f1 100644 --- a/src/remote/remote-controller.ts +++ b/src/remote/remote-controller.ts @@ -1,7 +1,10 @@ import { WebSocket } from "ws"; import { env } from "../env.ts"; import { log } from "../log.ts"; -import { RemoteSyncRequest } from "./remote.dto.ts"; +import { + InstallPgStatStatementsRequest, + RemoteSyncRequest, +} from "./remote.dto.ts"; import { Remote } from "./remote.ts"; import * as errors from "../sync/errors.ts"; import type { OptimizedQuery } from "../sql/recent-query.ts"; @@ -251,14 +254,24 @@ export class RemoteController { } async onInstallPgStatStatements(rawBody: string): Promise { - const body = RemoteSyncRequest.safeDecode(rawBody); + const body = InstallPgStatStatementsRequest.safeDecode(rawBody); if (!body.success) { return { status: 400, body: body.error }; } try { - const result = await this.remote.installPgStatStatements(body.data.db); - return { status: 200, body: { success: true, preloadUpdated: result.preloadUpdated } }; + const result = await this.remote.installPgStatStatements( + body.data.db, + body.data.schema, + ); + return { + status: 200, + body: { + success: true, + preloadUpdated: result.preloadUpdated, + schema: result.schema, + }, + }; } catch (error) { console.error(error); if (error instanceof errors.PostgresError) { diff --git a/src/remote/remote.dto.ts b/src/remote/remote.dto.ts index 5661d014..caf9b82a 100644 --- a/src/remote/remote.dto.ts +++ b/src/remote/remote.dto.ts @@ -25,7 +25,7 @@ export const InstallPgStatStatementsRequest = z.codec( z.string(), z.object({ db: z.custom(), - schema: z.string().optional(), + schema: z.string().min(1).optional(), }), { encode: (value) => diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 5c5e7fd8..e281c4e9 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -638,9 +638,12 @@ export class Remote extends EventEmitter { this.optimizer.restart({ clearQueries: true }); } - async installPgStatStatements(source: Connectable): Promise<{ preloadUpdated: boolean }> { + async installPgStatStatements( + source: Connectable, + schema?: string, + ): Promise<{ preloadUpdated: boolean; schema: string }> { const connector = this.sourceManager.getConnectorFor(source); - return connector.installPgStatStatements(); + return connector.installPgStatStatements({ schema }); } /** diff --git a/src/sync/pg-connector.test.ts b/src/sync/pg-connector.test.ts index cb41121d..e0771b24 100644 --- a/src/sync/pg-connector.test.ts +++ b/src/sync/pg-connector.test.ts @@ -437,8 +437,15 @@ test("getRecentQueries warns when the extension sits in public", async () => { try { await connector.getRecentQueries(); + // A fresh connector over the same connection, which is what every poll + // does. The warning dedupes per connection, so this must stay silent. + await manager.getConnectorFor(conn).getRecentQueries(); const messages = logged.mock.calls.map((call) => String(call[0])); + const warnings = messages.filter((message) => + message.includes("public schema") + ); + expect(warnings).toHaveLength(1); expect( messages.some((message) => message.includes("pg_stat_statements") && diff --git a/src/sync/pg-connector.ts b/src/sync/pg-connector.ts index 1689c2ae..725e427d 100644 --- a/src/sync/pg-connector.ts +++ b/src/sync/pg-connector.ts @@ -91,7 +91,17 @@ export class PostgresConnector implements DatabaseConnector, Rece public static readonly EXTENSION_SCHEMA = "query_doctor"; private readonly tupleEstimates = new Map(); private querySource: QuerySourceExtension | null = null; - private warnedAboutPublicSchema = false; + /** + * Keyed by connection, because the connector is rebuilt on every poll — an + * instance flag would re-warn every ten seconds for the whole run — while a + * process-wide flag would silence every source database after the first. + * ConnectionManager caches one Postgres per database, so it is the identity + * that matches "warn once about this database". + */ + private static readonly warnedAboutPublicSchema = new WeakMap< + Postgres, + Set + >(); private static extensionNotInstalledError = new ExtensionNotInstalledError([ "pg_stat_statements", "pg_stat_monitor" @@ -520,17 +530,47 @@ ORDER BY * resolve the schema on every read, so this is the one place that knows. */ private warnIfExtensionIsInPublic(source: QuerySourceExtension): void { - if (source.schema.unquoted() !== "public" || this.warnedAboutPublicSchema) { + const extension = source.extensionName.unquoted(); + if (source.schema.unquoted() !== "public") { return; } - this.warnedAboutPublicSchema = true; - const extension = source.extensionName.unquoted(); + let warned = PostgresConnector.warnedAboutPublicSchema.get(this.db); + if (!warned) { + warned = new Set(); + PostgresConnector.warnedAboutPublicSchema.set(this.db, warned); + } + if (warned.has(extension)) { + return; + } + warned.add(extension); log.warn( `${extension} is installed in the public schema. A migration tool that reconciles public cannot drop its extension-owned views and will abort mid-run (SQLSTATE 2BP01). Move it with: CREATE SCHEMA IF NOT EXISTS ${PostgresConnector.EXTENSION_SCHEMA}; ALTER EXTENSION ${extension} SET SCHEMA ${PostgresConnector.EXTENSION_SCHEMA};`, "postgres", ); } + /** + * The schema one named extension lives in. {@link getQuerySource} answers the + * broader "where do the queries come from", which can resolve to + * `pg_stat_monitor`; a caller that installed a specific extension needs to + * probe that one. + * + * @throws {ExtensionNotInstalledError} + */ + private async getExtensionSchema(extension: string): Promise { + const [row] = await this.db.exec<{ schema: string }>( + `SELECT n.nspname AS schema + FROM pg_extension e + JOIN pg_namespace n ON n.oid = e.extnamespace + WHERE e.extname = $1 -- @qd_introspection`, + [extension], + ); + if (!row) { + throw new ExtensionNotInstalledError([extension]); + } + return PgIdentifier.fromString(row.schema); + } + /** * Get the latest queries using pg_stat_statements * @throws {ExtensionNotInstalledError} - pg_stat_statements is not installed @@ -657,16 +697,20 @@ ORDER BY // Resolve the schema rather than assume it. The extension may predate this // call, and an unqualified probe reads through the search_path — which // reports 42P01 for the very placement the branch above just produced. - const source = await this.getQuerySource(); + const schema = await this.getExtensionSchema("pg_stat_statements"); + this.warnIfExtensionIsInPublic({ + extensionName: PgIdentifier.fromString("pg_stat_statements"), + schema, + }); try { await this.db.exec( - `SELECT 1 FROM ${source.schema}.${source.extensionName} LIMIT 1; -- @qd_introspection`, + `SELECT 1 FROM ${schema}.pg_stat_statements LIMIT 1; -- @qd_introspection`, ); } catch (err) { throw new PostgresError(err instanceof Error ? err.message : String(err)); } - return { preloadUpdated, schema: source.schema.unquoted() }; + return { preloadUpdated, schema: schema.unquoted() }; } public async checkPrivilege(): Promise<{ From 05ccec609c6aa2313a2ffd13d06dba9c0edb198e Mon Sep 17 00:00:00 2001 From: Jean-Philippe Sirois Date: Thu, 27 Aug 2026 21:48:29 -0300 Subject: [PATCH 3/3] fix(sync): classify a missing extension by SQLSTATE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getRecentQueries and resetPgStatStatements matched the literal text `relation "pg_stat_statements" does not exist`. Every statement they send is schema-qualified, so Postgres names the schema in the message and the match never fired: a missing extension reached the caller as a generic PostgresError instead of ExtensionNotInstalledError, and the app showed raw SQL text rather than its install panel. Both now read the SQLSTATE — 42P01 for a missing relation, 42883 for a missing function, 3F000 when the schema itself is gone, which is what a qualified function call reports. getQuerySource also logs the extension and schema it resolved. A run that read the statistics and a run that found nothing were indistinguishable in the logs. Co-Authored-By: Claude Opus 5 (1M context) --- src/sync/pg-connector.test.ts | 75 +++++++++++++++++++++++++++++++++++ src/sync/pg-connector.ts | 31 ++++++++++----- 2 files changed, 97 insertions(+), 9 deletions(-) diff --git a/src/sync/pg-connector.test.ts b/src/sync/pg-connector.test.ts index e0771b24..485c58d4 100644 --- a/src/sync/pg-connector.test.ts +++ b/src/sync/pg-connector.test.ts @@ -3,6 +3,7 @@ import { PostgreSqlContainer } from "@testcontainers/postgresql"; import { ConnectionManager } from "./connection-manager.ts"; import { Connectable } from "./connectable.ts"; import { PgIdentifier } from "@query-doctor/core"; +import { isMissingExtensionObject } from "./pg-connector.ts"; test("getRecentQueries resolves pg_stat_statements in a non-default schema", async () => { const pg = await new PostgreSqlContainer("postgres:17") @@ -460,3 +461,77 @@ test("getRecentQueries warns when the extension sits in public", async () => { await pg.stop(); } }); + +// The catch blocks in getRecentQueries/resetPgStatStatements used to match the +// literal `relation "pg_stat_statements" does not exist`. Every query that +// reaches them is schema-qualified, so Postgres names the schema too and the +// match never fired: a missing extension surfaced as a generic PostgresError +// rather than ExtensionNotInstalledError, and the UI showed raw SQL text +// instead of its install panel. +test("a qualified read of a missing extension reports 42P01, not the bare relation name", async () => { + const pg = await new PostgreSqlContainer("postgres:17") + .withCopyContentToContainer([ + { + content: `CREATE SCHEMA monitoring;`, + target: "/docker-entrypoint-initdb.d/init.sql", + }, + ]) + .start(); + + const manager = ConnectionManager.forLocalDatabase(); + const conn = Connectable.fromString(pg.getConnectionUri()); + const db = manager.getOrCreateConnection(conn); + + try { + const err = await db + .exec("SELECT 1 FROM monitoring.pg_stat_statements LIMIT 1") + .then(() => null, (e: unknown) => e as { code?: string; message: string }); + + expect(err).toBeTruthy(); + expect(err!.code).toBe("42P01"); + expect(err!.message).not.toContain( + 'relation "pg_stat_statements" does not exist', + ); + + const fnErr = await db + .exec("SELECT monitoring.pg_stat_statements_reset()") + .then(() => null, (e: unknown) => e as { code?: string; message: string }); + + expect(fnErr).toBeTruthy(); + expect(fnErr!.code).toBe("42883"); + expect(fnErr!.message).not.toContain( + "function pg_stat_statements_reset() does not exist", + ); + + // A function call through a schema that is gone entirely reports the + // schema rather than the function, so that code has to be recognised too. + const schemaErr = await db + .exec("SELECT absent_schema.pg_stat_statements_reset()") + .then(() => null, (e: unknown) => e as { code?: string }); + + expect(schemaErr?.code).toBe("3F000"); + } finally { + await manager.closeAll(); + await pg.stop(); + } +}); + +test("isMissingExtensionObject classifies the codes Postgres uses, and nothing else", () => { + expect(isMissingExtensionObject({ code: "42P01" })).toBe(true); + expect(isMissingExtensionObject({ code: "42883" })).toBe(true); + expect(isMissingExtensionObject({ code: "3F000" })).toBe(true); + + // Insufficient privilege and a syntax error are real failures the caller + // must see, not "the extension isn't installed". + expect(isMissingExtensionObject({ code: "42501" })).toBe(false); + expect(isMissingExtensionObject({ code: "42601" })).toBe(false); + + // The message text the old check matched carries no code of its own. + expect( + isMissingExtensionObject( + new Error('relation "pg_stat_statements" does not exist'), + ), + ).toBe(false); + expect(isMissingExtensionObject(null)).toBe(false); + expect(isMissingExtensionObject(undefined)).toBe(false); +}); diff --git a/src/sync/pg-connector.ts b/src/sync/pg-connector.ts index 725e427d..a73854ed 100644 --- a/src/sync/pg-connector.ts +++ b/src/sync/pg-connector.ts @@ -24,6 +24,20 @@ import { RawRecentQuery, RecentQuery } from "../sql/recent-query.ts"; import type { RecentQuerySource } from "../sql/recent-query.ts"; +/** + * Whether an error means the extension's table or function isn't there. + * + * Matching the message text does not work: every statement we send is + * schema-qualified, so Postgres names the schema in the message and a check for + * the bare `pg_stat_statements` never fires. The SQLSTATE is stable — 42P01 for + * a missing relation, 42883 for a missing function, and 3F000 when the schema + * itself is gone, which is what a qualified function call reports. + */ +export function isMissingExtensionObject(err: unknown): boolean { + const code = (err as { code?: unknown } | null)?.code; + return code === "42P01" || code === "42883" || code === "3F000"; +} + const ctidSymbol = Symbol("ctid"); type Row = NonNullable & { [ctidSymbol]: string; @@ -520,6 +534,12 @@ ORDER BY extensionName: PgIdentifier.fromString(firstResult.extension), schema: PgIdentifier.fromString(firstResult.schema) }; + // A run that reads the statistics and a run that silently found nothing + // used to look identical in the logs. Name what was resolved. + log.debug( + `query source: ${firstResult.extension} in schema ${firstResult.schema}`, + "postgres", + ); this.warnIfExtensionIsInPublic(this.querySource); return this.querySource; } @@ -591,10 +611,7 @@ ORDER BY return await syncQueries(results); } } catch (err) { - if ( - err instanceof Error && - err.message.includes('relation "pg_stat_statements" does not exist') - ) { + if (isMissingExtensionObject(err)) { throw PostgresConnector.extensionNotInstalledError; } console.error(err); @@ -621,11 +638,7 @@ ORDER BY `); } } catch (err) { - if ( - err instanceof Error && - (err.message.includes("function pg_stat_statements_reset() does not exist") || - err.message.includes("function pg_stat_monitor_reset() does not exist")) - ) { + if (isMissingExtensionObject(err)) { throw PostgresConnector.extensionNotInstalledError; } throw new PostgresError(err instanceof Error ? err.message : String(err));