The Darling viewer's procedure comparison, the two comparison twins and Query Store top text lookups get a time bound (#5420)

[erikdarlingdata]

Closes #5420.

Why

On a large store the Darling viewer's procedure comparison took about 25 s. Of that, 17.6 s went on finding each procedure's statement text. There were two causes, and both were reproduced on a rig.

1. The text lookup had no time bound. No index covers sql_handle (the query_stats handle index was dropped in Five tuning indexes on the raw hypertables write about a quarter of the store's WAL, and none has been read more than 13 times across the retained chunks #4247). A procedure with no text anywhere walked every retained chunk on the time index.
2. The period joins used IS NOT DISTINCT FROM on three columns. That cannot hash. Each period ran as a nested loop that discarded millions of rows.

The Query Store top reads had the same unbounded shape in their inline-text fallback (query_store_stats), in the viewer and in the MCP reader.

What changes

• Procedure comparison (ViewerDataService.ProcedureStatsComparisonSql):
  • Each period is aggregated once over the window. It is then joined to the top procedures on plain equalities: COALESCE(x, '') = COALESCE(y, '') AND (x IS NULL) = (y IS NULL) per key. That is exactly NULL-safe, because a NULL key never matches an empty string, and it has no join filter.
  • The text is found once in a texts CTE. It scans v_query_stats over the current range and the baseline range (two ranges, not the span between them, see review round 1). It is narrowed to the compared handles, and DISTINCT ON (sql_handle) takes the newest row with a text. v_query_stats is kept, so the payload dimension (query_text/query_plan_xml stored inline per row: 94% of a field store — normalize into hash-keyed dimension tables (~135x measured) #1767) still resolves.
  • Deviation from the seat ruling, on purpose. The ruling asked for a time bound on the per-procedure LATERAL. I built that first. At 24 h it fell from 24.8 s to 1.1 s. At 7 d it fell only from 25.6 s to 12.4 s on the rig. Every procedure with no text still scanned the whole 14-day span once. The one-scan form costs the same however many procedures have no text, and it is the shape Lite already uses (Lite: a read that includes query text can run out of memory at the 1 GB DuckDB limit #5381). The bound is still there. Say if you want the LATERAL form back.
• The two comparison twins, Top Queries and Query Store (QueryStatsComparisonSql and QueryStoreComparisonSql):
  • Each period is aggregated once in a derived table. The top list joins to it on the same NULL-safe equality pair as the procedure comparison. The Query Store text left join moved inside that aggregate.
  • The database filter also moved inside the window aggregates, here and in the procedure comparison. Rows are identical.
• The outer FULL JOIN of all three comparisons keeps NULL and empty keys apart. It joined on COALESCE(key, '') alone. When a NULL-key group and an empty-key group both made the top 100, they merged into one row and fanned out. Each outer join now also carries (c.key IS NULL) = (b.key IS NULL) for every key column, so it stays a hash-joinable condition (EXPLAIN shows a Hash Join on all three). The doc comments no longer claim a key is never the empty string. Same rows for every seed with no empty-string key. New live fact per twin: the NULL and empty groups both sit in the top 100 of both windows. The answer has two separate rows, each against its own baseline. RED: dropping one IS NULL half in each outer join fails all three facts. It also fails the ComparisonSql_UnionsTop100_FullOuterJoins_NullSafe pin, which now counts both halves of every key in the outer join. Procedure keys can be NULL (all three columns are nullable in collect.procedure_stats), so it is covered the same way.
• Query Store top fallback: s.collection_time >= $2 and <= $3 on the inline-text subquery. This is in the viewer's shared tail and the MCP reader's shared tail (raw, table and daily reads). The viewer keeps its open-end form ($3::timestamp IS NULL OR ...).
• Lite: no change in this PR any more. GetQueryStoreTopQueriesAsync had the same unbounded LATERAL, and I bounded it here. Then Lite: a read that includes query text can run out of memory at the 1 GB DuckDB limit #5381 on dev replaced that lateral with a keyed text read over all history, on purpose, "as the old lateral picked". That conflicted with my change. I took dev's side and dropped the Lite test that pinned the bound. Lite's procedure comparison and Top Queries text picks were already bounded in Lite: a read that includes query text can run out of memory at the 1 GB DuckDB limit #5381, and DuckDB hashes IS NOT DISTINCT FROM itself, so they are untouched. Lite's procedure comparison now matches the viewer. The Query Store top text is the one place the two differ, and that was ruled: the viewer takes Query Store text from query_store_text first. That table has one row per query and no time bound. So the viewer shows the same text Lite reads from the query's latest row over all history. Only the rare inline fallback is bounded to the window. That is a query with no query_store_text row yet, and in PostgreSQL its lookup has no key index and walked every chunk. Lite is not bounded. A bound there drops text Lite shows today.
• One visible change, documented: a procedure or query whose only captured text is older than the window now shows no text. One with no text anywhere already did.
• No migration, no index. DatabaseFilterTests still sees 6 database-filter placeholders in the comparison.
• QueryStoreTopDailyReadLiveTests re-takes the two statement hashes on purpose, because the tail changed. The reason is in the test comment.

Twin list: viewer ProcedureStatsComparisonSql, viewer QueryStoreTopSql and QueryStoreTopTableSql, MCP DarlingDataReader.QueryStoreTopSql, QueryStoreTopTableSql and QueryStoreTopDailyTableSql (the daily route has the same fallback). Not changed, with reasons:

• The ViewerDataService.QueryStats hourly-arm text lookup. It is keyed on query_hash with a kept index, and it reads past the window on purpose.
• DarlingReproScript.QueryStatsTextSql. It is one click-through lookup by hash with no window.
• The MCP DarlingDataReader top-queries LATERALs. They are already bounded to the window.

Test plan

Rig: PostgreSQL 18.6 with TimescaleDB, 1-day chunks, 15 days seeded. It holds 202 procedures, 864k procedure_stats rows and 767k query_stats rows, and about 20% of procedures have no text. The rig is not the large store. Its absolute numbers are smaller than the 25 s measured there, so compare the ratios. Five runs each, EXPLAIN (ANALYZE) execution time, custom plans:

Window	Before (ms)	After (ms)
24 h	30491, 23773, 23796, 19731, 12486	280, 251, 234, 276, 230
7 d	25605, 27473, 25918, 26385, 24426	2082, 1568, 2460, 1821, 3105

The first before-run at 24 h was cold and the rest were warm. The period joins now plan as Merge Join, not Hash Join, on the pre-aggregated inputs of about 200 rows. Both are plain-equality joins with no join filter, so the pin accepts either.

• New ViewerTextLookupWindowBoundLiveTests, on its own scratch store with hypertables and 13 chunks. The new comparison reads at most 3 of 13 query_stats chunks, and its joins have no DISTINCT FROM filter. The pre-change statement on the same seed reads more than 3 chunks and has that join filter, which proves the shape test is not vacuous.
• Old-SQL oracle parity on all 11 columns for every row. The seed has NULL keys, an empty-string key, GONE and NEW procedures and dim-resolved text. Exactly 5 procedures change text, all of them text-only-before-window.
• Query Store fallback chunk bound and text results in both the viewer and MCP copy, and all five Query Store statements carry the bound.
• RED proofs: reverting the Query Store bounds in both files fails 5 of the first 8 facts. The comparison's shape test is proved against the pre-change statement (second fact) rather than by reverting the C#.
• Darling.Tests and Lite.Tests build with 0 warnings. The two xUnit1031 warnings came from dev and are gone after the merge.
• Not measured on the large store.

Review round 1

The head moved from 5be1af60f to the merge of dev plus this round. origin/dev conflicted on Lite/Services/LocalDataService.QueryStore.cs (see the Lite bullet above). Everything else merged clean.

• F1, the text scan covered the gap between the windows. The texts CTE now reads (collection_time >= $2 AND <= $3) OR (>= $4 AND <= $5). The ends are inclusive, as in the rest of the read and as in Lite's pick. Pin: TheComparison_WithABaselineAWeekBack_ReadsTheTwoWindowsChunksAndNotTheGapBetweenThem. A baseline of End-8d to End-7d reads at most 4 of 13 query_stats chunks. RED: reverting to LEAST/GREATEST makes it read 9 of 13, and the fact fails.
• F2, no tie-break on the newest text. The order is now qs.sql_handle, qs.collection_time DESC, qs.collection_id DESC, so the row collected last wins. Pin: WhenOneProceduresStatementsShareAnInstant_TheRowCollectedLastSuppliesTheText. It uses two procedures with two rows each at one instant, one inserted lower id first and one higher id first. RED: without the tie-break it returns "tie A: the lower collection id".
• F3, the two twin comparisons. QueryStatsComparisonSql and QueryStoreComparisonSql now aggregate each period once in a derived table w. The top list joins to w on the NULL-safe equality pair, as the procedure comparison does. The Query Store text left join moved inside w, and the [BUG] 3.4.0 query_store collector runs 37–100 min on Azure SQL DB (3.3.0 median: 4.8 s) — starves all other collectors #2150 and Query Store aggregates double-count re-collected cumulative intervals - tier 2: interval identity + CAGG rebuild (tier 1 read-side dedup shipped in #1845) #1841 comments are kept.
• F3 pin, plan: TheTwinComparison_JoinsItsPeriodsOnPlainEqualities_NotANullSafeJoinFilter. The plan has no DISTINCT FROM join filter and at least two hash or merge joins on query_hash. The old statement on the same seed does have the filter.
• F3 pin, rows: TheTwinComparison_AnswersTheOldStatementsAnswer_OnEveryColumn_NullKeysIncluded. The old statement is copied into the test as the oracle. The seed has NULL database, NULL hash, both-NULL, GONE and NEW keys. Every column is compared by exact double bits, filtered and unfiltered, for both twins.
• F3 RED: restoring the old twin statements fails both plan facts. The oracle facts then compare the old statement with itself, so they guard the fix and not the revert.
• F4: the PgTableTuning.cs comment for the dropped idx_query_stats_server_handle_time now says the LATERAL reader is gone. The text is found by one window scan.
• F5, done. The database filter is now also inside the w aggregates of the procedure comparison and the Top Queries comparison. The Query Store dedupe CTEs already had it. Results are identical: TheComparison_UnderADatabaseFilter_AnswersTheOldStatementsAnswer_ExceptTheDocumentedTextChange and the filtered twin oracle facts pass. There is no RED, because it changes the work done and not the rows. DatabaseFilterTests needed no count change, because it checks that the $6 predicate is present and not how often.
• Also fixed: ViewerQueriesSqlTests.ComparisonSql_UnionsTop100_FullOuterJoins_NullSafe still pinned the old IS NOT DISTINCT FROM and GROUP BY th... text. It was already failing for the procedure comparison on the first round's head. It now pins the new NULL-safe pair.

Twin timings come from my rig run on the review's seed: 505,500 query_stats rows over 14 days, 3,000 hashes and 673,000 query_store_stats rows. Each is EXPLAIN (ANALYZE), 3 warm runs. The rows are identical both ways (EXCEPT 0 and 0) on all four reads: 195, 195, 196 and 182 rows.

Statement	Window	Before (ms)	After (ms)
Top Queries comparison	24 h	1296, 1299, 1262	88, 86, 86
Top Queries comparison	7 d	8648, 8580, 8465	323, 318, 379
Query Store comparison	24 h	2661, 2674, 2707	561, 565, 619
Query Store comparison	7 d	16385, 16957, 16669	1988, 2012, 2401

The review measured 2.4 to 3.4 s, 10 to 16 s, 3.8 to 3.9 s and 18 to 24 s before. It measured 0.11 to 0.13 s, 0.6 to 0.9 s, 0.49 to 0.60 s and 2.1 to 2.3 s after, on its own rig.

Tests after the merge (rig port 5714, UTC, TimescaleDB, pg_stat_statements preloaded):

• ViewerTextLookupWindowBoundLiveTests: 17 of 17 pass (8 old, 9 new).
• Classes *ViewerTextLookupWindowBoundLiveTests*, *QueryStoreTopDailyReadLiveTests*, *DatabaseFilterTests*, *TuningReaderBudgetLiveTests*, *PayloadDimensionTests*, *StatementFilter*, *Viewer*, *Comparison*, *Statement*, *Census* and *Ratchet*: Total 4937, Failed 2.
• Both failures are in classes this PR does not touch: SensitiveStatementAutoParamTests.Measure_20MbPlan_WithACut_ThreeShapes_StayUnderTheCeilings and StatementFilterAlertTests.Engine_AFourMegabyteReport_ReachesTheDelivererFilteredNotWithheldWhole. They are timing measures that failed under the 6 minute parallel load and pass alone (58 of 58).
• Lite.Tests selected classes (*QueryStoreTop*, *QueryStoreDedupRead*, *QueryStatsTextAfterRanking*, *ProcedureStats*, *Census*, *Ratchet*, *StatementFilter*): Total 123, Failed 0.
• Full suites not run (CI decides).
• The Lite gap is ruled, not open. Lite is not bounded (see the Lite bullet). The viewer shows the same Query Store text as Lite, except for the rare inline fallback.

Review round 2

• Empty keys in the twin seeds. Before, the seeds held NULL keys but no empty-string key, so dropping an IS NULL half left every fact green. Both twin seeds now hold the pairs the procedure seed already had.
  • Top Queries: row 48 is the one empty hash, in the database the heavy NULL-hash group sits in (40, 80, 120). Row 127 is the one empty database, and row 51 is a NULL database on the same hash.
  • Query Store: row 47 is the empty hash, in the database of NULL-hash group 80. Rows 127 and 51 pair the same way as in Top Queries.
  • One partner of each pair is light on purpose (one execution per row), so it never makes the top 100. The final full join can only compare on COALESCE, in the old statement as well as the new one. With both partners in the top lists, that join collides them. With one light, the final join stays apart. The period join then fans a heavy row out if it merges the pair.
• Guards in the oracle fact. It checks that the heavy keys are in the answer. It checks that the light ones are not, and that the rows exist in the table.
• The pin checks both halves of every key column. ComparisonSql_UnionsTop100_FullOuterJoins_NullSafe counts the COALESCE half and the IS NULL half for every key. Each must appear exactly twice, once per period. That is two columns for the twins and three for the procedure comparison.
• RED. I removed the query_hash IS NULL half from the Top Queries twin and the database_name half from the Query Store twin. All four facts failed: the two pin cases and both oracle facts (duplicate key db0|<null> for Top Queries and |h127 for Query Store). Both halves are restored.
• Tests: *ViewerTextLookupWindowBoundLiveTests*, *ViewerQueriesSqlTests*, *DatabaseFilterTests*, *Census*, *Ratchet* and *DocCommentHygiene*: Total 673, Failed 0 (rig port 5720, UTC). Build 0 warnings.
• The timings above used 505,500 query_stats rows (Top Queries) and 673,000 query_store_stats rows (Query Store). No product code changed in this round.