Store new plan XML gzip-compressed in the plan dimension (#2069)

[erikdarlingdata]

Closes #2069.

What

New execution plans are stored gzip-compressed (query_plan_gz bytea, V54) in query_plan_dim; the dimension's text column goes nullable and stays NULL on new rows. Every plan reader resolves text-else-gz through one rule (PayloadDimensions.ResolveContent).

Why

The #2068 self-metrics sweep quantified it: query_plan_dim is 101 GB of the production store's 147 GB (69%). lz4 TOAST already compresses it 9.2x; a bake-off on the store's own live content measured application-side gzip (Optimal) at 14.0x vs lz4's 8.9x on the identical sample — projected steady state ~64 GB. gzip, not zstd: PG18 TOAST enumvals are pglz, lz4 only (verified against the shipped binary), and .NET ships gzip natively. zstd stays tracked in #2071.

Design properties

• Digest over UNCOMPRESSED text — identity, dedup, fact digests, and has_query_plan presence all survive the format change (pinned by test).
• Self-converting coexistence — old rows keep text, readers take either form, the dim GC retires text rows on its normal horizon (~9 days measured). No rewrite, no peak-disk spike, metadata-only migration.
• All six read seams covered — service MCP reads (by-hash + by-sql_handle), viewer keyed fetches (query + procedure), FinOps workload (MAX aggregate + correlated sample subquery), actual-plan stored-query resolver (other source kinds bind NULL::bytea so the shared reader stays uniform).
• View shape — v_query_stats appends query_plan_gz after the digests (replace-only-appends rule); V38 pre-adds the column before the view for the partial-apply re-run case (pinned, same reasoning as the digest pre-adds).
• Disclosed loss — raw SQL sees NULL query_plan_xml on post-V54 plans; the bytes in query_plan_gz are a plain RFC 1952 gzip member (gunzip alone recovers them). Product surfaces unaffected. The live write-path test now pins this contract end to end.

Ladder ceremony

StorageVersion 54; 37th viewer probe sentinel (query_plan_dim.query_plan_gz) with newest-first arm; pins swept: observability x2, PVS x3, full-sentinel 36->37, live-migration applied 8->9, last-view-definer tripwire 51->54, view/DDL/upsert shape pins.

Tests

New: codec round-trip incl. non-ASCII + real-gzip magic, absent->null, text-first preference, digest stability across the format change, V38 gz-before-view ordering. Updated: the SQL shape pins above + the live write-path round-trip (now asserts NULL text + gz bytes in both the dim and the view).

Lite is untouched (no dims, no plan capture).

🤖 Generated with Claude Code

[claude]

Review: #2072 — gzip-compressed plan-dimension content

Overall this is a carefully executed, well-tested storage change: additive-only migration (V54), digest computed over uncompressed text so identity/dedup survive the format change, a single PayloadDimensions.ResolveContent resolution rule, and thorough test coverage for the codec, the upsert shape, the view generator, and the ladder ordering (V38 pre-add, newest-first schema-probe arm, StorageVersion bump). Lite is correctly untouched — it has no payload dimensions or plan-dim storage at all, so there's no parity drift there.

Correctness bug: two read seams still filter on the bare query_plan_xml column

The PR's stated guarantee is "every plan reader resolves text-else-gz" via PayloadDimensions.ResolveContent, and the description claims all read seams are covered. Two are not, and neither was touched by this PR:

• Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.QueryPerf.cs:466-474 (PlanAdvisorySql)
• Darling/PerformanceMonitor.Darling.Analysis/PgDrillDownCollector.Plans.cs:119-127 (PlanAdvisoryXmlSql)

Both select query_plan_xml from v_query_stats and gate on query_plan_xml IS NOT NULL, without projecting or checking query_plan_gz. Since every row written since V54 leaves query_plan_xml NULL (the content lives only in query_plan_gz), these predicates will silently exclude all post-migration plans from the "top 10 by delta_worker_time" set — the exact "zero rows, no error, indistinguishable from no plan captured" failure mode this PR explicitly guards against everywhere else (DarlingStoredPlanReader, ViewerDataService.Plans.cs, the FinOps aggregate/subquery reads).

Practical effect: CollectPlanAdvisoryFactsAsync (WS4 MISSING_INDEX / PLAN_WARNING fact collector) and CollectPlanAdvisoryDetail (the matching drill-down detail) go dark for any busy server whose top-10-by-CPU queries are all recent — plausibly within hours to a day of applying this migration, well before the ~9-day GC horizon the PR description cites. Neither SQL string has a pinning test (unlike every other plan-read SQL touched in this PR), so nothing in the suite catches the regression.

Suggested fix: project query_plan_gz alongside query_plan_xml in both queries, widen the guard to (query_plan_xml IS NOT NULL OR query_plan_gz IS NOT NULL), and resolve each row with PayloadDimensions.ResolveContent the same way DarlingStoredPlanReader.ReadPlanTextOrGzipAsync already does.

(I attempted to leave these as inline comments but GitHub rejects line comments on files outside the diff — flagging here instead since both files are otherwise untouched by this PR.)

Minor / non-blocking

• ViewerDataService.FinOps.Workload.cs's HighImpactQueriesSql now runs two independent correlated subqueries for query_plan_xml and query_plan_gz per output row instead of one. The code comment acknowledges they can land on different sample rows — an accepted tradeoff, not a bug — but note it's added subquery cost, and once the store has fully turned over the query_plan_xml subquery will almost always scan for a row that no longer exists (dead weight for the life of the store, not just the transition window).
• Everything else — codec round-trip, digest stability, upsert SQL, view generation/ordering, migration idempotency, schema-probe sentinel, and the retention/GC path (keyed by digest, format-agnostic) — checks out. No SQL injection surface (all statements are static text with typed/parameterized binds), no secrets or file/network/process concerns introduced.

🤖 Generated with Claude Code

[claude]

Review

Scope: gzip-compresses new plan XML into query_plan_dim.query_plan_gz (Darling only; Lite is untouched by design, confirmed no query_plan_dim/PayloadDimensions/CompressContent references anywhere under Lite/ or Lite.Tests/), with a single PayloadDimensions.ResolveContent (text-else-gz) rule applied consistently across every read seam.

Correctness

Traced every touched read path (service MCP reads, viewer keyed plan fetches, FinOps MAX/correlated-subquery aggregates, the actual-plan resolver's three source kinds) and the column indices/NULL::bytea placeholders line up correctly in each. The write path (PayloadDimensionWriter.FlushAsync → PayloadDimensions.CompressContent) only ever receives non-null payloads (PayloadDimensionBatch.Add throws on null before it can reach the batch), so the ArgumentNullException guard in CompressContent is dead code but harmless. DecompressContent's null/empty-bytea → null handling matches the "no content" convention the rest of the codebase already uses for empty text.

The V38 pre-add (PgSchemaGenerator.GenerateV38PayloadDimensions) and the V54 migration are ordered correctly relative to the regenerated view, and ViewerDataService.MapProbedSchemaVersion's new hasPlanDimGzip sentinel is appended last, matching the appended last column in StoreSchemaProbeSql (37 EXISTS clauses ↔ GetBoolean(0..36) ↔ 37 parameters). Good pinning coverage on the ladder (StorageVersion 53→54, applied-count 8→9, definer tripwire 51→54).

One thing worth naming explicitly (not a blocker, since the PR description already discloses the mechanism): the ON CONFLICT DO UPDATE SET last_seen = EXCLUDED.last_seen upsert for query_plan_dim never rewrites an existing text row to gzip — it only refreshes last_seen. So a digest that stays "hot" (same plan text keeps being seen) never converts to gzip; only digests that go idle long enough to fall out of the GC's retention window get removed and effectively replaced by a fresh gzip row. The ~64 GB steady-state projection depends on plan digests churning within roughly the measured ~9-day GC horizon rather than persisting indefinitely. Given the production numbers cited (6.5M distinct plans, 9-day measured GC turnover) this is likely fine in practice, but it means an unusually long-lived stable plan (e.g., a rarely-recompiled proc under a fixed digest) stays uncompressed text forever under this design — worth confirming that's the intended tradeoff vs. a future in-place rewrite path.

Performance

GetHighImpactQueriesAsync's HighImpactQueriesSql now runs 4 correlated subqueries per query_hash group instead of 3 (a dedicated qs2.query_plan_gz IS NOT NULL subquery alongside the existing query_plan_xml one). This is a deliberate, disclosed tradeoff (the two subqueries can legitimately land on different sample rows), but it's worth flagging as incremental cost on an already subquery-heavy viewer read — likely fine given this is an interactive, low-QPS path, not a hot ingest path.

Security

No new user-input handling; the compressed bytes come from the service's own SQL Server plan capture and are only ever decompressed by the same service. No injection surface changes — all new SQL is compile-time constant strings with parameterized binds, consistent with the rest of the codebase.

Style / tests

New SQL literals use /* */ block comments (not --), consistent with the T-SQL style guide's spirit even though this is generated Postgres DDL/DML. Test coverage is thorough: codec round-trip (incl. non-ASCII + real gzip magic), null/empty resolution, digest stability across the format change, V38 ordering, and every SQL-shape pin updated in lockstep with the production code. Nothing found that would need Lite-side changes, since Lite has no dimension tables or plan capture at all.

No blocking issues found.