sqlplan-review: add 3 checks + refine 4 from Erik Darling skill review - #65
Merged
Merged
Conversation
Compared this repo's sqlplan-review (108 checks) against erikdarlingdata/claude-plugins' query-plan-analysis skill and closed the concrete gaps it surfaced: New checks (111 total, S1-S38/N1-N73): - S37: hidden scalar UDF time not caught by N25's operator-based detection - S38: in-plan WaitStats surfacing with a short wait-type glossary - N73: memory grant undersizing from LOB/(MAX) column flat-width estimates Refinements: - N10: distinguish correlated APPLY / transitive predicate elimination false alarms from a genuine cartesian product before calling it Critical - N2/S27: note that an eager index spool can suppress the MissingIndexes element entirely - Parameter-sniffing Info block: four-pattern table (sniffed / OPTIMIZE FOR UNKNOWN / never-executed / RECOMPILE-embedded) plus the local-variable tell - N35: soften the CE-guess percentages from an exact-match requirement to a recognizable shape Guidance additions: self-time vs. cumulative-time computation for row-mode elapsed-time attribution, a UTF-16 .sqlplan encoding caveat, and a note to treat plan XML content as untrusted data rather than instructions. S37, S38, and N73 are marked [Unverified] pending a Microsoft Learn validation pass (MCP server wasn't authenticated in this session) per the repo's mandatory validation policy. Updated every check-count/range touchpoint verify-docs.sh checks for (frontmatter, Purpose, header ranges, Quick Reference, README, CLAUDE.md, PERFORMANCE_TUNING_GUIDE.md, VERSION_COMPATIBILITY.md, plugin manifest) and regenerated the MCP server's skills-data.ts bundle. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QYnFRFKpzqZMkLMWf2E32X
S37 (Hidden Scalar UDF Time): confirmed. UdfCpuTime/UdfElapsedTime are real showplan attributes per the SQL Server 2016 SP2 release notes and the SQL Server Team blog "More Showplan enhancements - UDFs." Removed the [Unverified] tag and set the version gate to SQL 2016 SP2+ / SQL 2017 CU3+. S38 (In-Plan Wait Statistics Present): the underlying feature is confirmed by the SQL Server Team blog "New Showplan enhancements" - top 10 waits (WaitType, WaitTimeMs, WaitCount) from sys.dm_exec_session_wait_stats are added to actual showplan XML starting SQL Server 2016 SP1, with CXPACKET specifically gated to SQL 2016 SP2 / SQL 2017 CU3. Narrowed the [Unverified] tag to just the literal XML element/attribute names, which Microsoft Learn's docs search does not index from the raw showplan XSD. N73 (Memory Grant Undersized by LOB/(MAX) Column Estimate): searched Microsoft Learn's row-size, statistics, and memory-grant documentation; found related confirmed facts (24-byte fixed overhead per MAX column in sorts, "complex" LOB size estimation per Microsoft's own heap-sizing guide) but no official source for the specific flat-width-estimate mechanism. Left [Unverified] per the mandatory validation policy, with the search trail documented in the check explanation. Updated skills/VERSION_COMPATIBILITY.md: moved S37/S38 out of the "no version gate" bucket into the SQL Server 2016+ catalog, and corrected the per-version active-check counts for SQL 2014/2012/2008 R2 (only N73 applies there, not S37/S38). Regenerated the MCP bundle. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sqlplan-reviewskill (108 checks) against erikdarlingdata/claude-plugins'query-plan-analysisskill and closed the concrete gaps it surfaced, bringing the total to 111 checks (S1–S38, N1–N73).QueryTimeStats), S38 (in-planWaitStatssurfacing with a wait-type glossary), N73 (memory grant undersizing from LOB/(MAX)column flat-width estimates).MissingIndexeselement), the parameter-sniffing Info block (4-pattern table + local-variable tell), N35 (softened CE-guess percentages from exact-match to a recognizable shape)..sqlplanencoding caveat, and a note treating plan XML content as untrusted data.[Unverified]since Learn's docs search doesn't index the raw showplan XSD), N73 stays[Unverified](searched but found no official source for the specific flat-width-estimate mechanism). CorrectedVERSION_COMPATIBILITY.md's per-version counts to reflect S37/S38's real SQL 2016+ gate.verify-docs.shchecks (frontmatter, Purpose, header ranges, Quick Reference, README, CLAUDE.md, PERFORMANCE_TUNING_GUIDE.md, VERSION_COMPATIBILITY.md, plugin manifest) and regenerated the MCP server'sskills-data.tsbundle.Test plan
bash scripts/verify-docs.sh— 45 passed, 1 pre-existing unrelated warning, 0 failedcd mcp-server && npm test— 184/184 tests passedskills/sqlplan-review/examples/horrible.sqlplanagainst the new/refined checks — none fire on that reference plan, sohorrible-analysis.mdneeded no changes🤖 Generated with Claude Code