Descend into UDF and procedure bodies inside cursor plans

#456 taught the parser to read StoredProc/UDF sub-plan bodies, but that
descent lives in ParseStatement - and a StmtCursor's operation statements
never pass through ParseStatement. They are built in the cursor branch of
ParseStatementAndChildren straight from CursorPlan > Operation > QueryPlan,
so a function called by the cursor's query carried its whole body in the
XML (the Operation element holds the UDF sub-plan beside its QueryPlan)
and the parser dropped every statement of it: not enumerated, not
analyzed, not counted. Same failure mode as #455 - well-formed, plausible,
and silently incomplete output.

Fix (#491): extract ParseStatement's UDF/StoredProc reads into a shared
ParseSubPlans helper and call it from the cursor branch on each Operation
element, attaching the bodies to that operation's statement. That is the
whole integration: PlanStatements.EnumerateAll already walks
UdfPlans/StoredProcPlan on every statement it yields, and since #486 every
consumer (analyzer, scorer, result mapper, statements grid, web viewer)
reads that traversal, so the bodies flow through analysis and counts with
no consumer changes.

The caller's depth carries into the new descent unchanged, preserving
#484: a generated bomb alternating cursor and procedure shapes past
MaxParseDepth throws the catchable depth error on the big-stack thread,
and was verified to fail cleanly (parse completes, assert reports the
miss) against a simulated depth reset at the cursor boundary. Tests use
generated XML on the depth-limit tests' precedent - a cursor wrapping a
UDF sub-plan is three nested element shapes, stated more clearly by a
minimal document than a captured fixture.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PvAv72Pwb8czsjDWsCCk7n

Author: erikdarlingdata

src/PlanViewer.Core/Services/ShowPlanParser.cs

@@ -214,6 +214,20 @@ private static List<PlanStatement> ParseStatementAndChildren(
                         stmt.CursorRequestedType = cursorRequestedType;
                         stmt.CursorConcurrency = cursorConcurrency;
                         stmt.CursorForwardOnly = cursorForwardOnly;
+
+                        /* #491: the same StoredProc/UDF descent every other statement shape gets.
+                           #456 taught ParseStatement to read sub-plan bodies, but a cursor's
+                           operation statements are built HERE, through ParseQueryPlanAsStatement,
+                           and never pass through ParseStatement - so a function called by the
+                           cursor's query carried its whole body in the XML (the Operation element
+                           holds the UDF sub-plan right next to this QueryPlan) and the parser
+                           dropped every statement of it. Attaching the bodies to the operation's
+                           statement is all it takes: PlanStatements.EnumerateAll already walks
+                           UdfPlans/StoredProcPlan on every statement it yields, and since #486
+                           every consumer reads that traversal. Depth passes through unchanged
+                           (#484) - resetting it at this boundary would reopen the MaxParseDepth
+                           bypass across cursor/procedure nesting. */
+                        ParseSubPlans(stmt, opEl, depth, cancellationToken);
                         results.Add(stmt);
                     }
                 }
@@ -300,46 +314,7 @@ prevent. Carrying the caller's depth through this method closes that reset. */
            so it took that early return and never reached this code, seventy lines further down. The
            parser looked like it descended into procedures and in the one case that matters never
            did. The same was true of a UDF call whose statement carries no plan of its own. */
-        // XSD gap: UDF sub-plans
-        foreach (var udfEl in stmtEl.Elements(Ns + "UDF"))
-        {
-            var udfInfo = new FunctionPlanInfo
-            {
-                ProcName = udfEl.Attribute("ProcName")?.Value ?? "",
-                IsNativelyCompiled = udfEl.Attribute("IsNativelyCompiled")?.Value is "true" or "1"
-            };
-            var udfStmts = udfEl.Element(Ns + "Statements");
-            if (udfStmts != null)
-            {
-                foreach (var childStmt in udfStmts.Elements())
-                {
-                    var parsed = ParseStatementAndChildren(childStmt, depth + 1, cancellationToken);
-                    udfInfo.Statements.AddRange(parsed);
-                }
-            }
-            stmt.UdfPlans.Add(udfInfo);
-        }
-
-        // XSD gap: StoredProc sub-plan
-        var storedProcEl = stmtEl.Element(Ns + "StoredProc");
-        if (storedProcEl != null)
-        {
-            var spInfo = new FunctionPlanInfo
-            {
-                ProcName = storedProcEl.Attribute("ProcName")?.Value ?? "",
-                IsNativelyCompiled = storedProcEl.Attribute("IsNativelyCompiled")?.Value is "true" or "1"
-            };
-            var spStmts = storedProcEl.Element(Ns + "Statements");
-            if (spStmts != null)
-            {
-                foreach (var childStmt in spStmts.Elements())
-                {
-                    var parsed = ParseStatementAndChildren(childStmt, depth + 1, cancellationToken);
-                    spInfo.Statements.AddRange(parsed);
-                }
-            }
-            stmt.StoredProcPlan = spInfo;
-        }
+        ParseSubPlans(stmt, stmtEl, depth, cancellationToken);
 
         if (queryPlanEl == null)
         {
@@ -400,6 +375,64 @@ did. The same was true of a UDF call whose statement carries no plan of its own.
         return stmt;
     }
 
