[erikdarlingdata]

Partial #3653 — item 13 (Q9)

The lie

collect.perfmon_stats.cntr_value_per_second is the Full edition's per-second rate for every perfmon counter, and it shipped as

cntr_value_delta / NULLIF(sample_interval_seconds, 0)

cntr_value_delta is bigint, sample_interval_seconds is integer, and T-SQL divides two integer types as an integer. Seven batch requests over a 60-second interval read as 0 per second. Fifty-nine over sixty read as 0. Every counter running under one event per second — compilations, re-compilations, lock waits, log flushes on a quiet store — rendered as a flat zero, and report.daily_summary_v2's batch_requests_per_sec / compilations_per_sec rows were the mean of those truncated integers. On the rig, the same four rows average to 0 under the old column and 0.55 under the new one.

#3658 fixed the Dashboard's READ (both perfmon readers recompute cntr_value_delta * 1.0 / NULLIF(...) and stopped selecting the column; DashboardMirrorPinTests pins that shape). The schema column kept lying to anything else that read it — the reporting view above, and any ad-hoc query against the table. Erik's Q9 ruling: integer division is a defect, the frozen twin's rule is bug-fix-only, this qualifies; the upgrade path must be idempotent.

What changes

install/02_create_tables.sql — the CREATE defines the column as

cntr_value_delta * 1.0 / NULLIF(sample_interval_seconds, 0)

* 1.0 is the idiom the memory_stats percentage columns in the same file already use, and it is byte-for-byte the expression the Dashboard read uses since #3658, so the two halves agree. The type is chosen deliberately and stated in the comment: bigint * numeric(2,1) is numeric(22,1); numeric(22,1) / integer is numeric(33,12) under the T-SQL precision/scale rules. The rig confirms precision 33, scale 12. The column is non-persisted (is_persisted = 0, verified), so the redefinition is metadata-only and costs nothing on disk.

install/06_ensure_collection_table.sql — three things:

1. The procedure's own CREATE block for perfmon_stats carries the same definition (the [BUG] Linux Host CPU is shown at 100% all the time (SQL Server CPU consumption probably correct) #1048 follow-up is the precedent for keeping 06's CREATE aligned with 02 — the recreate path must not re-introduce the defect).
2. An install-time upgrade block at the end of the file, on the is_processed precedent that already lives there. The installer runs upgrades/ first and then every install/*.sql in order, so this block runs on every re-install/upgrade of an existing Full install, and it runs before install/47 re-creates the view that averages the column.
3. A runtime branch inside config.ensure_collection_table's "table already exists, checking for schema upgrades" path, for a caller that asks the procedure to ensure a table that already exists. Logs one SCHEMA_UPGRADE row to config.collection_log, as the is_processed branch does.

The guard predicate: the column exists in sys.computed_columns for collect.perfmon_stats and TYPE_NAME(cc.system_type_id) is an integer type (bigint/int/smallint/tinyint). The type IS the defect — integer division yields an integer-typed column, real division yields numeric — so a converged column can never match and a second run is a no-op. I chose this over matching the stored definition text because SQL Server rewrites that text (the old shape is stored as ([cntr_value_delta]/nullif([sample_interval_seconds],(0))), lower-case nullif, bracketed, parenthesised literals) and its spelling is not a contract across versions.

Drop and re-add are guarded independently (the #1048 upgrade script's "partial-failure safe" shape): if a run dropped the column and died before the re-add, the next run finds the column missing and re-adds it rather than leaving the table without its rate. Rig-verified.

Dependencies checked: nothing in install/ or upgrades/ creates an index on the column, no view in 47/48/54 uses SCHEMABINDING (none in install/ at all), and no CREATE STATISTICS names it — so the DROP COLUMN cannot be blocked. Auto-created statistics drop with the column.

install/47_create_reporting_views.sql — a comment on the throughput CTE, no filter. The brief asked whether the view should also guard sample_interval_seconds > 0 the way #3695 added FILTER (WHERE sample_interval_seconds IS DISTINCT FROM 0) on the PG/DuckDB tiers. I looked, and the answer is no, for a reason the rig then corrected me on: the Full edition's delta framework (install/05) does write a (0, 0) row for a series' first collection — the previous_collection CTE's MIN(collection_id) arm makes the first row its own previous, so delta = value − value = 0 and interval = DATEDIFF(self, self) = 0 (167 of 167 first-collection rows on the rig; the /*Exclude first collection where no previous exists*/ comment in 05 is misleading). But the column's own NULLIF(sample_interval_seconds, 0) turns that row's rate into NULL, and AVG skips NULL (rate_null = 167). On PG/DuckDB the division happens in the aggregate against a raw interval column, so the exclusion had to be a FILTER; here the division and its NULLIF live in the column, so the exclusion is already made before the aggregate sees the row. Adding a WHERE would be a second copy of a rule the column already enforces.

