Declare each collector's PostgreSQL extension dependency and pin the README to it

[erikdarlingdata]

Refs #3187.

Darling/README.md's permissions paragraph names the collectors that need a PostgreSQL extension installed. Nothing derived that list, and the concrete cost of that is checkable: #3184's commit be617669c ("Name the four collectors that need an extension") named four of the six, and all eight checks passed on it. a9a2e6c2f corrected it, because a reviewer read the paragraph — not because anything failed. The squash merge means dev never carried the short list, but nothing automated would have stopped it.

Each of the six now declares its dependency, and the paragraph is pinned to the declaration.

The authority is a new declaration, not the availability roster

The issue proposed pinning the paragraph to PgExtensionAvailabilityCollector's VALUES roster. That roster is the wrong set, in both directions at once. It carries eight entries — hypopg, pg_trgm and pg_cron are on it and no collector reads any of them — and it deliberately omits pg_wait_sampling, which a collector does read. Its own comment says what it is for: it exists ONLY so that absence is reportable. Every consumer of is_monitoring_relevant treats it as exactly that and none treats it as a dependency oracle — it orders the reader's result set, rides through get_pg_extensions as a reported field, and counts the Viewer's actionable and outdated extensions (ViewerServerTab.Postgres.cs:210-211), which is advice about what an operator could install rather than a statement of what a collector requires. Reportable-absence and collector-dependency overlap; they are not the same question.

No structured dependency declaration existed to pin to — RequiredExtension, RequiresExtension, ExtensionDependency and RequiredPgExtension all returned zero matches repo-wide, and CollectorRuntimePrecondition deliberately carries no extension names ("the remedy text is FRAMED here, not authored here"). So this adds one.

ICollectorSchemaInfo.RequiredPgExtensions declares the extension a collector's query cannot run without, plus PgExtensionInstallKind — CreateExtension for a plain statement, SharedPreloadLibraries for the ones that need a server restart first. It sits on the non-generic surface beside AppliesTo and StateKeys so the catalog can enumerate it without the row type, as a default interface member so the test doubles that implement the interface directly need no change.

The dependency set was derived from each PostgreSQL definition's own query text rather than inherited: six collectors read an extension-owned object, and three others name one in order to REPORT on it (pg_extension_availability's roster, pg_table_bloat_stats' pgstattuple_available flag, pg_plan_capture_readiness' GUC probes). The install kinds match the verification rig's own shared_preload_libraries line exactly.

Two axes are deliberately absent from the declaration because they are already derivable. Whether an extension is needed in every database is RunsPerDatabase, which is true for exactly pg_index_bloat and pg_predicate_stats — the two the paragraph calls per-database. And pg_stat_kcache's dependency on pg_stat_statements is a property of that extension rather than of the collector, so only the extension whose objects the query reads is declared.

What the pins hold

ReadmeDerivedCountPinTests gains a set-pin family alongside its numeric one, compared in both directions — missing catches the defect be617669c carried, extra catches its mirror image, a paragraph claiming a dependency nothing has. Three spans are pinned: the collector/extension list as pairs, the shared_preload_libraries set, and the per-database set. Each asserts its pattern matched before comparing, per the file's existing rule, and the new extractions are registered in EveryPin_ReportsAnInjectedDrift — mutated once per MEMBER rather than once per pin, so every name in each sentence is individually shown to be load-bearing. One mutation per pin would prove only that the sentence is read at all, which is not the property that failed.

PgExtensionDependencyContractTests holds what the README pin cannot. The README pin fires when the declaration and the paragraph disagree; it says nothing when a new collector reads an extension and never declares it, because both sides then stay consistent and both stay wrong. So a PostgreSQL collector whose query text touches an extension-owned object has to declare it, the declaration has to be well-formed and PostgreSQL-only, and the verification rig has to be able to load and create everything declared.

The reverse direction is checked too, because of what the README pin would otherwise do with a false declaration: it reports the extension as missing from the paragraph, and the obvious way to clear that is to write the extension into the prose — which lands a dependency the product does not have in the documentation, looking derived. So a declared extension whose objects the collector's query never touches fails. Only extensions the object map knows are checked, so declaring a new extension does not first require extending the map; needing to extend it must not become a reason to weaken it.

Three word-numerals over the same set, deleted

The paragraph carried "Six collectors need something installed", "All six degrade identically" and "light up the five that are true extensions" — three uncounted numerals over the set this pin makes authoritative. Deleting a number from prose is the other sanctioned outcome under #3072, and leaving them would have meant shipping a pin that guards the list while three numerals about that same list stayed unguarded.

