Skip to content

sp_QuickieStore: added the 'log' and 'total log' sort orders - #849

Merged
erikdarlingdata merged 1 commit into
erikdarlingdata:devfrom
ReeceGoding:sp-quickie-store-add-log-sort-order
Aug 7, 2026
Merged

sp_QuickieStore: added the 'log' and 'total log' sort orders#849
erikdarlingdata merged 1 commit into
erikdarlingdata:devfrom
ReeceGoding:sp-quickie-store-add-log-sort-order

Conversation

@ReeceGoding

@ReeceGoding ReeceGoding commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Added the 'log' and 'total log' sort orders to sp_QuickieStore. Closes #819.

sp_QuickieStore has a lot of features now, so I don't know if I've tested everywhere that I need to. Hopefully the screenshots are convincing enough? I've never taken the time to understand the new Pareto stuff but my impression is that you need to opt sort orders into that before you can break anything.

I still cannot shake the feeling that we used to have this feature.

Normal usage:

image image

Regression mode:

image image

@erikdarlingdata erikdarlingdata left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! Coverage is complete across all nine sort-order CASE sites, docs are updated in all three places, and the README Examples link fix is a nice drive-by. The avg log/average log prefix forms also work automatically, and there's no collision with @wait_filter = 'log' — I checked.

Two real problems need fixing before merge, and both are invisible to CI:

  1. Regression mode reports the log change 8,192x too large, on the default path. The @regression_comparator = 'absolute' display conversion (and absolute is the default) uses the 8KB-pages-to-MB formula for log/total log, but avg_log_bytes_used is in bytes, not pages. Details inline.

  2. SQL Server 2016 gets a hard error instead of the graceful fallback. avg_log_bytes_used only exists in sys.query_store_runtime_stats on 2017+/Azure (@new = 1). tempdb is protected twice — the up-front version check that reverts to cpu with a warning, and inline CASE WHEN @new = 1 fallbacks — log has neither. CI can't catch this because SQL Server on Linux starts at 2017. Details inline.

One style nit inline as well (missing N prefixes).

Non-blocking FYI: the @find_parameter_sensitive rank-metric whitelist (~line 7244) doesn't include 'log', so PS mode silently ranks by cpu. That's the documented fallback ("anything else = cpu"), fine to leave, but easy to add if you want it.

On your "I still cannot shake the feeling that we used to have this feature" — git log -S "'total log'" comes up empty for the file's whole history. The columns have been in the output since early on; the sort orders never existed. You were half-remembering the columns.

Comment thread sp_QuickieStore/sp_QuickieStore.sql Outdated
Comment thread sp_QuickieStore/sp_QuickieStore.sql
Comment thread sp_QuickieStore/sp_QuickieStore.sql Outdated
@ReeceGoding
ReeceGoding force-pushed the sp-quickie-store-add-log-sort-order branch from 7e9fabd to 671bd7b Compare August 4, 2026 21:46
@ReeceGoding

Copy link
Copy Markdown
Contributor Author

Addressed comments. Will re-test when I get a chance, but these small changes shouldn't break anything.

@erikdarlingdata erikdarlingdata left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick turnaround — the 2016 protection is now done properly and thoroughly: the up-front revert covers log/total log (and pulling total tempdb into that same gate is a nice consistency drive-by — it previously relied on the inline fallbacks alone, with no warning), and every inline arm carries the CASE WHEN @new = 1 fallback mirroring tempdb. The N-prefixes landed too.

Two things still block, and the second is the same one as last round:

1. Syntax error — the proc won't create. In the ORDER BY expression block, the 'total log' arm is missing its THEN:

WHEN 'total log' THEN CASE WHEN @new = 1 N'qsrs.avg_log_bytes_used * qsrs.count_executions' ELSE ...
                                        ^^ missing THEN

Compare the 'total tempdb' line directly below it. CI will catch this one when it runs (the proc fails to CREATE), but naming it saves you the round trip.

2. The absolute-regression conversion is still wrong — now 8× instead of 8,192×. Both 'log' and 'total log' arms now read:

N'(hashes_with_changes.change_since_regression_time_period * 8.) / 1048576.'

You corrected the divisor (KB→bytes) but kept the * 8. — that multiplier converts 8 KB pages to KB, and avg_log_bytes_used is already in bytes. Bytes → MB is simply:

N'hashes_with_changes.change_since_regression_time_period / 1048576.'

With the current expression, @regression_comparator = 'absolute' (the default) reports every log change 8× too large. CI cannot catch this one — it runs on 2017+ where the query executes happily and produces confidently wrong numbers, which is why I'd ask for one actual run of regression mode with @sort_order = 'log' against a known workload before the next push, rather than "shouldn't break anything" — that instinct is exactly what both of these slipped past.

Everything else from last round is resolved. Fix the THEN and drop the * 8. and this merges.

@ReeceGoding
ReeceGoding force-pushed the sp-quickie-store-add-log-sort-order branch from 671bd7b to e95a753 Compare August 6, 2026 21:42
@ReeceGoding

Copy link
Copy Markdown
Contributor Author

Addressed comments. Will re-test when I get a chance.

@ReeceGoding

Copy link
Copy Markdown
Contributor Author

Re-tested. I think this all works.

@erikdarlingdata
erikdarlingdata merged commit aeaf211 into erikdarlingdata:dev Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants