Repository navigation
Plan analysis: adaptive join note, truncated-text rule, UDF/serial dedup, invariant percentages (#4531, #4532, #4533, #4536) - #4548
Merged
Conversation
# Conflicts: # PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs
erikdarlingdata
marked this pull request as ready for review
September 28, 2026 03:22
This was referenced Sep 28, 2026
Closed
erikdarlingdata
added a commit
that referenced
this pull request
Sep 28, 2026
…00Z UTC (#4621) Moves the CHANGELOG entries carried in merged pull-request descriptions into [Unreleased]. The cut is PRs merged after 2026-09-26T17:37:33Z and at or before 2026-09-28T17:40:00Z; the next splice starts after it. - 76 PRs are spliced: Fixed 36, Changed 28 and Added 12, each counted once under its first section. That is 79 bullets: #4481's Fixed entry holds three, and #4548 adds a second group under Added. - 21 PRs have no user-visible entry (None, test-only, CI-only, or deferred to a parent). - The [#4509] link definition, which #4538 also carries, is defined once. - The IMPORTANT upgrade notes from #4489, #4495, #4501, #4506 and #4541 are held for the release cut and are not in this change. - Only CHANGELOG.md changes.
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
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.
Fixes #4531
Fixes #4532
Fixes #4533
Fixes #4536
Part of #4511
Why
Four small findings in plan analysis carried forward from PerformanceStudio (PS):
P0format, which is culture-sensitive in the space before the%sign (100%on en-US,100 %underInvariantCulture). That makes warning text differ by machine.What changes
Mirrors these PerformanceStudio commits, adapted to PM's single
PlanAnalyzer.cs(PS splits the analyzer into partial files) and with noAnalyzerConfig/IsRuleDisabledguard (PM has none):erikdarlingdata/PerformanceStudio@4a5efa5(adaptive-join part only): the Excessive Memory Grant finding now adds a note when an adaptive join executed as Nested Loops at runtime, viaHasAdaptiveJoinChoseNestedLoop(node.IsAdaptiveandnode.ActualJoinTypecontaining "Nested"). That commit's Serial Plan benefit-formula rework is a separate issue and isn't part of this change.erikdarlingdata/PerformanceStudio@35249e2(rule-39 part only):PlanStatement.IsTextTruncated(length ≥ 3,990) replaces the inline length check rule 3 already had, and a new rule 39 adds an Info finding per statement whose text hit the cap. The full-text-recovery half of that commit needs a captured-query buffer PM has no equivalent for, and isn't part of this port.erikdarlingdata/PerformanceStudio@495750fplus@cc18844's Lite: MCP tool coverage gaps — 8 collectors with no tools #576 fix: rule 6 now skips its Scalar UDF finding when the statement already carries a Serial Plan finding for one of the UDF-related non-parallel reasons (TSQLUserDefinedFunctionsNotParallelizable,CLRUserDefinedFunctionRequiresDataAccess,CouldNotGenerateValidParallelPlan). Both conditions are checked together — checking only the reason would drop the UDF finding whenever rule 3 was disabled or gated out, with nothing replacing it.erikdarlingdata/PerformanceStudio@582c88e: the three{pct:P0}/{skewRatio:P0}sites (the two spill "Operator time" messages and the parallel thread-skew message) now format as{pct * 100:N0}%, which is culture-independent.No change to
AnalyzerConfig,BenefitScorer, or any store rung. Consumers (PlanAdvisoryAggregator, the MCP plan tools, Lite's analysis, the plan viewer) readPlanWarning/PlanStatementthrough the same shapes; nothing new is exposed beyondPlanStatement.IsTextTruncated/TruncationLengthThresholdand the new "Truncated Query Text" warning type.Test plan
New tests, one class per issue, construct a
ParsedPlan/PlanStatement/PlanNodedirectly and run it throughPlanAnalyzer.Analyze— the same pattern as the existingPlanSync4524Tests:PlanSync4531Tests: an adaptive join withActualJoinTypecontaining "Nested" plus an excessive grant gets the note; the same shape with a Hash MatchActualJoinType, or a non-adaptive join, does not.PlanSync4532Tests:IsTextTruncatedat/under the 3,990 threshold; a 3,995-character statement gets the rule-39 Info finding, a 3,000-character one does not.PlanSync4533Tests: a serial plan withNonParallelPlanReason = TSQLUserDefinedFunctionsNotParallelizable(and the CLR variant) suppresses rule 6; the same UDF in a plan with no non-parallel reason (so no Serial Plan finding) still gets the rule-6 finding.PlanSync4536Tests: runs the parallel-skew path underCultureInfo.InvariantCultureand asserts the message contains100%with no space.Runtime RED on dev (each new file in a detached worktree at dev):
PlanSync4531Tests.AdaptiveJoinRanNestedLoops_NotesTheUnusedGrant:Assert.Contains() Failure … Not found: "adaptive join".PlanSync4533Tests: both UDF-reason facts fail withExpected: False, Actual: True(rule 6 still fired).PlanSync4536Tests:… "Thread 1 processed 100 % of rows …".PlanSync4532Testsdoes not compile on dev because two facts name the newPlanStatement.IsTextTruncatedmember. Its two rule-39 facts use only existing members, and dev has no "Truncated Query Text" rule, so they fail there by construction; the rule-39 mutation below shows the same failure.Mutations, each reverted:
HasAdaptiveJoinChoseNestedLoopshort-circuited to false (#4531); the rule-39 condition short-circuited to false (#4532); the rule-6 suppression disabled (#4533);P0restored at the thread-skew site (#4536). Each fails its issue's test.Run: the four new classes plus the plan-analysis classes:
Total: 242, Failed: 0, Skipped: 2(the live-Postgres plan-tool class runs in CI).Lite.Testsbuilds. No test enumerates warning types, so no census pin changed.Not ported: the full-query-text recovery half of PerformanceStudio@35249e2, which needs a captured query buffer PM does not have.
CHANGELOG
SECTION: Fixed
ENTRY: - Plan analysis: adaptive-join memory grants, redundant UDF findings, and culture-dependent percentages ([#4548]) - The Excessive Memory Grant finding now notes when an adaptive join ran as Nested Loops, so the unused grant isn't mistaken for a separate problem. Rule 6's Scalar UDF finding no longer duplicates a Serial Plan finding that already names the same UDF as the reason a plan runs serially. Three warning messages that reported a percentage now format it the same way regardless of the machine's locale.
REF: [#4548]: #4548
SECTION: Added
ENTRY: - Plan analysis flags a query whose text was cut off by SQL Server's showplan limit ([#4548]) - When a statement's copy of its query text hit SQL Server's roughly 4,000-character cap when the plan was captured, the analysis now says so instead of leaving the reader to work out why re-running or formatting that text fails.
REF: [#4548]: #4548