+    /// <summary>
+    /// Reads the StoredProc/UDF sub-plan bodies hanging off <paramref name="containerEl"/> onto
+    /// <paramref name="stmt"/>. One reader shared by ParseStatement — where StmtSimple carries the
+    /// UDF/StoredProc elements directly — and the StmtCursor branch (#491), where the same UDF
+    /// element sits beside the QueryPlan under CursorPlan &gt; Operation instead, so the two shapes
+    /// cannot drift apart the way the analyzer and the mapper once did (#455).
+    /// The caller's depth carries into the body statements (#484): this descent is what makes
+    /// module nesting recursive, and resetting depth at a sub-plan boundary is exactly the
+    /// MaxParseDepth bypass #484 closed.
+    /// </summary>
+    private static void ParseSubPlans(
+        PlanStatement stmt,
+        XElement containerEl,
+        int depth,
+        CancellationToken cancellationToken)
+    {
+        // XSD gap: UDF sub-plans
+        foreach (var udfEl in containerEl.Elements(Ns + "UDF"))
+        {
+            var udfInfo = new FunctionPlanInfo
+            {
+                ProcName = udfEl.Attribute("ProcName")?.Value ?? "",
+                IsNativelyCompiled = udfEl.Attribute("IsNativelyCompiled")?.Value is "true" or "1"
+            };
+            var udfStmts = udfEl.Element(Ns + "Statements");
+            if (udfStmts != null)
+            {
+                foreach (var childStmt in udfStmts.Elements())
+                {
+                    var parsed = ParseStatementAndChildren(childStmt, depth + 1, cancellationToken);
+                    udfInfo.Statements.AddRange(parsed);
+                }
+            }
+            stmt.UdfPlans.Add(udfInfo);
+        }
+
+        // XSD gap: StoredProc sub-plan
+        var storedProcEl = containerEl.Element(Ns + "StoredProc");
+        if (storedProcEl != null)
+        {
+            var spInfo = new FunctionPlanInfo
+            {
+                ProcName = storedProcEl.Attribute("ProcName")?.Value ?? "",
+                IsNativelyCompiled = storedProcEl.Attribute("IsNativelyCompiled")?.Value is "true" or "1"
+            };
+            var spStmts = storedProcEl.Element(Ns + "Statements");
+            if (spStmts != null)
+            {
+                foreach (var childStmt in spStmts.Elements())
+                {
+                    var parsed = ParseStatementAndChildren(childStmt, depth + 1, cancellationToken);
+                    spInfo.Statements.AddRange(parsed);
+                }
+            }
+            stmt.StoredProcPlan = spInfo;
+        }
+    }
+
     /// <summary>
     /// Parse a QueryPlan element that comes from a cursor Operation (no parent StmtSimple attributes).
     /// </summary>

tests/PlanViewer.Core.Tests/CursorSubPlanTests.cs

