From dd71bad23f09a88420151e9e908b80ec32c0df74 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sun, 26 Jul 2026 08:06:15 -0400 Subject: [PATCH] Add the missing CHANGELOG entry for #1667 The merged fix had no [Unreleased] entry. Records the behaviour change as well as the fix: that one row's sql_variant base type goes from binary(8) to nvarchar(20), so anyone querying the view directly and converting to varbinary needs to adjust -- though the old value was garbled and therefore unusable, which is the bug being fixed. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09b502c05..413cd27ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Full Dashboard (deprecated): `report.daily_summary_v2` no longer garbles `worst_query_hash`** ([#1667]) - the view boxed the raw `binary(8)` `query_hash` straight into its `sql_variant` pivot column, so anything that stringified that value reinterpreted the 8 bytes as UTF-16 and printed mojibake (`稟렿坧譈`) instead of a hash. It is the only non-scalar payload among the view's 25 pivoted metrics - every other row boxes a number or a string and round-trips cleanly. Now hex-encoded before boxing (`CONVERT(nvarchar(20), ..., 1)`), yielding `0x1F7A7FB76757088B`. Community fix by @argpna ([#1666]), verified live against SQL Server 2022. **Note for anyone querying this view directly:** the `sql_variant`'s base type for that one row changes from `binary(8)` to `nvarchar(20)`, so a caller doing `CONVERT(varbinary(8), metric_value)` needs updating - though in practice the old value was unusable, which is the bug. + - **Darling viewer: built-in tabs no longer silently lose history past the raw retention horizon** ([#1661]) - the three-tier retention from [#1623] drops raw `query_stats` at 4 days and runs automatically on every service start, but the CAGG read-routing from [#1625]/[#1626]/[#1627] only ever covered the Custom Views composer. The viewer's built-in tabs read `v_query_stats`, a plain passthrough over the raw hypertable, so the Performance Calendar reported zero distinct queries for every day older than ~4 days and the FinOps panels aggregated a 4-day sample while the picker said 30. Nothing errored - the numbers were just quietly wrong, which for a cost view whose whole purpose is trend over time is the worst failure mode available. Each PR was correct in isolation: the write side and read side were designed as a pair and the read side was scoped to the surface being built at the time. **Never released** - caught in `dev` before 3.3.0. Fixed by giving the tier decision one definition (`RetentionTierRouter`, in Storage because the composer lives in the service and the tabs live in the viewer and neither can see the other; the composer now delegates to it and no longer keeps its own thresholds) and routing every reader that can be routed: the calendar's query count, the `get_daily_health` MCP reader, and three FinOps aggregates. The FinOps database-grain view needed I/O sums no rollup carried, so it gets a NEW `query_stats_db_hourly`/`_daily` pair rather than a widened existing one - TimescaleDB cannot ALTER columns into a continuous aggregate, and with retention live a drop-and-recreate would rebuild from 4 days of raw and permanently destroy the very history the tiers exist to preserve. Readers that project per-row `query_text`/`query_plan` cannot be routed at all (no rollup carries text), so the expensive-queries panel clamps to the text horizon and **says so** instead of presenting a few days as a month. Routing that also exposed a second copy of the entire daily-summary query in the MCP health reader, documented as the viewer's "verbatim" with nothing enforcing it - unified into one definition, since routing one and not the other would have had the calendar and the MCP tool disagree about the same day. The routing is gated on the rollups actually existing (a `to_regclass` probe, cached in the viewer): the continuous aggregates are runtime TimescaleDB setup, so a bring-your-own **plain PostgreSQL** store never has them - by-age routing alone threw `42P01` at the user there (caught by the gated-live CI, which runs plain PG). On plain PG the raw tables are never dropped either (retention policies need the extension), so routing everything to raw is complete, not degraded; a partially-built TimescaleDB store degrades one tier at a time (daily missing -> hourly, hourly missing -> raw), mirroring the composer's existing rule. - **Lite + Darling: a monitoring login without `VIEW SERVER STATE` no longer loses its entire `server_properties` row** ([#1591]) - `ServerPropertiesCollector` read hardware facts straight out of `sys.dm_os_sys_info` in the FROM clause of its main SELECT, alongside a dozen permission-free `SERVERPROPERTY` scalars. That DMV requires VIEW SERVER STATE (VIEW DATABASE STATE on Azure SQL DB), so a login lacking it failed the whole statement and collected **nothing** - no edition, no version, no patch level, no service objective - when only the six hardware columns were actually gated. Azure SQL DB users hit this hardest, since the grant is database-scoped there and monitoring logins routinely lack it on `master`. The DMV read now happens up front into variables inside TRY/CATCH - the exact pattern the supplemental health query ten lines below already used - and the projection reads no table at all, so a permission failure costs only `cpu_count`, `hyperthread_ratio`, `physical_memory_mb`, `socket_count`, `cores_per_socket` and `sqlserver_start_time`, which come back NULL. The first three were `NOT NULL` in DuckDB and are now nullable (Lite schema **v48** migrates existing databases with `ALTER COLUMN ... DROP NOT NULL`; Postgres was already nullable), and the collector's `Row` types them `int?`/`int?`/`long?` so "unknown" is representable rather than a fabricated zero. Downstream is unaffected: the FinOps and MCP readers consume the stored row and already null-guarded these columns. Pinned by three guards - the projection must reference no table, the hardware columns must bind to the TRY/CATCH variables while identity columns stay direct `SERVERPROPERTY` reads, and the `Row` fields must stay nullable - plus an explicit, documented exception in the schema-equivalence proof so the deliberate NOT NULL drop is reviewable instead of silently rewriting the frozen golden snapshot. @@ -1620,3 +1622,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#1650]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1650 [#1591]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1591 [#1661]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1661 +[#1667]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1667 +[#1666]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1666