Skip to content

Commit 35249e2

Browse files
Fix #502: hand back the full query, and say when the plan's copy is short
SQL Server caps StatementText at 4,000 characters when it writes showplan XML. Everything that read it inherited the cap: Copy Query Text, Open in Query Editor, the Ctrl+C copy guard, and the advice built from the parsed plan. The reporter's symptom was the second-order one -- a statement cut mid-token is not valid T-SQL, so re-running or formatting it failed with a syntax error that pointed nowhere near the truncation. Two halves. Say so. Rule 39 raises an Info warning when a statement hits the cap, so the plan reports it instead of leaving the user to work out why their query stops early. Because it is a plan warning it reaches the Statements grid count, Human Advice, and the Robot Advice JSON -- the advice still analyzes the short text, but it is no longer silent about it. The 3990 literal that Rule 3 already used for the same test moved onto PlanStatement as one definition. Hand back the real query. PlanViewerControl already holds the text a plan was captured from; GetQueryTextFromPlan preferred it, the statement menu did not. It now does, for a truncated single-statement plan. Single-statement only, deliberately: the captured text is the whole batch and showplan records no statement offsets, so there is no way to tell which slice belongs to the selected row, and a confidently wrong statement is worse than a short one. Rule 39 still fires either way. RunnableStatementText carried a comment saying the plan is the only source because the editor buffer is wrong for plans opened from a file or Query Store. That reasoning is sound but it is about the live buffer; this is the text captured at LoadPlan, which is null for a file-opened plan and the stored query for Query Store. The comment now draws that distinction rather than being quietly contradicted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nD83xZyWPzKWhs7Gg9Vyq
1 parent 7e7d18f commit 35249e2

4 files changed

Lines changed: 261 additions & 11 deletions

File tree