the five that are true extensions became every one of them except pg_wait_sampling, which is the same fact stated so it cannot go stale by arithmetic.

Scope: this closes the automatable gap, it does not prevent the class

Of the five defects found reviewing #3184, a pin catches one — the missing collectors. The other four were a grant/extension category error, an invalid GRANT SELECT on a schema, an over-broad claim about the availability roster, and a stale prose count, and every one of them needed a reviewer. Nothing here should be read as making that paragraph safe; it makes exactly one of its claims derived.

The install-kind correction is worth calling out as a defect this pin did not catch and would not have. The paragraph named shared_preload_libraries for pg_stat_statements and pg_wait_sampling only, while DarlingMcpPgKernelStatsTools and DarlingMcpPgPredicateTools both say in shipped operator-facing text that pg_stat_kcache and pg_qualstats need it too — and the rig's shared_preload_libraries=pg_stat_statements,pg_wait_sampling,pg_stat_kcache,pg_qualstats,auto_explain settles it. An operator following the paragraph would have created pg_stat_kcache and watched the collector store nothing. That is now stated and pinned, but it was found by reading, not by a test.

pg_wait_sampling is not a preload-only module, and the paragraph asserted it twice. tools/pg-verification-rig/seed.sql runs CREATE EXTENSION IF NOT EXISTS pg_wait_sampling;, and PgWaitSamplingCollector reads pg_wait_sampling_profile — a function only that statement creates. auto_explain genuinely owns no SQL object; the two were lumped together and only auto_explain fits.

Both assertions are gone from this paragraph. The install-kind sentence no longer says pg_wait_sampling needs a preload "rather than a CREATE EXTENSION" — that clause is deleted, and the replacement claims only that a restart is required, which the rig confirms. And the roster-exclusion sentence no longer explains the exclusion by saying preload-only modules never appear in pg_available_extensions: that mechanism is false for the one case it was applied to, so it is replaced by the reason PgExtensionDependency states twenty lines away — the roster is a reportable-absence list rather than a list of collector dependencies, and the two are different sets. The issue references now attach to the fact that pg_wait_sampling was taken off the roster rather than to why.

PgExtensionAvailabilityCollector's own type header and the V89 migration row still carry the preload-only characterization and are left alone: the exclusion rationale living next to it is pinned in Lite.Tests/PgExtensionAvailabilityCollectorDefinitionTests.cs, and whether the roster should exclude pg_wait_sampling at all is a separate question from whether this paragraph may state a false reason for it.

CHANGELOG entry text

• A collector's PostgreSQL extension dependency is declared rather than described ([Pin the README's extension-dependent collector list to PgExtensionAvailabilityCollector's roster #3187]) - Darling/README.md's permissions paragraph named the collectors that need an extension installed, and nothing derived that list - Document the PostgreSQL grant and extensions that decide whether a collector returns anything #3184's be617669c named four of the six and all eight checks passed on it, corrected only because a reviewer read the paragraph. An enumeration in prose is only better than a count if something breaks when it is wrong. ICollectorSchemaInfo.RequiredPgExtensions now carries the extension each collector cannot run without plus what installing it costs - CreateExtension for a statement, SharedPreloadLibraries for the ones needing a server restart first - and the paragraph is pinned to it in BOTH directions, so a dependency the product has and the documentation omits fails the build, and so does a paragraph claiming one nothing declares. The nearby list that looks like the authority is not it: PgExtensionAvailabilityCollector's roster carries eight entries because it exists so ABSENCE is reportable, three of which no collector reads, and it deliberately omits pg_wait_sampling, which one does. Two axes stay derived rather than declared twice - per-database is RunsPerDatabase, and pg_stat_kcache sitting on pg_stat_statements is a property of that extension. A second pin closes the case the README pin cannot see, where a new collector reads an extension and never declares it: a PostgreSQL definition whose query text touches an extension-owned object has to declare it, and the verification rig has to be able to load and create everything declared. The paragraph's three word-numerals over the same set are deleted rather than pinned, the Darling/README.md says twelve PostgreSQL read tools and there are thirty-two, which is the third hand-maintained count found stale tonight #3072 alternative. Two corrections found by reading rather than by a pin: pg_stat_kcache and pg_qualstats need shared_preload_libraries too - which the MCP tools already told operators and the rig's own preload line confirms - so following the paragraph meant creating the extension and watching the collector store nothing. And the paragraph explained pg_extension_availability's exclusion of pg_wait_sampling by saying preload-only modules never appear in pg_available_extensions, which is false for that one case: the rig's seed.sql creates it with CREATE EXTENSION and the collector reads a function only that statement creates. The exclusion now carries the set-membership reason instead - reportable-absence and collector-dependency are different sets - which is the same reason the new type states.