@@ -0,0 +1,151 @@
+using System.Linq;
+using PlanViewer.Core.Output;
+using PlanViewer.Core.Services;
+
+namespace PlanViewer.Core.Tests;
+
+/// <summary>
+/// #491: #456 taught the parser to descend into StoredProc/UDF sub-plans, but that descent lives
+/// in ParseStatement — and a StmtCursor's operation statements never go through ParseStatement.
+/// They are built in ParseStatementAndChildren's cursor branch, straight from
+/// CursorPlan &gt; Operation &gt; QueryPlan, so a function called by the cursor's query carried
+/// its whole body in the XML (the Operation element holds the UDF sub-plan beside its QueryPlan)
+/// and the parser dropped every statement of it. Same failure mode as #455: the output stayed
+/// well-formed and plausible — the cursor's own query analyzed, the function body silently
+/// contributed nothing.
+///
+/// <para>The plan XML here is generated rather than captured, on the depth-limit tests'
+/// precedent: a cursor wrapping a UDF sub-plan is three nested element shapes, and a minimal
+/// document states that more clearly than a 40KB fixture could. The element shapes mirror the
+/// real showplan schema — StmtCursor &gt; CursorPlan &gt; Operation &gt; QueryPlan, with the UDF
+/// element and its Statements sitting beside the QueryPlan under Operation — matching the
+/// namespace and attributes the parser reads from committed fixtures.</para>
+/// </summary>
+public sealed class CursorSubPlanTests
+{
+    /* One cursor operation whose query calls dbo.CursorFn; the function body carries two
+       statements — one bare (RETURN, no QueryPlan of its own, like the EXEC in #455) and one
+       with a plan — so the descent is proven for both statement shapes. */
+    private const string CursorOverUdfPlan =
+        "<ShowPlanXML xmlns=\"http://schemas.microsoft.com/sqlserver/2004/07/showplan\" Version=\"1.564\" Build=\"16.0.4135.4\">" +
+        "<BatchSequence><Batch><Statements>" +
+        "<StmtCursor StatementText=\"DECLARE cur CURSOR FAST_FORWARD FOR SELECT dbo.CursorFn(o.Id) FROM dbo.Orders AS o\" StatementId=\"1\" StatementCompId=\"1\">" +
+        "<CursorPlan CursorName=\"cur\" CursorActualType=\"FastForward\" CursorRequestedType=\"FastForward\" CursorConcurrency=\"Read Only\" ForwardOnly=\"true\">" +
+        "<Operation OperationType=\"FetchQuery\">" +
+        "<QueryPlan>" +
+        "<RelOp NodeId=\"0\" PhysicalOp=\"Clustered Index Scan\" LogicalOp=\"Clustered Index Scan\" EstimateRows=\"10\" EstimatedTotalSubtreeCost=\"0.005\" />" +
+        "</QueryPlan>" +
+        "<UDF ProcName=\"dbo.CursorFn\" IsNativelyCompiled=\"false\">" +
+        "<Statements>" +
+        "<StmtSimple StatementText=\"RETURN @Id * 2\" StatementId=\"2\" StatementCompId=\"2\" />" +
+        "<StmtSimple StatementText=\"SELECT @Total = COUNT(*) FROM dbo.Numbers AS n\" StatementId=\"3\" StatementCompId=\"3\">" +
+        "<QueryPlan>" +
+        "<RelOp NodeId=\"0\" PhysicalOp=\"Clustered Index Scan\" LogicalOp=\"Clustered Index Scan\" EstimateRows=\"100\" EstimatedTotalSubtreeCost=\"0.02\" />" +
+        "</QueryPlan>" +
+        "</StmtSimple>" +
+        "</Statements>" +
+        "</UDF>" +
+        "</Operation>" +
+        "</CursorPlan>" +
+        "</StmtCursor>" +
+        "</Statements></Batch></BatchSequence></ShowPlanXML>";
+
+    /// <summary>
+    /// The attach point, pinned the way #455 pinned the EXEC's: the cursor operation's statement
+    /// is the one that must carry the function body, because it is the statement the operation's
+    /// query belongs to — and it is built by a code path ParseStatement's descent never touches.
+    /// </summary>
+    [Fact]
+    public void ACursorOperationStatementCarriesItsFunctionBody()
+    {
+        var plan = ShowPlanParser.Parse(CursorOverUdfPlan);
+
+        Assert.Null(plan.ParseError);
+        var operation = Assert.Single(Assert.Single(plan.Batches).Statements);
+
+        // Proves this went down the cursor branch, not some fallback path.
+        Assert.Equal("cur", operation.CursorName);
+
+        var udf = Assert.Single(operation.UdfPlans);
+        Assert.Equal("dbo.CursorFn", udf.ProcName);
+        Assert.Equal(2, udf.Statements.Count);
+        Assert.Equal("RETURN @Id * 2", udf.Statements[0].StatementText);
+        Assert.StartsWith("SELECT @Total", udf.Statements[1].StatementText);
+    }
+
+    /// <summary>
+    /// The shared traversal surfaces the body — which is the whole fix. Every consumer (#486)
+    /// reads PlanStatements.EnumerateAll rather than walking batch.Statements itself, so once the
+    /// bodies are here, the analyzer, the scorer, the mapper, and the statements grid all see
+    /// them without any change of their own. Order matters like it did for #455: the operation
+    /// comes first, its body follows in source order.
+    /// </summary>
+    [Fact]
+    public void EnumerateAllSurfacesCursorFunctionBodyStatements()
+    {
+        var plan = ShowPlanParser.Parse(CursorOverUdfPlan);
+
+        var all = PlanStatements.EnumerateAll(plan).ToList();
+        var operation = Assert.Single(plan.Batches.SelectMany(b => b.Statements));
+
+        Assert.Equal(3, all.Count);
+        Assert.Same(operation, all[0]);
+        Assert.Equal("RETURN @Id * 2", all[1].StatementText);
+        Assert.StartsWith("SELECT @Total", all[2].StatementText);
+
+        /* The context-carrying walk names the module, so a grid row for a body statement says
+           where it came from instead of showing a bare SELECT beside the cursor's. */
+        var entries = PlanStatements.EnumerateAllWithContainer(plan).ToList();
+        Assert.Null(entries[0].ContainerPath);
+        Assert.All(entries.Skip(1), e => Assert.Equal("dbo.CursorFn", e.ContainerPath));
+    }
+
+    /// <summary>
+    /// Downstream of the traversal: the same document through analyze + score + map counts all
+    /// three statements. TotalStatements is the number a report reader trusts first, and before
+    /// this fix a cursor-over-UDF plan reported 1 — the #455 signature all over again, one
+    /// container type later.
+    /// </summary>
+    [Fact]
+    public void CursorFunctionBodyStatementsAreAnalyzedAndCounted()
+    {
+        var plan = ShowPlanParser.Parse(CursorOverUdfPlan);
+        PlanAnalyzer.Analyze(plan);
+        BenefitScorer.Score(plan);
+
+        var result = ResultMapper.Map(plan, "cursor_over_udf");
+
+        Assert.Equal(3, result.Summary.TotalStatements);
+    }
+
+    /// <summary>
+    /// The StoredProc half of the same read. The published XSD only puts UDF under a cursor
+    /// Operation, but the parser reads UDF and StoredProc as a pair everywhere else it descends
+    /// ("XSD gap" reads — plans have carried elements the schema omits before), so the cursor
+    /// branch reads both on purpose. Pinned so the symmetric half cannot rot into untested code.
+    /// </summary>
+    [Fact]
+    public void ACursorOperationStoredProcSubPlanIsReadToo()
+    {
+        const string xml =
+            "<ShowPlanXML xmlns=\"http://schemas.microsoft.com/sqlserver/2004/07/showplan\">" +
+            "<BatchSequence><Batch><Statements>" +
+            "<StmtCursor StatementText=\"DECLARE cur CURSOR FOR SELECT 1\">" +
+            "<CursorPlan CursorName=\"cur\"><Operation OperationType=\"FetchQuery\">" +
+            "<QueryPlan><RelOp NodeId=\"0\" /></QueryPlan>" +
+            "<StoredProc ProcName=\"dbo.CursorProc\"><Statements>" +
+            "<StmtSimple StatementText=\"SELECT 2\" />" +
+            "</Statements></StoredProc>" +
+            "</Operation></CursorPlan></StmtCursor>" +
+            "</Statements></Batch></BatchSequence></ShowPlanXML>";
+
+        var plan = ShowPlanParser.Parse(xml);
+
+        Assert.Null(plan.ParseError);
+        var operation = Assert.Single(Assert.Single(plan.Batches).Statements);
+        Assert.NotNull(operation.StoredProcPlan);
+        Assert.Equal("dbo.CursorProc", operation.StoredProcPlan!.ProcName);
+        Assert.Equal("SELECT 2", Assert.Single(operation.StoredProcPlan.Statements).StatementText);
+        Assert.Equal(2, PlanStatements.EnumerateAll(plan).Count());
+    }
+}

