Skip to content

Address issue #178 round 3 feedback (items 17-25)#213

Merged
erikdarlingdata merged 4 commits intodevfrom
issue-178-round-3
Apr 9, 2026
Merged

Address issue #178 round 3 feedback (items 17-25)#213
erikdarlingdata merged 4 commits intodevfrom
issue-178-round-3

Conversation

@erikdarlingdata
Copy link
Copy Markdown
Owner

Summary

Addresses all items from Joe Obbish's round 3 feedback on issue #178, plus a parser bug fix.

PlanAnalyzer changes

Parser fixes

  • Fix <Statements> parsing to iterate all children (multi-statement batches with DECLARE + SELECT)
  • Add synthetic root nodes for DECLARE/ASSIGN statements so they appear with proper operator icons

Test plan

  • All 72 unit tests pass
  • Verified against Joe's example plans (issues_19_and_20, issues_21_to_25, statement_1_and_not_statement_2)
  • Sync changes to PerformanceMonitor Dashboard and Lite

🤖 Generated with Claude Code

erikdarlingdata and others added 4 commits April 9, 2026 08:39
- #17: Bump SubTreeCost threshold from 0.01 to 1.0 (Rules 3 & 20) — CTFP is an integer
- #18: Smarter MaxDOPSetToOne severity — check query text for MAXDOP 1, detect truncation
- #19: Already fixed by #20
- #20: Scope "allocated resources" message to Hash/Sort/Spool operators only
- #21: Fix non-SARGable false positive when function is on parameter side
- #22: Enrich parallelism warnings with targeted wait stats advice
- #23: Enrich scan-with-predicate with cost %, elapsed %, row selectivity; elevate to Critical

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
A Batch element can contain multiple <Statements> children (e.g., one
for DECLARE and one for the SELECT). The parser used .Element() which
only reads the first, causing "no plan loaded" when the actual query
plan was in the second <Statements> block. Changed to .Elements() to
iterate all of them.

Reported by Joe Obbish via email (related to issue #178).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Statements with no QueryPlan (like DECLARE/ASSIGN) were filtered from
the statement tab list because they had no RootNode. Now creates a
synthetic root node with the appropriate icon (assign/declare) so they
appear as tabs alongside the actual query plans.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rule 32 (actual plans): When an expensive scan (>= 50% of plan) has a
>= 10x row overestimate and < 10% selectivity, flag that the bad
estimate likely caused the optimizer to choose a scan over a seek.

Rule 33 (estimated plans): Detect well-known CE default selectivity
guesses (30%, 10%, 9%, ~16.4%, 1%) on expensive scans against large
tables (>= 100K rows). These patterns suggest the optimizer is using
a default guess instead of accurate statistics.

Addresses issue #178 items #24 and #25 (Joe Obbish feedback).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@erikdarlingdata erikdarlingdata merged commit f4df0b2 into dev Apr 9, 2026
2 checks passed
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.

1 participant