diff --git a/src/PlanViewer.Core/Services/ParameterSubstitution.cs b/src/PlanViewer.Core/Services/ParameterSubstitution.cs
index 3ab9c52..0e3a2e9 100644
--- a/src/PlanViewer.Core/Services/ParameterSubstitution.cs
+++ b/src/PlanViewer.Core/Services/ParameterSubstitution.cs
@@ -47,46 +47,60 @@ public static ParameterSubstitutionResult Apply(
if (values.Count == 0)
return new ParameterSubstitutionResult(statementText, 0);
+ /* A plan from the plan cache or Query Store keeps an sp_executesql statement's declaration
+ list in front of it: "(@p1 int, @p2 int)SELECT …". The names in that list declare the
+ parameters, they do not read them. Substituted, they became "(10 int, 20 int)SELECT …",
+ which is neither the plan's text nor runnable. So only the statement after the list gets
+ values, and when it gets any, the list is dropped: this text is meant to run, and a
+ declaration list is not T-SQL on its own. Nothing substituted leaves the text as it was.
+ A list that never closes is text the plan cut off at 4,000 characters inside the list,
+ so there is no statement to put values into. */
+ var bodyStart = DeclarationListEnd(statementText);
+ if (bodyStart < 0)
+ return new ParameterSubstitutionResult(statementText, 0);
+
+ var text = bodyStart == 0 ? statementText : statementText[bodyStart..].TrimStart();
+
/* One fact about the whole statement, settled up front: inside an EXEC statement, every
token sitting to the left of an "=" is an assignment target — the return-status variable
or a named argument's name — because EXEC grammar has no other use for "=" at all. A
per-token back-scan cannot see this for the FIRST named argument (what precedes it is
the procedure name, not a keyword), which is how "EXEC dbo.p @debug = @debug" got its
left-hand side substituted into "EXEC dbo.p 1 = @debug". */
- var assignsThroughEquals = StatementLeadsWithExec(statementText);
+ var assignsThroughEquals = StatementLeadsWithExec(text);
- var sb = new StringBuilder(statementText.Length);
+ var sb = new StringBuilder(text.Length);
var substitutions = 0;
var i = 0;
- while (i < statementText.Length)
+ while (i < text.Length)
{
- var c = statementText[i];
+ var c = text[i];
/* Regions where an @name is text, not a parameter. A string literal is the case that
matters in practice — LIKE 'kexin%' sits right next to the parameters in the #466
repro — but a delimited identifier can hold anything, and a comment is not code. */
if (c == '\'' || c == '"')
{
- i = CopyDelimited(statementText, i, c, c, sb);
+ i = CopyDelimited(text, i, c, c, sb);
continue;
}
if (c == '[')
{
- i = CopyDelimited(statementText, i, '[', ']', sb);
+ i = CopyDelimited(text, i, '[', ']', sb);
continue;
}
- if (c == '-' && i + 1 < statementText.Length && statementText[i + 1] == '-')
+ if (c == '-' && i + 1 < text.Length && text[i + 1] == '-')
{
- i = CopyLineComment(statementText, i, sb);
+ i = CopyLineComment(text, i, sb);
continue;
}
- if (c == '/' && i + 1 < statementText.Length && statementText[i + 1] == '*')
+ if (c == '/' && i + 1 < text.Length && text[i + 1] == '*')
{
- i = CopyBlockComment(statementText, i, sb);
+ i = CopyBlockComment(text, i, sb);
continue;
}
@@ -94,15 +108,15 @@ matters in practice — LIKE 'kexin%' sits right next to the parameters in the #
tail of an identifier such as "t@0" is part of that identifier. The scan below claims
the longest run of identifier characters, which handles the first; the preceding
character is checked here, which handles the second. */
- if (c == '@' && !IsIdentifierPart(i > 0 ? statementText[i - 1] : '\0'))
+ if (c == '@' && !IsIdentifierPart(i > 0 ? text[i - 1] : '\0'))
{
var end = i + 1;
- while (end < statementText.Length && IsIdentifierPart(statementText[end]))
+ while (end < text.Length && IsIdentifierPart(text[end]))
end++;
- var token = statementText[i..end];
+ var token = text[i..end];
if (values.TryGetValue(token, out var value)
- && !IsAssignmentTarget(statementText, i, end, assignsThroughEquals))
+ && !IsAssignmentTarget(text, i, end, assignsThroughEquals))
{
sb.Append(value);
substitutions++;
@@ -125,6 +139,37 @@ tail of an identifier such as "t@0" is part of that identifier. The scan below c
: new ParameterSubstitutionResult(sb.ToString(), substitutions);
}
+ ///
+ /// Where the statement starts in text that opens with an sp_executesql declaration list,
+ /// such as (@p1 int, @p2 decimal(18,2))SELECT …. Returns 0 when the text has no list,
+ /// and -1 when the list never closes: a plan cuts statement text off at 4,000 characters, and
+ /// the declarations for a long IN list can fill all of them. The list ends at the parenthesis
+ /// that closes its first one, so the parentheses of a type are counted, not taken for the end.
+ /// A statement cannot begin with (@, so that opening always means a list.
+ /// ReproScriptBuilder strips the list with this too. (The web project compiles this
+ /// file without it, so the name is not a cref.)
+ ///
+ internal static int DeclarationListEnd(string text)
+ {
+ var i = 0;
+ while (i < text.Length && char.IsWhiteSpace(text[i]))
+ i++;
+
+ if (i + 1 >= text.Length || text[i] != '(' || text[i + 1] != '@')
+ return 0;
+
+ var depth = 0;
+ for (; i < text.Length; i++)
+ {
+ if (text[i] == '(')
+ depth++;
+ else if (text[i] == ')' && --depth == 0)
+ return i + 1;
+ }
+
+ return -1; // the list never closes: the text was cut off inside it
+ }
+
///
/// True when the parameter token spanning to is
/// being assigned TO rather than read from, in which case its value must not be written over it.
diff --git a/src/PlanViewer.Core/Services/ReproScriptBuilder.cs b/src/PlanViewer.Core/Services/ReproScriptBuilder.cs
index c19569c..a7f8a59 100644
--- a/src/PlanViewer.Core/Services/ReproScriptBuilder.cs
+++ b/src/PlanViewer.Core/Services/ReproScriptBuilder.cs
@@ -365,39 +365,13 @@ private static List ExtractSetOptionsFromPlan(string planXml)
///
/// Strips the parameter declaration prefix from query text captured via sp_executesql.
/// Query text like "(@p1 int, @p2 nvarchar(50))SELECT ..." becomes "SELECT ...".
- /// Uses same approach as sp_QueryReproBuilder: find the closing ) followed by non-comma.
+ /// The list is found by the same parser that keeps parameter substitution out of it.
///
private static string StripParameterPrefix(string queryText)
{
- if (!queryText.StartsWith("(@", StringComparison.Ordinal))
- {
- return queryText;
- }
-
- /* Find the closing parenthesis that ends the parameter list.
- Look for ) followed by a character that's not a comma (which would indicate
- we're still inside nested parentheses in a type like decimal(18,2)). */
- int depth = 0;
- for (int i = 0; i < queryText.Length; i++)
- {
- char c = queryText[i];
- if (c == '(')
- {
- depth++;
- }
- else if (c == ')')
- {
- depth--;
- if (depth == 0)
- {
- /* Found the closing paren — return everything after it, trimmed */
- return queryText[(i + 1)..].TrimStart();
- }
- }
- }
-
- /* Couldn't find balanced parens — return original */
- return queryText;
+ /* No list (0), or a list that never closes (-1): the text stays as it is, as before. */
+ var bodyStart = ParameterSubstitution.DeclarationListEnd(queryText);
+ return bodyStart <= 0 ? queryText : queryText[bodyStart..].TrimStart();
}
///
diff --git a/tests/PlanViewer.Core.Tests/ComparisonBaseline.txt b/tests/PlanViewer.Core.Tests/ComparisonBaseline.txt
index 96ce828..e95b56e 100644
--- a/tests/PlanViewer.Core.Tests/ComparisonBaseline.txt
+++ b/tests/PlanViewer.Core.Tests/ComparisonBaseline.txt
@@ -312,7 +312,7 @@ Plan A: in_list_dynamic_seek_plan.sqlplan
Plan B: in_list_dynamic_seek_plan.sqlplan
--- Statement 1 ---
-(10 int, 20 int)SELECT t.Id, t.A FROM dbo.T AS t WHERE t.A IN (10, 20)
+SELECT t.Id, t.A FROM dbo.T AS t WHERE t.A IN (10, 20)
Estimated cost: 0.0033 -> 0.0033 (0.0% costlier)
Estimated rows: 2 -> 2 (0.0% more)
@@ -697,7 +697,7 @@ Plan A: non_sargable_compound_predicate_plan.sqlplan
Plan B: non_sargable_compound_predicate_plan.sqlplan
--- Statement 1 ---
-(52 tinyint,2 int)SELECT COUNT(*) FROM [dbo].[T] [t] WHERE [t].[A]=52 AND [t].[B]=CONVERT([tinyint],2)
+SELECT COUNT(*) FROM [dbo].[T] [t] WHERE [t].[A]=52 AND [t].[B]=CONVERT([tinyint],2)
Estimated cost: 0.0033 -> 0.0033 (0.0% costlier)
Estimated rows: 1 -> 1 (0.0% more)
@@ -1500,7 +1500,7 @@ Plan A: in_list_dynamic_seek_plan.sqlplan
Plan B: isnull_plan.sqlplan
--- Statement 1 ---
-(10 int, 20 int)SELECT t.Id, t.A FROM dbo.T AS t WHERE t.A IN (10, 20)
+SELECT t.Id, t.A FROM dbo.T AS t WHERE t.A IN (10, 20)
Estimated cost: 0.0033 -> 3,119.42 (9,999% costlier)
Estimated rows: 2 -> 1 (50.0% fewer)
@@ -1904,7 +1904,7 @@ Plan A: non_sargable_compound_predicate_plan.sqlplan
Plan B: non_sargable_function_plan.sqlplan
--- Statement 1 ---
-(52 tinyint,2 int)SELECT COUNT(*) FROM [dbo].[T] [t] WHERE [t].[A]=52 AND [t].[B]=CONVERT([tinyint],2)
+SELECT COUNT(*) FROM [dbo].[T] [t] WHERE [t].[A]=52 AND [t].[B]=CONVERT([tinyint],2)
Estimated cost: 0.0033 -> 3,097.52 (9,999% costlier)
Estimated rows: 1 -> 1 (0.0% more)
diff --git a/tests/PlanViewer.Core.Tests/ParameterSubstitutionTests.cs b/tests/PlanViewer.Core.Tests/ParameterSubstitutionTests.cs
index d6440d6..7ab6ee8 100644
--- a/tests/PlanViewer.Core.Tests/ParameterSubstitutionTests.cs
+++ b/tests/PlanViewer.Core.Tests/ParameterSubstitutionTests.cs
@@ -390,4 +390,70 @@ parameter name survives anywhere in the text. */
Assert.Contains("like 'kexin%'", result.Text);
Assert.DoesNotContain("@", result.Text);
}
+
+ [Fact]
+ public void DeclarationList_IsLeftOutAndNotSubstituted()
+ {
+ /* A plan from the plan cache or Query Store keeps an sp_executesql statement's declaration
+ list in front of it. Substituted like the rest, "(@p1 int, @p2 int)" became
+ "(10 int, 20 int)", which is neither the plan's text nor runnable. */
+ var result = ParameterSubstitution.Apply(
+ "(@p1 int, @p2 int)SELECT t.Id FROM dbo.T AS t WHERE t.A IN (@p1, @p2)",
+ new List { Param("@p1", "(10)"), Param("@p2", "(20)") });
+
+ Assert.Equal("SELECT t.Id FROM dbo.T AS t WHERE t.A IN (10, 20)", result.Text);
+ Assert.Equal(2, result.SubstitutionCount);
+ }
+
+ [Fact]
+ public void DeclarationListWithParenthesizedTypes_EndsAtItsOwnClosingParenthesis()
+ {
+ /* The comma inside decimal(18,2) and the parentheses of both types are part of the list.
+ Stopping at the first closing parenthesis would leave ",@b nvarchar(50))" in the text. */
+ var result = ParameterSubstitution.Apply(
+ "(@a decimal(18,2),@b nvarchar(50))SELECT * FROM t WHERE x = @a AND y = @b",
+ new List { Param("@a", "(1.50)"), Param("@b", "N'abc'") });
+
+ Assert.Equal("SELECT * FROM t WHERE x = 1.50 AND y = N'abc'", result.Text);
+ }
+
+ [Fact]
+ public void DeclarationList_StaysWhenNothingIsSubstituted()
+ {
+ /* With no value to put back, the text is shown as the plan recorded it, list included. */
+ const string text = "(@p1 int)SELECT 1";
+ var result = ParameterSubstitution.Apply(text, new List { Param("@p1", "(10)") });
+
+ Assert.Equal(text, result.Text);
+ Assert.Equal(0, result.SubstitutionCount);
+ }
+
+ [Fact]
+ public void DeclarationListCutOffByTruncation_IsLeftAsItIs()
+ {
+ /* A plan cuts statement text off at 4,000 characters, and the declarations for a long IN
+ list can fill all of them. Then the text is only declarations, with no statement after
+ them, and putting values into it would make "(1 int,2 int,…" again. */
+ const string text = "(@p0 int,@p1 int,@p2 in";
+ var result = ParameterSubstitution.Apply(
+ text, new List { Param("@p0", "(1)"), Param("@p1", "(2)") });
+
+ Assert.Equal(text, result.Text);
+ Assert.Equal(0, result.SubstitutionCount);
+ }
+
+ [Fact]
+ public void AutoParameterizedPlan_LosesItsDeclarationList()
+ {
+ /* The #556 reproduction, an auto-parameterized plan: its text starts with
+ "(@1 tinyint,@2 int)", and the comparison report printed "(52 tinyint,2 int)SELECT". */
+ var plan = PlanTestHelper.LoadAndAnalyze("non_sargable_compound_predicate_plan.sqlplan");
+ var statement = PlanTestHelper.FirstStatement(plan);
+ Assert.StartsWith("(@1 tinyint,@2 int)", statement.StatementText);
+
+ var result = ParameterSubstitution.Apply(statement.StatementText, statement.Parameters);
+
+ Assert.StartsWith("SELECT COUNT(*) FROM [dbo].[T] [t] WHERE [t].[A]=52", result.Text);
+ Assert.DoesNotContain("@", result.Text);
+ }
}
diff --git a/tests/PlanViewer.Core.Tests/ReproScriptBuilderTests.cs b/tests/PlanViewer.Core.Tests/ReproScriptBuilderTests.cs
new file mode 100644
index 0000000..9efbb2e
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/ReproScriptBuilderTests.cs
@@ -0,0 +1,46 @@
+using PlanViewer.Core.Services;
+
+namespace PlanViewer.Core.Tests;
+
+///
+/// The repro script declares the parameters itself, so the declaration list that a cached
+/// sp_executesql statement starts with must not reach the script's query text. The list is found
+/// by the parser that parameter substitution uses (ParameterSubstitution.DeclarationListEnd).
+///
+public class ReproScriptBuilderTests
+{
+ private const string Plan = """
+
+
+
+
+
+
+
+
+
+
+
+ """;
+
+ [Fact]
+ public void BuildReproScript_DeclarationList_IsLeftOutOfTheQueryText()
+ {
+ var sql = ReproScriptBuilder.BuildReproScript(
+ "(@id decimal(18,2))SELECT * FROM dbo.T WHERE Id = @id", "db", Plan, null);
+
+ Assert.Contains("SELECT * FROM dbo.T WHERE Id = @id", sql);
+ Assert.DoesNotContain("(@id decimal(18,2))SELECT", sql);
+ Assert.Contains("@id = 42.50", sql);
+ }
+
+ [Fact]
+ public void BuildReproScript_DeclarationListCutOffByTruncation_IsKeptAsItIs()
+ {
+ /* The parser reports a list that never closes as -1. The text stays as it was, as it did
+ before the parser was shared, and the -1 must never reach a slice. */
+ var sql = ReproScriptBuilder.BuildReproScript("(@id decimal(18,2),@x nvarch", "db", Plan, null);
+
+ Assert.Contains("(@id decimal(18,2),@x nvarch", sql);
+ }
+}