query_store: collect per database on Azure SQL Database (#1836)

[erikdarlingdata]

Closes #1836.

The bug

query_store returned zero rows on Azure SQL Database, on every cycle, since the collector existed.

It is enumeration-shaped (list the databases whose Query Store is usable, then collect each one), and its Azure enumeration probed every candidate through QUOTENAME(@db) + N'.sys.sp_executesql' — a cross-database three-part reference that Azure SQL Database rejects for every database, from master and from a user database alike. Each rejection landed in an empty CATCH, so the list came back empty, the cycle logged SUCCESS with 0 rows, and nothing said why. Setting the server entry's Database field could not save it either: it would still have been asking one database for the whole instance, the wrong way.

The fix

Azure SQL DB now runs the collector per database (RunsPerDatabase), the same way query_stats and the five database-scoped siblings do, so a logical server with ten databases collects ten databases instead of none. Each per-database run starts with the same eligibility gate the on-prem cursor probes with (actual_state IN (1, 2, 4) and readonly_reason & 8 = 0, the #1546/#1558 semantics), evaluated locally against the connected database: an ineligible database costs one catalog lookup and returns no result set, never touching the Query Store views. The dead Azure enumeration is deleted rather than left unreachable, so a future edit cannot revive it.

The on-prem/RDS/MI enumeration + per-item pair is unchanged in behavior.

One payload, two shapes

The guard this rework is designed around. BuildPayloadBody returns the payload as ordinary single-quoted T-SQL — what Azure's per-database connection executes verbatim — and BuildPerItemQuery quote-doubles that same string to nest it inside [db].sys.sp_executesql. The single Replace is the whole difference between the two paths' SQL. A test asserts the containment, so hand-editing either path's columns fails loudly rather than drifting the two paths out of the reader's 53 fixed ordinals. Same structure as IndexObjectStatsCollector, the existing dual-shape collector.

Two column decisions, going opposite ways

• plan_type is now ON for Azure. The probe reports PRODUCTVERSION 12 there, but the engine is evergreen; sys.query_store_plan's applies-to banner names Azure SQL Database and its only documented exclusion is Synapse (engine edition 6, never IsAzureSqlDb).
• replica_role is explicitly gated OFF for Azure, emitting the same typed NULL placeholder at the same ordinal a pre-2022 box does — the column set is unchanged, only the attribution is given up. sys.query_store_runtime_stats.replica_group_id is documented for SQL Server 2022+ and is silent on Azure SQL Database on a page whose sibling columns name Azure explicitly, and Query Store for secondary replicas is documented as unavailable on Hyperscale with no statement about whether the view still binds. A column that does not bind is not a NULL: it fails the whole SELECT, which on this path means every database. The comment names the exact one-line probe that would justify flipping it.

Consequences handled rather than left

• The 24h catch-up clamp now lives in the collector's cutoff computation, so it holds on the new path. The host's per-database branch is shared with the XE ring-buffer collectors, where clamping would wrongly truncate legitimate catch-up, so the bound travels with the collector that needs it. It is idempotent with the enumeration path's existing clamp.
• The clamp no longer fires silently. WatermarkPolicy's premise is a "deliberate, logged, bounded" hole; the definition now signals the host when it clamps, and both runners log the same WARNING the enumeration path emits.
• The per-database truncation WARNING (row cap / 256MB text budget) now fires on the Azure branch in both SKUs — reachable only because this change puts the one collector that declares those bounds on that branch.

Test plan

• New pins: RunsPerDatabase (true Azure, false on-prem/MI), Azure BuildQuery (eligibility predicates present, guard precedes payload, no cross-database reference, no sp_executesql, payload columns present), the shared-body containment, BuildQuery throwing off-Azure, the replica/plan_type gate split, the clamp and its host signal, and ReadAsync taking database_name from CurrentDatabaseName (and throwing rather than writing blank-database rows without it).
• Lite.Tests: 1654 passed, 0 failed. Darling.Tests: 3590 passed, 0 failed (191 skipped = the gated live-Postgres tests).
• Both apps rebuild with 0 warnings, 0 errors.
• Live against SQL 2022 (both shapes executed, not just generated): the Azure-shape batch returns 53 fields with database_name from the connected database and drops self-marker rows; a Query-Store-off database returns 0 fields / 0 rows with no error, confirming the RETURN guard short-circuits cleanly through ExecuteReader; the on-prem wrapper still collects across databases from master.

Not reproduced on live Azure SQL Database — that is the one remaining validation, along with the replica_group_id probe noted above.

🤖 Generated with Claude Code

[claude]

Review

Went through the full diff (8 files, both QueryStoreCollector.cs and the two host runners) against CONTRIBUTING.md's T-SQL style section and the shared PerformanceMonitor.Collectors conventions. This is a careful piece of engineering — the writeup below is mostly confirmation, not correction.

Correctness

• RETURN-based eligibility gate is sound. AzureEligibilityGateText + BuildPayloadBody(...) produces a single batch where an ineligible database (actual_state/readonly_reason gate) exits via RETURN; before touching any Query Store view — no SELECT executes, so ExecuteReaderAsync/ReadAsync see zero result-set rows without an exception. Matches the "0 fields / 0 rows, no error" behavior the PR description says was verified live.
• Single-payload-body guard actually works. Traced BuildPayloadBody → .Replace("'", "''") → nested inside [db].sys.sp_executesql N'...' for the on-prem path, and confirmed the new test (BuildQuery_Azure_AndPerItemWrapper_ShareTheSinglePayloadBody) asserts real containment, not just structural similarity. A hand-edit to one path's columns without the other will fail loudly, as intended.
• Clamp propagation is correct and idempotent. BuildCutoffParameters calls WatermarkPolicy.ClampCatchup and sets CatchupClampApplied unconditionally (not accumulated), so a stale true from a prior database in the same cycle can't leak onto the next one. On the enumeration path the host already clamps the watermark before calling BuildPerItemQuery, so the definition's re-clamp is a correct no-op (clamped == context.Watermark) — no duplicate WARNING. Verified this against both RemoteCollectorService.DefinitionRunner.cs and DarlingCollectorRunner.cs.
• ReadAsync throwing on missing CurrentDatabaseName is a reasonable design: since the host sets context.CurrentDatabaseName unconditionally right before every per-database attempt, a null here would fire for every database in the cycle, which correctly escalates through the existing "all databases failed → rethrow first failure" path rather than silently writing blank-database rows that could never advance a watermark.
• GetAzureDatabaseListAsync (pre-existing, shared, unchanged by this PR) enumerates via database_id > 0 with only user-configured exclusions — no default system-database filter — so master will now also flow through query_store's per-database loop on Azure. Confirmed this is harmless: master has no usable Query Store, so the eligibility gate's IF NOT EXISTS short-circuits it to a single cheap catalog lookup per cycle. Not a bug, just noting it since this PR is what newly routes query_store through that shared enumeration.

Lite/Darling parity

Diffed Lite/Services/RemoteCollectorService.DefinitionRunner.cs against Darling/PerformanceMonitor.Darling.Service/DarlingCollectorRunner.cs line-by-line for the two new blocks (clamp WARNING, per-database truncation WARNING) — they're structurally identical modulo server.DisplayName vs server.Config.DisplayName and DuckDB- vs Postgres-flavored connection plumbing. No drift. The shared logic (QueryStoreCollector, CollectorContext, ICollectorDefinition, WatermarkPolicy) lives in PerformanceMonitor.Collectors, so both apps get the fix from one definition — consistent with how collector-definition tests already live only in Lite.Tests (confirmed no Darling.Tests copy exists for any collector definition, so the absence of a Darling-side QueryStoreCollectorDefinitionTests.cs isn't a coverage gap, it's the existing convention).

Style

New/changed T-SQL (AzureEligibilityGateText, BuildPayloadBody) follows the house style: uppercase keywords, AS on every table alias, column_name = expression aliasing, trailing commas, /* */ comments only, and the WHERE/AND column alignment used elsewhere in the file. No -- comments or abbreviated types introduced.

Security

No new dynamic SQL surface beyond what already existed — @cutoff_time stays parameterized on both paths, the on-prem quote-doubling is a straightforward escape of a fully-controlled (not user-input-derived) string, and escapedDbName reuses the existing ]→]] bracket-escaping for database names sourced from sys.databases. No secrets, file, network, or process handling touched.

Residual risk (self-disclosed, worth reiterating)

The PR body already flags this, but worth surfacing for reviewers: not yet reproduced against a live Azure SQL Database — only SQL Server 2022 exercised both shapes live. The hasPlanType/hasReplicaAttribution gating decisions for Azure are doc-driven rather than probe-verified (the replica_group_id bind-safety probe the comment calls for hasn't been run against real Azure SQL DB / Hyperscale). Given a bad guess on hasReplicaAttribution would fail the entire per-database payload on Azure (not just null out a column), it'd be good to get that one live confirmation before/soon after merge.

No blocking issues found.