Release v1.22.0 - #461
Merged
Merged
Conversation
An EXEC <procedure> plan analyzed as one statement, no warnings, cost 0, exit 0, on a file carrying dozens of statement plans. Reproduced against SQL Server 2025 before touching anything: six StmtSimple, four QueryPlan, summed cost 1.88, and `analyze` reported total_statements 1 and max_estimated_cost 0. The reported diagnosis was that the parse never descends into the procedure. It is subtler than that, and the distinction is the fix. ShowPlanParser has ALWAYS read StoredProc sub-plans - but that code sits below an early return taken when a statement carries no QueryPlan of its own, and an EXEC statement is precisely a statement with no plan of its own, because every plan lives in the body. The descent existed and was unreachable in the only case it was written for. The same was true of a UDF call whose calling statement carries no plan. So the sub-plan parsing moves above that early return. That alone fixes it. Two more places had the same blind spot and are now sharing one traversal, because the traversal was never the missing part - PlanOperations.ValidateComplexity has always descended, which is how the complexity limit counted statements the analysis never saw: - PlanAnalyzer walked batch.Statements, so no rule ever ran on a procedure body. - ResultMapper walked batch.Statements, which is where total_statements 1 and max_estimated_cost 0 came from. And a third, which is the one worth pausing on: PlanTestHelper.AllWarnings walked batch.Statements too. The golden master and the analyzer shared a blind spot, so the characterization test could not have caught the analyzer skipping procedure bodies no matter how many procedure plans were committed. A test that cannot see what the code cannot see is not covering it. It now uses the same traversal. What this does NOT change: no committed plan's verdict moves. Regenerating WarningBaseline.txt across the corpus produces additions only - the new fixture and nothing else - because the fix only ever adds statements that were being dropped. The CLI output hash is unchanged for the same reason: a plain batch enumerates exactly as before. Also caught on the way in, and worth knowing: PlanViewer.Web compiles Core sources through an explicit file list rather than a glob, so a new Core file breaks the solution build until it is added there. It is the same shape of trap as the call sites in #438 and #439 - something you must remember at a second location - and I walked into it. Tested: 327 passing, 0 failed. The new tests fail against the original parser - three of them, exactly the three asserting the body is reached, while the ordering test and the unchanged-plan cases correctly still pass. Verified by reverting the parser rather than assumed. Reported by samplesty, with a genuinely good writeup: file statistics, the contrast against StmtCond working correctly, and the observation that the output is plausible rather than obviously broken, which is what makes it worth fixing rather than documenting. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bumps Microsoft.SqlServer.TransactSql.ScriptDom from 180.78.1 to 180.102.0 --- updated-dependencies: - dependency-name: Microsoft.SqlServer.TransactSql.ScriptDom dependency-version: 180.102.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: patch-and-minor - dependency-name: Microsoft.SqlServer.TransactSql.ScriptDom dependency-version: 180.102.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: patch-and-minor - dependency-name: Microsoft.SqlServer.TransactSql.ScriptDom dependency-version: 180.102.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: patch-and-minor - dependency-name: Microsoft.SqlServer.TransactSql.ScriptDom dependency-version: 180.102.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: patch-and-minor ... Signed-off-by: dependabot[bot] <support@github.com>
…nViewer.App/dev/patch-and-minor-bf92bcdfd4 deps: Bump the patch-and-minor group with 1 update
The app has opened .sql files since the first release: the picker behind "Open .sqlplan..." carries a SQL Scripts filter, OpenFileByExtension routes .sql to LoadSqlFile, and drag and drop accepts it. Nothing in the menu says so, so a reporter asked for a feature that already shipped. He was right that it was missing - it was missing from the menu, which is the only place he could have looked. Open Query... and Save Query... now sit under New Query, where someone looking for query operations is already looking. Saving always prompts but defaults to the file the query came from, so writing back over it is the easy path and Save As is the same gesture. Sessions gain SourceFilePath, matching PlanViewerControl, which is what lets the picker start where the file lives and lets the tab retitle itself when a query is saved somewhere new. Also fixes a startup crash found while testing this. The error dialog was ShowDialog(this) unconditionally, but the command-line open and the restore of the previous session's tabs both run from the MainWindow constructor, before the window is visible - so a missing or corrupt file named on the command line threw "Cannot show window with non-visible owner" instead of reporting the problem. It is shown ownerless until the window is up.
Co-authored-by: Claude Opus 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
1 participant
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.
Release v1.22.0.
Descend into stored procedure bodies when analyzing a plan (#455, #456)
planview analyzeon a plan captured aroundEXEC <procedure>reported one statement, zero cost and zero warnings — a well-formed, plausible answer that happened to be about none of the plan.ShowPlanParserhad always readStoredProcsub-plans, but that code sat below an early return taken when a statement carries noQueryPlanof its own, which is exactly what anEXECstatement is. The descent existed and was unreachable in the only case it was written for. Three consumers shared the blind spot, including the test helper, so no committed procedure plan could have caught it.Open Query and Save Query in the File menu (#458, #459)
.sqlfiles could always be opened, but only through Open .sqlplan..., which gave nobody a reason to think so. Queries now get their own Open Query... (Ctrl+Shift+O) and Save Query... (Ctrl+S) under New Query. Save defaults to the file the query came from and opens the picker in that folder; saving somewhere new retitles the tab.Startup crash on an unreadable file passed at launch
Found by a test for the new save path. The file-error dialog called
ShowDialogagainst the main window before it was visible, so a corrupt or missing plan on the command line crashed the app instead of reporting the problem. Present since the first release.Plus a Dependabot patch-and-minor bump (#457) and the version bump (#460).