Surface Query Store plan-load failures instead of silent 'No Plan Loaded'#367
Merged
Merged
Conversation
…ded" Loading a plan from the Query Store grid could blank to "No Plan Loaded" while the grid vanished, with no indication why. Two compounding causes: - PlanViewerControl.LoadPlan treated a failed parse identically to an empty plan: it never inspected ParsedPlan.ParseError (which ShowPlanParser sets instead of throwing) and just showed the empty state on zero statements. - AddPlanTab unconditionally added and selected the new plan tab even when the load failed, navigating away from the grid to a blank tab. (Commit e8e5a21's parser hardening was the regression vector: it converted a tree-walk exception on a bad plan from a visible crash into a silent zero-statement ParseError, producing exactly this symptom.) LoadPlan now returns bool and exposes LastLoadError, distinguishing blank/NULL plan XML, a parse error (surfaced verbatim), and parsed-but-no-statements. AddPlanTab no longer switches away on failure -- it keeps the grid active and leaves a persistent status explaining why. Clear() resets the empty state so a reused viewer cannot show a stale error. Co-Authored-By: Claude Opus 4.8 (1M context) <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
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.
Problem
Loading a plan from the Query Store grid could blank to "No Plan Loaded" while the QS grid itself vanished, with no explanation.
Root cause
PlanViewerControl.LoadPlantreated a failed parse identically to an empty plan:ShowPlanParser.Parsenever throws — it records failures inParsedPlan.ParseErrorand returns a zero-statement plan — but the desktop app never readParseError, so any parse failure rendered as a bare "No Plan Loaded".AddPlanTabunconditionally added and selected the new plan tab even when the load failed, navigating away from the grid to that blank tab.Commit
e8e5a21(parser hardening) was the regression vector: it wrapped the parser tree-walk in a catch-all, converting what used to be a visible crash on a bad plan into a silent zero-statementParseError— i.e. exactly this symptom.Fix
LoadPlanreturnsbooland exposesLastLoadError, distinguishing blank/NULL plan XML, a parse error (surfaced verbatim in the empty-state panel), and parsed-but-no-statements.AddPlanTabno longer switches away on failure — it keeps the Query Store grid active and leaves a persistent status line explaining why.Clear()resets the empty state so a reused viewer can't show a stale error (caught by the Avalonia gotcha review).The four direct
LoadPlancallers (File→Open, paste, estimated/actual execution) ignore the new return value and keep their prior behavior; they additionally benefit from real error text in the empty state.Note
This makes failures visible and non-destructive; it does not alter the parser. If a specific plan still fails, the on-screen "Parse error: …" now names the cause so the underlying parser issue can be fixed separately.
Testing
dotnet buildPlanViewer.App: 0 errors.🤖 Generated with Claude Code