tests/PlanViewer.Core.Tests/ShowPlanParserLimitsTests.cs

@@ -74,6 +74,68 @@ public void ProcedureNestingBelowTheDepthLimitStillParsesEveryLevel()
         Assert.Equal(depth, levels);
     }
 
+    /// <summary>
+    /// #491 gave the cursor branch the same StoredProc/UDF descent as every other statement
+    /// shape, which is a NEW descent path — and #484 is the proof that a new descent path is
+    /// where the depth reset comes back. A plan alternating cursor and procedure shapes
+    /// (StmtCursor &gt; CursorPlan &gt; Operation &gt; UDF wrapping StmtSimple &gt; StoredProc,
+    /// repeated) must hit the same catchable depth error as pure procedure nesting; if the
+    /// cursor-side descent ever forgets to pass depth, the guard can never fire across a cursor
+    /// boundary and this parse runs to completion instead — failing on the assert, safely, per
+    /// the big-stack reasoning on <see cref="BombDepth"/>.
+    /// </summary>
+    [Fact]
+    public void CursorProcedureNestingPastTheDepthLimitFailsWithACatchableError()
+    {
+        var xml = NestedCursorProcedurePlan(BombDepth);
+
+        ParsedPlan? plan = null;
+        var thread = new Thread(() => plan = ShowPlanParser.Parse(xml), 8 * 1024 * 1024);
+        thread.Start();
+        thread.Join();
+
+        Assert.NotNull(plan);
+        Assert.NotNull(plan!.ParseError);
+        Assert.Contains("depth limit", plan.ParseError);
+    }
+
+    /// <summary>
+    /// And the same in reverse for the cursor shape: carrying depth through cursor operation
+    /// bodies must not overcount and reject legitimate nesting. Every alternating level below
+    /// the limit still parses, attached to the right container, all the way down.
+    /// </summary>
+    [Fact]
+    public void CursorProcedureNestingBelowTheDepthLimitStillParsesEveryLevel()
+    {
+        const int depth = 50;
+
+        var plan = ShowPlanParser.Parse(NestedCursorProcedurePlan(depth));
+
+        Assert.Null(plan.ParseError);
+        var stmt = Assert.Single(Assert.Single(plan.Batches).Statements);
+        var cursorLevels = 0;
+        var procLevels = 0;
+        while (true)
+        {
+            if (stmt.UdfPlans.Count == 1)
+            {
+                cursorLevels++;
+                stmt = Assert.Single(stmt.UdfPlans[0].Statements);
+            }
+            else if (stmt.StoredProcPlan is not null)
+            {
+                procLevels++;
+                stmt = Assert.Single(stmt.StoredProcPlan.Statements);
+            }
+            else
+            {
+                break;
+            }
+        }
+        Assert.Equal(depth / 2, cursorLevels);
+        Assert.Equal(depth / 2, procLevels);
+    }
+
     [Fact]
     public void SynchronousParseRejectsOversizedInput()
     {
@@ -108,4 +170,35 @@ private static string NestedProcedurePlan(int levels)
         xml.Append("</Statements></Batch></BatchSequence></ShowPlanXML>");
         return xml.ToString();
     }