Why this shape and not another

• Not a new upgrades/<from>-to-<to>/ script: that path keys on a version boundary, and the Full twin's versioned ladder stops at 3.0.0-to-3.1.0 while the product is 3.8.0 — picking a boundary would be an invention. The is_processed install-time block in 06 is the established precedent for a schema upgrade that must land on every re-run regardless of version, and its position before 47 is load-bearing here.
• Not a text match on definition: see above.
• Not a CONVERT(decimal(19,4), …): * 1.0 matches the memory_stats idiom and the Dashboard read exactly; a narrower CONVERT would make the schema column and the read disagree in the twelfth decimal for no consumer that needs it.

What it does NOT do

• Does not touch any C#. DashboardMirrorPinTests:207 pins the Dashboard's read SQL (unchanged, still correct). The Dashboard read's comment says "the schema column is left as it is" — after this PR that sentence is stale but harmless (the read recomputes the same expression either way); reported below, not fixed, per the lane's T-SQL-only bound.
• Does not fix the restart row's rate: on a server restart the Full edition's delta framework writes delta = cntr_value (since restart) over interval = spacing between collections, which under-states the rate (the counter accumulated over less than the spacing). Separate defect class, separate lane — reported.
• Does not add a WHERE to daily_summary_v2 (declined with reasons above).
• Does not tick item 13 — Q7 (cpu_utilization.sample_time UTC) and Q8 (utc_offset zone id) still belong to another lane.

Executed

CI runs T-SQL on the fresh-install path only: sql-validation.yml installs install/01–56 (minus 45, 97–99) once on SQL Server 2017 / 2019 / 2022 / 2025 containers, then seeds one row per table and materialises every report.* view — all four green on this PR, so the new 02 definition compiles on every supported engine and daily_summary_v2 executes against it. What CI cannot see is the upgrade of an existing old-shape table, the second pass (idempotency — IdempotencyTests is excluded in build.yml; it needs a real SQL2022 host), and the procedure's runtime branch. Those ran here:

mcr.microsoft.com/mssql/server:2022-latest (RTM-CU26, x64 under emulation on the arm64 Mac, desktop-linux context), scratch PerformanceMonitor database via the real install/01:

step	result
HEAD's (old) 02, seed (7, 60), (0, 0), (NULL, NULL), (59, 60)	definition ([cntr_value_delta]/nullif([sample_interval_seconds],(0))), bigint, is_persisted 0; rates 0, NULL, NULL, 0
this branch's 06, run 1	PRINTs Dropped … + Re-added …; definition (([cntr_value_delta]*(1.0))/nullif([sample_interval_seconds],(0))), numeric(33,12), is_persisted 0; rates 0.116666666666, NULL, NULL, 0.983333333333
06, run 2	silent; column_id unchanged → no-op
reset to old shape, EXECUTE config.ensure_collection_table @table_name = N'perfmon_stats', @debug = 1 ×2	run 1 rewrites and logs exactly one SCHEMA_UPGRADE row; run 2 no-op, still one row
interrupted state (column dropped by hand), 06	Re-added … only — recovers
DROP TABLE, this branch's 02 fresh	real shape, numeric(33,12), 0.116666666666
07–56 (minus 45 Agent jobs) in order, then the whole 01–56 sequence a second time	0 failures both passes; 06 silent on pass 2
report.daily_summary_v2 sort_order 20 on the seeded rows	old column averages 0; new column 0.549999999999 (numeric)
live collect.perfmon_stats_collector ×2 + delta framework	334 rows; first collection is (0, 0) → rate NULL; second collection shows fractional rates (Log Flushes/sec 1/3 → 0.333…) that were 0 before

Builds: deprecated/Installer.Core (embeds install/**/*.sql) and deprecated/Installer.Tests — -c Release -p:EnableWindowsTargeting=true, 0 warnings; CI's Installer.Tests selection 194/194 green locally.

Container torn down after the run.