‎src/PlanViewer.App/Controls/PlanViewerControl.Statements.cs‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System;
1+
using System;
22
using System.Collections.Generic;
33
using System.Linq;
44
using Avalonia;
@@ -189,7 +189,7 @@ private void StatementsContextMenu_Opening(object? sender, System.ComponentModel
189189
private void UpdateStatementMenuForSelection()
190190
{
191191
var substitutable = StatementsGrid.SelectedItem is StatementRow row
192-
&& ParameterSubstitution.Apply(row.Statement.StatementText, row.Statement.Parameters)
192+
&& ParameterSubstitution.Apply(PlanOrCapturedStatementText(row.Statement), row.Statement.Parameters)
193193
.SubstitutionCount > 0;
194194

195195
// Say so on the label rather than substituting silently: the text the plan records and the
@@ -219,7 +219,7 @@ private async void CopyStatementText_Click(object? sender, RoutedEventArgs e)
219219
private async void CopyParameterizedStatementText_Click(object? sender, RoutedEventArgs e)
220220
{
221221
if (StatementsGrid.SelectedItem is not StatementRow row) return;
222-
var text = row.Statement.StatementText;
222+
var text = PlanOrCapturedStatementText(row.Statement);
223223
if (string.IsNullOrEmpty(text)) return;
224224

225225
await ClipboardHelper.TrySetTextAsync(this, text);
@@ -237,13 +237,34 @@ private void OpenInEditor_Click(object? sender, RoutedEventArgs e)
237237
/// <summary>
238238
/// The statement text with the plan's parameter values put back, or the text as-is when the plan
239239
/// carries no values to put back.
240+
/// </summary>
241+
private string RunnableStatementText(PlanStatement statement) =>
242+
ParameterSubstitution.Apply(PlanOrCapturedStatementText(statement), statement.Parameters).Text;
243+
244+
/// <summary>
245+
/// The text to hand the user for a statement: the plan's copy normally, the text this plan was
246+
/// captured from when the plan's copy has been truncated (#502).
247+
///
248+
/// <para>This used to read the plan and nothing else, on the reasoning that the query editor's
249+
/// buffer is only right when the user just executed from that tab. That reasoning still holds
250+
/// for the buffer — but this is not the buffer. It is the text handed to <c>LoadPlan</c> at the
251+
/// moment the plan was captured, which is null for a plan opened from a file and is the stored
252+
/// query for one opened from Query Store. When it is there, it is the same query the plan
253+
/// describes, minus SQL Server's 4,000-character cap.</para>
240254
///
241-
/// <para>The plan is the only source used. The query editor's buffer holds the original text in
242-
/// exactly one case — the user just executed it from that tab — and is wrong for a plan opened
243-
/// from a file, from Query Store, or from another session.</para>
255+
/// <para>Single-statement plans only. The captured text is the whole batch, and showplan records
256+
/// no statement offsets, so for a multi-statement batch there is no way to tell which slice of it
257+
/// belongs to the row the user picked — and handing back a confidently wrong statement is worse
258+
/// than handing back a short one. Rule 39 says so on the plan either way.</para>
244259
/// </summary>
245-
private static string RunnableStatementText(PlanStatement statement) =>
246-
ParameterSubstitution.Apply(statement.StatementText, statement.Parameters).Text;
260+
private string PlanOrCapturedStatementText(PlanStatement statement) =>
261+
statement.IsTextTruncated && !string.IsNullOrEmpty(_queryText) && IsSingleStatementPlan
262+
? _queryText!
263+
: statement.StatementText;
264+
265+
/// <summary>True when the loaded plan holds exactly one statement across all of its batches.</summary>
266+
private bool IsSingleStatementPlan =>
267+
_currentPlan != null && _currentPlan.Batches.Sum(b => b.Statements.Count) == 1;
247268

248269
private static void CollectNodeWarnings(PlanNode node, List<PlanWarning> warnings)
249270
{

‎src/PlanViewer.Core/Models/PlanModels.cs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System.Collections.Generic;
1+
using System.Collections.Generic;
22
using System.Linq;
33

44
namespace PlanViewer.Core.Models;
@@ -31,7 +31,22 @@ public class PlanBatch
3131

3232
public class PlanStatement
3333
{
34+
/// <summary>
35+
/// SQL Server caps StatementText at 4,000 characters when it writes showplan XML, so a
36+
/// statement at or near that length is almost certainly missing its tail. The few characters
37+
/// of margin absorb the trimming the server does around the boundary (#502).
38+
/// </summary>
39+
public const int TruncationLengthThreshold = 3990;
40+
3441
public string StatementText { get; set; } = "";
42+
43+
/// <summary>
44+
/// True when <see cref="StatementText"/> looks like it hit the showplan cap. The plan carries
45+
/// no marker for this, so length is the only signal there is — and the consequence is not
46+
/// cosmetic: a statement cut mid-token is not valid T-SQL, so anything that tries to re-run or
47+
/// format it fails for reasons that look unrelated to the plan (#502).
48+
/// </summary>
49+
public bool IsTextTruncated => StatementText.Length >= TruncationLengthThreshold;
3550
public string StatementType { get; set; } = "";
3651
public double StatementSubTreeCost { get; set; }
3752
public double StatementEstRows { get; set; }

‎src/PlanViewer.Core/Services/PlanAnalyzer.Statement.cs‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System;
1+
using System;
22
using System.Collections.Generic;
33
using System.Linq;
44
using System.Text.RegularExpressions;
@@ -22,6 +22,33 @@ private static void AnalyzeStatement(PlanStatement stmt, AnalyzerConfig cfg, Ser
2222
Rule38_StandardEditionDop(stmt, cfg, serverMetadata);
2323
Rule30_MissingIndexQuality(stmt, cfg, serverMetadata);
2424
Rule22Stmt_TableVariable(stmt, cfg, serverMetadata);
25+
Rule39_TruncatedStatementText(stmt, cfg, serverMetadata);
26+
}
27+
28+
/// <summary>
29+
/// Rule 39: the plan's copy of the query text hit SQL Server's showplan cap (#502).
30+
///
31+
/// <para>Worth saying out loud rather than leaving the user to work it out. Everything
32+
/// downstream of the plan reads this text — the advice on this screen, Copy Query Text, Open in
33+
/// Query Editor — so all of them show a query that stops mid-statement, and re-running or
34+
/// formatting that text fails with a syntax error that points nowhere near the real cause. The
35+
/// full query is not in the plan at all; only the session that ran it still has it.</para>
36+
/// </summary>
37+
private static void Rule39_TruncatedStatementText(PlanStatement stmt, AnalyzerConfig cfg, ServerMetadata? serverMetadata)
38+
{
39+
if (cfg.IsRuleDisabled(39) || !stmt.IsTextTruncated)
40+
return;
41+
42+
stmt.PlanWarnings.Add(new PlanWarning
43+
{
44+
WarningType = "Truncated Query Text",
45+
Message =
46+
"SQL Server truncated this query's text at 4,000 characters when it wrote the plan, "
47+
+ "so the query shown here stops early and is not valid T-SQL on its own. "
48+
+ "Advice, copied text, and Open in Query Editor are all working from the shortened "
49+
+ "version. Go back to the original query text to re-run or format it.",
50+
Severity = PlanWarningSeverity.Info
51+
});
2552
}
2653

2754
private static void Rule03_SerialPlan(PlanStatement stmt, AnalyzerConfig cfg, ServerMetadata? serverMetadata)
@@ -105,7 +132,7 @@ private static void Rule03_SerialPlan(PlanStatement stmt, AnalyzerConfig cfg, Se
105132
{
106133
var text = stmt.StatementText ?? "";
107134
var hasMaxdop1InText = Regex.IsMatch(text, @"MAXDOP\s+1\b", RegexOptions.IgnoreCase);
108-
var isTruncated = text.Length >= 3990;
135+
var isTruncated = stmt.IsTextTruncated;
109136

110137
if (hasMaxdop1InText)
111138
{
Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
using System;
2+
using System.IO;
3+
using System.Linq;
4+
using Avalonia.Controls;
5+
using Avalonia.Input;
6+
using Avalonia.Interactivity;
7+
using Avalonia.LogicalTree;
8+
using PlanViewer.App.Controls;
9+
using PlanViewer.Core.Models;
10+
using PlanViewer.Core.Services;
11+
12+
namespace PlanViewer.Core.Tests;
13+
14+
/// <summary>
15+
/// #502: SQL Server caps StatementText at 4,000 characters when it writes showplan XML, so for a
16+
/// long query every consumer of the plan — advice, Copy Query Text, Open in Query Editor — was
17+
/// handing back a query that stops mid-statement. The reporter's symptom was the second-order one:
18+
/// the shortened text is not valid T-SQL, so re-running or formatting it failed with a syntax error
19+
/// that pointed nowhere near the truncation.
20+
///
21+
/// <para>Two halves. The plan says so (Rule 39), and where the full query was captured alongside the
22+
/// plan it is handed out instead of the plan's short copy.</para>
23+
/// </summary>
24+
public class TruncatedStatementTextTests
25+
{
26+
private const string CapturedQuery = "SELECT 'the full query the session actually ran';";
27+
28+
// ---------------------------------------------------------------
29+
// Rule 39: say so on the plan
30+
// ---------------------------------------------------------------
31+
32+
[Fact]
33+
public void Rule39_FlagsAStatementThatHitTheCap()
34+
{
35+
var plan = AnalyzeStatementOfLength(PlanStatement.TruncationLengthThreshold);
36+
37+
var warning = Assert.Single(PlanTestHelper.WarningsOfType(plan, "Truncated Query Text"));
38+
Assert.Equal(PlanWarningSeverity.Info, warning.Severity);
39+
Assert.Contains("4,000 characters", warning.Message);
40+
}
41+
42+
[Fact]
43+
public void Rule39_SaysNothingAboutAQueryThatFits()
44+
{
45+
var plan = AnalyzeStatementOfLength(PlanStatement.TruncationLengthThreshold - 1);
46+
47+
Assert.Empty(PlanTestHelper.WarningsOfType(plan, "Truncated Query Text"));
48+
}
49+
50+
[Fact]
51+
public void Rule39_CanBeTurnedOff()
52+
{
53+
var stmt = StatementOfLength(PlanStatement.TruncationLengthThreshold);
54+
var plan = new ParsedPlan();
55+
plan.Batches.Add(new PlanBatch { Statements = { stmt } });
56+
57+
PlanAnalyzer.Analyze(plan, new AnalyzerConfig { Rules = new RulesConfig { Disabled = { 39 } } });
58+
59+
Assert.Empty(PlanTestHelper.WarningsOfType(plan, "Truncated Query Text"));
60+
}
61+
62+
// ---------------------------------------------------------------
63+
// Hand back the captured query instead of the plan's short copy
64+
// ---------------------------------------------------------------
65+
66+
[Fact]
67+
public void SingleStatement_OpenInQueryEditor_HandsBackTheCapturedQuery()
68+
{
69+
HeadlessUi.Run(() =>
70+
{
71+
var viewer = LoadPlan("isnull_plan.sqlplan", Truncated, CapturedQuery);
72+
73+
Assert.Equal(CapturedQuery, OpenInEditorText(viewer));
74+
});
75+
}
76+
77+
/// <summary>
78+
/// A plan opened from a file has no captured text, and nothing can invent it — the short copy is
79+
/// all there is. Rule 39 is what tells the user why.
80+
/// </summary>
81+
[Fact]
82+
public void SingleStatement_WithNothingCaptured_StillHandsBackThePlanText()
83+
{
84+
HeadlessUi.Run(() =>
85+
{
86+
var viewer = LoadPlan("isnull_plan.sqlplan", Truncated, queryText: null);
87+
88+
var text = OpenInEditorText(viewer);
89+
Assert.NotEqual(CapturedQuery, text);
90+
Assert.True(text!.Length >= PlanStatement.TruncationLengthThreshold);
91+
});
92+
}
93+
94+
/// <summary>
95+
/// The captured text is the whole batch and showplan records no statement offsets, so for a
96+
/// multi-statement plan there is no way to tell which slice belongs to the selected row. Handing
97+
/// back a confidently wrong statement would be worse than handing back a short one.
98+
/// </summary>
99+
[Fact]
100+
public void MultiStatement_KeepsThePlanTextEvenThoughItIsTruncated()
101+
{
102+
HeadlessUi.Run(() =>
103+
{
104+
var viewer = LoadPlan("eager_table_spool_plan.sqlplan", Truncated, CapturedQuery);
105+
106+
Assert.NotEqual(CapturedQuery, OpenInEditorText(viewer));
107+
});
108+
}
109+
110+
[Fact]
111+
public void UntruncatedPlan_IsUnaffectedByAnyOfThis()
112+
{
113+
HeadlessUi.Run(() =>
114+
{
115+
var viewer = LoadPlan("isnull_plan.sqlplan", xml => xml, CapturedQuery);
116+
117+
Assert.NotEqual(CapturedQuery, OpenInEditorText(viewer));
118+
});
119+
}
120+
121+
// ---------------------------------------------------------------
122+
// Helpers
123+
// ---------------------------------------------------------------
124+
125+
private static PlanStatement StatementOfLength(int length) =>
126+
new() { StatementText = new string('x', length), StatementType = "SELECT" };
127+
128+
private static ParsedPlan AnalyzeStatementOfLength(int length)
129+
{
130+
var plan = new ParsedPlan();
131+
plan.Batches.Add(new PlanBatch { Statements = { StatementOfLength(length) } });
132+
PlanAnalyzer.Analyze(plan);
133+
return plan;
134+
}
135+
136+
/// <summary>
137+
/// Pads the plan's first recorded statement past the cap, so the fixture looks the way a real
138+
/// capped plan looks. Letters and a space only — this goes back into an XML attribute.
139+
/// </summary>
140+
private static string Truncated(string xml)
141+
{
142+
const string marker = "StatementText=\"";
143+
var start = xml.IndexOf(marker, StringComparison.Ordinal);
144+
Assert.True(start >= 0, "fixture has no StatementText to pad");
145+
146+
var valueEnd = xml.IndexOf('"', start + marker.Length);
147+
Assert.True(valueEnd > start, "fixture's StatementText is not terminated");
148+
149+
return xml[..valueEnd]
150+
+ " " + new string('x', PlanStatement.TruncationLengthThreshold)
151+
+ xml[valueEnd..];
152+
}
153+
154+
private static PlanViewerControl LoadPlan(string planFileName, Func<string, string> shape, string? queryText)
155+
{
156+
var path = Path.Combine("Plans", planFileName);
157+
Assert.True(File.Exists(path), $"Test plan not found: {path}");
158+
var xml = shape(File.ReadAllText(path).Replace("encoding=\"utf-16\"", "encoding=\"utf-8\""));
159+
160+
var viewer = new PlanViewerControl();
161+
Assert.True(viewer.LoadPlan(xml, planFileName, queryText), $"Plan failed to load: {viewer.LastLoadError}");
162+
163+
return viewer;
164+
}
165+
166+
/// <summary>
167+
/// Drives the menu entry the reporter used and returns the text it handed over. Goes through the
168+
/// real right-click so the Opening event configures the menu first, the way #467 established.
169+
/// </summary>
170+
private static string? OpenInEditorText(PlanViewerControl viewer)
171+
{
172+
var window = new Window { Content = viewer, Width = 1400, Height = 900 };
173+
window.Show();
174+
window.UpdateLayout();
175+
176+
var grid = viewer.GetLogicalDescendants().OfType<DataGrid>().First(g => g.Name == "StatementsGrid");
177+
grid.RaiseEvent(new ContextRequestedEventArgs());
178+
179+
string? opened = null;
180+
viewer.OpenInEditorRequested += (_, text) => opened = text;
181+
182+
grid.ContextMenu!.Items.OfType<MenuItem>().First(i => i.Name == "OpenStatementInEditorItem")
183+
.RaiseEvent(new RoutedEventArgs(MenuItem.ClickEvent));
184+
185+
return opened;
186+
}
187+
}

0 commit comments

Comments
 (0)