[claude]

Reviewed this PR (declaration of RequiredPgExtensions per collector, README pin extension, new contract tests).

Checked in detail:

• The 6 collectors that get RequiredPgExtensions (pg_statement_stats, pg_index_bloat, pg_buffer_usage, pg_kernel_stats, pg_predicate_stats, pg_wait_sampling) match what their query text actually reads — traced each one against ExtensionObjects/ReportsRatherThanReads in PgExtensionDependencyContractTests.cs, and against the other PostgreSQL collectors that merely mention an extension name in doc comments or advisory strings (PgCpuUtilizationCollector, PgBlockingCollector, PgWaitStatsCollector, PgSessionStatesCollector, PgPlanCaptureReadinessCollector, PgTableBloatStatsCollector, PgExtensionAvailabilityCollector) — none of those mentions sit inside a verbatim query literal, so the source-scan test correctly doesn't (and shouldn't) pick them up.
• RunsPerDatabase derivation for the new PerDatabaseExtensions pin (pgstattuple, pg_qualstats) matches each collector's actual override/default.
• New SetPins() regexes in ReadmeDerivedCountPinTests.cs correctly match the rewritten Darling/README.md paragraph in both directions (extraction verified by hand against the new prose), and the mutation test (EveryPin_ReportsAnInjectedDrift) exercises each name individually.
• The default interface member on ICollectorSchemaInfo.RequiredPgExtensions (mirroring the existing TargetEngine/StateKeys pattern) means none of the direct ICollectorSchemaInfo test-double implementers (PgSchemaGeneratorTests.TruncatedSchema, PayloadDimensionTests.FakeSchema, CollectorEngineCapabilityDerivationTests.SyntheticCollector) need updating — confirmed.
• No Lite/Darling parity issue: this is PostgreSQL-only surface, Lite never implements ICollectorSchemaInfo with a non-empty extension set, and nothing in Lite/ references the new types.
• No T-SQL, no new query text, no new user-facing input handling — nothing applicable to the SQL-injection/secrets/OPTION(RECOMPILE) checks.

No correctness issues or parity drift found. This looks solid and unusually well self-audited (the PR description already anticipates and rebuts the two obvious alternative designs).

[claude]

Reviewed. This is a well-scoped, well-tested change — I traced the full contract end to end and it holds together:

• RequiredPgExtensions/PgExtensionDependency is a clean addition to the shared, engine-neutral collector schema surface (default interface member, so no existing implementer or test double needs to change).
• Verified all six declarations against each collector's actual query text (PgBufferUsageCollector → pg_buffercache, PgIndexBloatCollector → pgstattuple, PgKernelStatsCollector → pg_stat_kcache, PgPredicateStatsCollector → pg_qualstats, PgStatementStatsCollector → pg_stat_statements, PgWaitSamplingCollector → pg_wait_sampling) and against tools/pg-verification-rig's docker-compose.yml/seed.sql — the install kinds (CreateExtension vs SharedPreloadLibraries) match the rig exactly.
• Confirmed the new ReadmeDerivedCountPinTests regexes actually match the new Darling/README.md prose and extract the expected sets (collector/extension pairs, the preload set, the per-database set), including the pg_kernel_stats (pg_stat_kcache, which sits on top of pg_stat_statements) case where the extraction correctly avoids capturing the parenthetical aside as a second dependency.
• Confirmed RunsPerDatabase (not re-declared) correctly implies the per-database set: pg_index_bloat and pg_predicate_stats are the only two of the six with RunsPerDatabase(target) == true, matching the README sentence.
• No Lite/Darling parity concern: PerformanceMonitor.Collectors is shared, but Lite has no PostgreSQL target at all (confirmed via DuckDbSchemaGenerator.cs/McpEngineCapability.cs comments), so there's nothing for Lite to mirror here.
• No security, T-SQL style, or performance concerns — this touches no T-SQL and no runtime request path; RequiredPgExtensions is currently documentation-only (not yet wired into any runtime precondition/skip logic), which the PR description explicitly scopes out as future work rather than this PR's job.

Left one inline note: the paragraph correctly retires the false "preload-only modules never appear in pg_available_extensions" claim for pg_wait_sampling, but Lite.Tests/PgExtensionAvailabilityCollectorDefinitionTests.cs still carries that same now-disproven claim in its doc comment. Worth a quick follow-up so the stale rationale doesn't get copied forward again.

🤖 Generated with Claude Code