+
+    /// <summary>
+    /// Alternating levels of StmtCursor &gt; CursorPlan &gt; Operation &gt; UDF &gt; Statements
+    /// and StmtSimple &gt; StoredProc &gt; Statements around one innermost bare statement, so the
+    /// depth counter has to survive a hand-off between the cursor branch's descent (#491) and
+    /// ParseStatement's (#456/#484) at every second boundary. Each cursor Operation carries the
+    /// QueryPlan the schema requires — the branch only builds a statement (the sub-plan's attach
+    /// point) for an operation that has one.
+    /// </summary>
+    private static string NestedCursorProcedurePlan(int levels)
+    {
+        var xml = new StringBuilder(
+            "<ShowPlanXML xmlns=\"http://schemas.microsoft.com/sqlserver/2004/07/showplan\"><BatchSequence><Batch><Statements>");
+        for (var level = 0; level < levels; level++)
+        {
+            if (level % 2 == 0)
+                xml.Append("<StmtCursor StatementText=\"DECLARE c CURSOR\"><CursorPlan CursorName=\"c\"><Operation OperationType=\"FetchQuery\"><QueryPlan><RelOp NodeId=\"0\" /></QueryPlan><UDF ProcName=\"f\"><Statements>");
+            else
+                xml.Append("<StmtSimple StatementText=\"EXEC p\"><StoredProc ProcName=\"p\"><Statements>");
+        }
+        xml.Append("<StmtSimple StatementText=\"SELECT 1\" />");
+        for (var level = levels - 1; level >= 0; level--)
+        {
+            if (level % 2 == 0)
+                xml.Append("</Statements></UDF></Operation></CursorPlan></StmtCursor>");
+            else
+                xml.Append("</Statements></StoredProc></StmtSimple>");
+        }
+        xml.Append("</Statements></Batch></BatchSequence></ShowPlanXML>");
+        return xml.ToString();
+    }
 }