Skip to content

Commit c235a67

Browse files
committed
Deep expression nesting now fails gracefully instead of overflowing the stack, added longer-term fix to the backlog.
1 parent c0505ee commit c235a67

7 files changed

Lines changed: 157 additions & 15 deletions

File tree

SqlServerSimulator.Tests.SqlClient/RpcParameterTypeTests.cs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,19 +204,26 @@ public async Task LargeStatement_SentAsNtextRpcParam_Executes()
204204
}
205205

206206
// The @statement itself carrying > 4000 chars of real SQL (not padding) —
207-
// ntext value must decode to the exact query the server then runs.
207+
// ntext value must decode to the exact query the server then runs. The
208+
// arithmetic chain stays shallow (200 terms) because expression parsing
209+
// recurses per operator and default 1 MB thread stacks overflow near 700
210+
// levels; a positionally-distinctive 3000-char literal carries the bulk
211+
// of the statement past the nvarchar(4000) ntext threshold instead, and
212+
// its char-exact round-trip is the decode-exactness evidence.
208213
[TestMethod]
209214
public async Task LargeStatement_NtextValue_DecodesExactly()
210215
{
211216
var simulation = new Simulation();
212217
await using var listener = await simulation.ListenAsync(0, TestContext.CancellationToken);
213218
await using var connection = await Wire.OpenAsync(listener, TestContext.CancellationToken);
214-
var terms = string.Join(" + ", Enumerable.Repeat("1", 3000));
215-
await using var command = new SqlCommand($"SELECT ({terms}) AS total, @p AS p", connection);
219+
var marker = string.Concat(Enumerable.Range(0, 500).Select(i => $"m{i:D4}"));
220+
var terms = string.Join(" + ", Enumerable.Repeat("1", 200));
221+
await using var command = new SqlCommand($"SELECT ({terms}) AS total, N'{marker}' AS marker, @p AS p", connection);
216222
_ = command.Parameters.Add(new SqlParameter("@p", SqlDbType.NVarChar, 20) { Value = "ok" });
217223
await using var reader = await command.ExecuteReaderAsync(TestContext.CancellationToken);
218224
IsTrue(await reader.ReadAsync(TestContext.CancellationToken));
219-
AreEqual(3000, reader.GetInt32(0));
220-
AreEqual("ok", reader.GetString(1));
225+
AreEqual(200, reader.GetInt32(0));
226+
AreEqual(marker, reader.GetString(1));
227+
AreEqual("ok", reader.GetString(2));
221228
}
222229
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
using static Microsoft.VisualStudio.TestTools.UnitTesting.Assert;
2+
3+
namespace SqlServerSimulator;
4+
5+
/// <summary>
6+
/// Expression-nesting limits. Expression parsing recurses per operator and
7+
/// grouping level, and a .NET stack overflow is uncatchable and
8+
/// process-fatal — so pathological depth must surface as SQL Server's own
9+
/// graceful errors instead. Probed against SQL Server 2025 (2026-07-15):
10+
/// a 6000-term <c>1 + 1 + …</c> chain raises Msg 8631 Class 17 ("Server
11+
/// stack limit"), a stack-dependent threshold the simulator mirrors via the
12+
/// runtime's remaining-stack probe (so its threshold likewise scales with
13+
/// the calling thread's stack size, firing near 750 levels on a default
14+
/// 1 MB thread); 2000 nested parens raise Msg 191 Class 15, a structural
15+
/// limit the simulator enforces at 512 so the paren shape reports Msg 191
16+
/// before the stack probe would claim it.
17+
/// </summary>
18+
[TestClass]
19+
public sealed class ExpressionNestingLimitTests
20+
{
21+
private static string AdditionChain(int terms) =>
22+
$"select ({string.Join(" + ", Enumerable.Repeat("1", terms))})";
23+
24+
[TestMethod]
25+
public void AdditionChain_ModerateDepth_Evaluates()
26+
{
27+
AreEqual(200, new Simulation().ExecuteScalar(AdditionChain(200)));
28+
}
29+
30+
[TestMethod]
31+
[Timeout(60000)]
32+
public void AdditionChain_ExtremeDepth_RaisesMsg8631NotStackOverflow()
33+
{
34+
var ex = new Simulation().AssertSqlError(AdditionChain(50000), 8631);
35+
AreEqual(17, ex.Class);
36+
AreEqual("Internal error: Server stack limit has been reached. Please look for potentially deep nesting in your query, and try to simplify it.", ex.Message);
37+
}
38+
39+
[TestMethod]
40+
public void NestedParens_WithinLimit_Evaluates()
41+
{
42+
AreEqual(1, new Simulation().ExecuteScalar($"select {new string('(', 200)}1{new string(')', 200)}"));
43+
}
44+
45+
[TestMethod]
46+
[Timeout(60000)]
47+
public void NestedParens_OverLimit_RaisesMsg191()
48+
{
49+
var ex = new Simulation().AssertSqlError($"select {new string('(', 513)}1{new string(')', 513)}", 191);
50+
AreEqual(15, ex.Class);
51+
AreEqual("Some part of your SQL statement is nested too deeply. Rewrite the query or break it up into smaller queries.", ex.Message);
52+
}
53+
}

SqlServerSimulator/Errors/SimulatedSqlException.SyntaxErrors.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,30 @@ partial class SimulatedSqlException
1616
internal static SimulatedSqlException UnclosedStringLiteral(string body) =>
1717
new($"Unclosed quotation mark after the character string '{body}'.", 105, 15, 1);
1818

19+
/// <summary>
20+
/// Mimics SQL Server error 8631: expression parsing consumed the thread's
21+
/// stack down to the runtime's safety threshold. Real SQL Server raises
22+
/// this from an actual stack probe (probe-confirmed 2026-07-15: a
23+
/// 6000-term <c>1 + 1 + …</c> chain fails, 3000 succeeds — the threshold
24+
/// is stack-dependent, not a fixed count); the simulator mirrors that via
25+
/// <see cref="System.Runtime.CompilerServices.RuntimeHelpers.EnsureSufficientExecutionStack"/>,
26+
/// so the depth it tolerates scales with the calling thread's stack size.
27+
/// Class 17 — batch-aborting, matching real.
28+
/// </summary>
29+
internal static SimulatedSqlException ServerStackLimitReached() =>
30+
new("Internal error: Server stack limit has been reached. Please look for potentially deep nesting in your query, and try to simplify it.", 8631, 17, 1);
31+
32+
/// <summary>
33+
/// Mimics SQL Server error 191: parenthesized-expression nesting exceeded
34+
/// the structural limit. Probe-confirmed 2026-07-15: 1000 nested parens
35+
/// succeed on the reference, 2000 raise this — the simulator's limit is
36+
/// deliberately lower (512) so the structural Msg 191 fires before the
37+
/// stack probe converts the same shape into Msg 8631 on default-size
38+
/// (1 MB) threads.
39+
/// </summary>
40+
internal static SimulatedSqlException StatementNestedTooDeeply() =>
41+
new("Some part of your SQL statement is nested too deeply. Rewrite the query or break it up into smaller queries.", 191, 15, 1);
42+
1943
internal static SimulatedSqlException SyntaxErrorNearKeyword(ReservedKeyword token) => new($"Incorrect syntax near the keyword '{token}'.", 156, 15, 1);
2044

2145
/// <summary>

SqlServerSimulator/Parser/Expression.cs

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,25 @@ private protected Expression()
4141
/// <exception cref="NotSupportedException">A condition was encountered that may be valid but can't currently be parsed.</exception>
4242
public static Expression Parse(ParserContext context)
4343
{
44+
// Stack-probe guard: expression parsing recurses per operator /
45+
// nesting level (TwoSidedExpression's parsing ctor re-enters Parse
46+
// for its right side), and a .NET stack overflow is uncatchable and
47+
// process-fatal — unacceptable for an in-process library fed a
48+
// pathological query. Real SQL Server's Msg 8631 is likewise a
49+
// genuine stack probe (threshold varies with its thread stack), so
50+
// deferring to the runtime's own remaining-stack check is the
51+
// faithful shape. Every recursive parse path (boolean chains, CASE,
52+
// function arguments, subquery projections) passes through here at
53+
// least once per nesting level, so this single site bounds them all.
54+
try
55+
{
56+
System.Runtime.CompilerServices.RuntimeHelpers.EnsureSufficientExecutionStack();
57+
}
58+
catch (InsufficientExecutionStackException)
59+
{
60+
throw SimulatedSqlException.ServerStackLimitReached();
61+
}
62+
4463
Expression expression;
4564
switch (context.Token)
4665
{
@@ -547,20 +566,39 @@ internal virtual void VisitColumnReferences(Action<MultiPartName> visit) { }
547566
/// matching the lookahead contract <see cref="Parse"/>'s
548567
/// binary loop expects.
549568
/// </summary>
569+
/// <summary>
570+
/// Maximum <c>(</c>-nesting depth for grouped expressions before Msg 191.
571+
/// Real SQL Server's structural limit is higher (1000 nested parens
572+
/// succeed, 2000 fail — probe-confirmed 2026-07-15) but stack-dependent;
573+
/// 512 keeps the structural error firing before <see cref="Parse"/>'s
574+
/// stack probe would convert the same shape into Msg 8631 on a
575+
/// default-size (1 MB) thread.
576+
/// </summary>
577+
private const int MaxGroupingDepth = 512;
578+
550579
private static Expression ParseGroupedExpression(ParserContext context)
551580
{
552-
context.MoveNextRequired();
553-
if (context.Token is ReservedKeyword { Keyword: Keyword.Select })
581+
if (++context.GroupingDepth > MaxGroupingDepth)
582+
throw SimulatedSqlException.StatementNestedTooDeeply();
583+
try
554584
{
555-
var inner = Selection.Parse(context, depth: 1, outerTypeResolver: context.OuterTypeResolver);
556-
return inner.Schema.Length != 1
557-
? throw SimulatedSqlException.SubqueryNotIntroducedWithExists()
558-
: context.Token is not Operator { Character: ')' }
559-
? throw SimulatedSqlException.SyntaxErrorNear(context)
560-
: (Expression)new ScalarSubqueryExpression(inner);
561-
}
585+
context.MoveNextRequired();
586+
if (context.Token is ReservedKeyword { Keyword: Keyword.Select })
587+
{
588+
var inner = Selection.Parse(context, depth: 1, outerTypeResolver: context.OuterTypeResolver);
589+
return inner.Schema.Length != 1
590+
? throw SimulatedSqlException.SubqueryNotIntroducedWithExists()
591+
: context.Token is not Operator { Character: ')' }
592+
? throw SimulatedSqlException.SyntaxErrorNear(context)
593+
: (Expression)new ScalarSubqueryExpression(inner);
594+
}
562595

563-
return new Parenthesized(Expression.Parse(context));
596+
return new Parenthesized(Expression.Parse(context));
597+
}
598+
finally
599+
{
600+
context.GroupingDepth--;
601+
}
564602
}
565603

566604
private static Expression ResolveBuiltIn(string name, ParserContext context)

SqlServerSimulator/Parser/ParserContext.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,16 @@ internal sealed class ParserContext(SimulatedDbCommand command, BatchContext bat
6464
/// </summary>
6565
public bool QuotedIdentifiers = command.Connection!.QuotedIdentifiers;
6666

67+
/// <summary>
68+
/// Live <c>(</c>-nesting depth inside grouped-expression parsing,
69+
/// maintained by <c>Expression.ParseGroupedExpression</c> (increment on
70+
/// entry, decrement in <c>finally</c>). Crossing the structural limit
71+
/// raises Msg 191; the companion stack-probe guard at
72+
/// <see cref="Expression.Parse"/> entry raises Msg 8631 when actual
73+
/// remaining stack runs low first.
74+
/// </summary>
75+
public int GroupingDepth;
76+
6777
/// <summary>
6878
/// The most recently identified token in the command string.
6979
/// </summary>

docs/claude/backlog.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ Real bugs / limitations against shipped behavior — fixes are concrete work, no
5757
- **`SimulatedSqlException` ERROR tokens carry `Server=''` / `Line 0`** — real SQL Server fills the server name and a 1-based line number on every error; the simulator's query errors surface blank/0 both in-process and over the wire (sqlcmd renders `Server , Line 0`). Inconsistently, `NotSupportedException`-derived wire errors *do* carry `Server=SIMULATED, Line 1`. Engine-level: the fix is stamping `SimulatedError.Server` (from `SERVERPROPERTY('ServerName')` = `SIMULATED`) at construction and threading real line numbers from the tokenizer — the latter is the large part. Harvested from the go-sqlcmd shakedown (2026-07-14).
5858
- **String literals typed at container width, not value width**`SELECT 'included'` advertises `varchar(8000)` (and `@@VERSION` `nvarchar(4000)`) in projection schema; real types literals at exact length (`varchar(8)`). Correct data, but clients that size display columns from COLMETADATA (sqlcmd) render absurdly wide output, and `GetSchemaTable`-driven consumers see wrong sizes. Fix lives in literal `SqlType` inference (`Literal` → length-parameterized type), with CONCAT/CASE/set-op width-combination ripple effects — probe real's width algebra before starting. Harvested from the go-sqlcmd shakedown (2026-07-14).
5959
- **Skip-mode deferred name resolution abandons the enclosing statement mid-parse** — a statement skipped because the batch is in error/skip mode (e.g. inside an un-taken `IF`) whose deferred table/column resolution misses raises Msg 207/208 and *throws out of the middle of the parse* (throw-and-recover) instead of parsing the whole statement and discarding it the way real SQL Server compiles-then-skips. So `IF EXISTS (SELECT … FROM <missing>) <then> ELSE <else>` inside a skipped region orphans the `<then>` / `ELSE` fragments, which the dispatch loop then re-enters as bare statements — surfacing spurious Msg 102/156 (a bare `ELSE` is a structural error). The dispatch-loop non-progress guard (`Simulation.DispatchStatementsUntil`) bounds the damage; pre-guard this infinite-looped the wire path (the SSMS Query Store probe crash of 2026-07-15, since the abandoned recovery scan advanced zero tokens with the cursor already on a statement-boundary token). The real fix is parse-continuation: skip-mode resolvers returning placeholder metadata instead of throwing, so a skipped statement parses to completion and is discarded whole. Open. See [`control-flow.md`](control-flow.md).
60+
- **Expression-chain depth ceiling far below real's** — expression parsing recurses per binary operator with fat `Expression.Parse` frames, so the Msg 8631 stack-probe guard (see [`grammar.md`](grammar.md) "Expression nesting limits") fires near 750 chained operators on a default 1 MB thread where the reference server handles 3000+ (its own Msg 8631 fires between 3000 and 6000). Graceful today (no process-fatal overflow — the guard shipped 2026-07-15 after `LargeStatement_NtextValue_DecodesExactly`'s 3000-term chain stack-overflowed Windows Release test runs), but lifting the ceiling toward real's requires restructuring the binary-operator parse from ctor-recursion to iterative precedence-climbing (which would also obsolete `AdjustForPrecedence`'s post-hoc rotation) and bounding `Run` / `GetSqlType` tree recursion. Sizeable parser-core change; low real-world demand (generated SQL rarely nests past tens).
6061
- **Trailing-space MIN/MAX representative** — for a group of values differing only in trailing spaces (sort-equal under SQL Server), MIN/MAX returns a different byte-variant than the live server's scan-order representative. Surfaced by the AdventureWorks crosscheck on synthetic XML data (`vJobCandidateEducation._max_Edu_Loc_CountryRegion`). Needs trailing-space-insensitive compare + SQL Server's unspecified MAX-tie scan-order. See [`collations.md`](collations.md) "byte-exact sort" trailing-space note. **Deferred** — synthetic data, and the representative is unspecified scan-order on the live side.
6162
- **Leaked-connection session cleanup** — a `SimulatedDbConnection` that's never `Dispose`d never reclaims its session state: an open transaction holds its locks and pins the MVCC version store, `##temp` tables linger, session-owned app locks stay held, and the SPID accumulates. Real SqlClient's GC-finalization eventually closes a leaked connection and the server resets the session, so this is a genuine fidelity divergence. **Scope correction (2026-07-11): the fix is bigger than "weaken the registry."** Investigation found *three* global strong-reference cycles that pin exactly the connections that hold session state, so GC can't collect them and a finalizer never fires: (1) `LockResource.Hold.Owner` is a strong `SimulatedDbConnection` (reachable Database → table → lock → hold) — pins any lock- or session-app-lock-holding connection; (2) `HeapTable.OwnerConnection` is strong and `Simulation.GlobalTempTables` holds the table — pins any `##temp` owner; (3) `Database.ActiveSnapshotTxs` holds the transaction, which strongly refs its connection — pins any open-snapshot session. Weakening `Simulation.Connections` alone accomplishes nothing because the resource *is* the pin. A correct fix must break all three cycles — cleanest via a one-way `SessionToken` indirection (resources reference a lightweight token identity; the connection references the token, not vice versa) plus a finalizer that enqueues a **deferred teardown** drained on a normal worker thread (next `CreateDbConnection` / version-store GC) so transaction rollback stays off the finalizer thread. This is a broad, mechanical owner-indirection refactor landing on the most regression-sensitive subsystem (lock manager × GC timing × threading). Payoff is bounded (EF disposes scrupulously; only buggy consumer code leaks), so it's **deliberately deferred** as high-risk / low-frequency. Eventual home: [`locking.md`](locking.md).
6263
- **Workload-harness divergence reporting quirks** (`.vs/workload/Program.cs`, local-only) — the parity report's example line rebuilds parameters from the op seed and can mismatch the actual divergent instance, and divergent instances aren't re-run single-threaded to classify transient-vs-stable. Both made the 2026-07-10 shared-plan-state hunt slower than it needed to be (the fixed bug class itself — instance-bound aggregate/window results, baked TOP/OFFSET counts, frozen RAND, unstamped replay clock — is documented in [`plan-cache.md`](plan-cache.md)'s shared-plan contract section).

docs/claude/grammar.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,12 @@ Empty `""` (ON) and properly-closed empty `[]` raise **Msg 1038** (class 15, **s
4242

4343
- **Per-object creation-time QI capture is NOT modeled.** Real SQL Server stamps procedures / views / triggers / tables with the `QUOTED_IDENTIFIER` in effect at CREATE time (`sys.sql_modules.uses_quoted_identifier`, `OBJECTPROPERTY(id, 'IsQuotedIdentOn')`) and executes their bodies under that captured setting regardless of the caller's session. The simulator re-parses bodies under the **executing session's** current setting instead. Rare legacy pattern (creating an object under a non-default QI and relying on the stamp); most code runs everything under the default ON.
4444
- **Multi-statement-TVF bodies treat a `SET QUOTED_IDENTIFIER` as top-level** rather than rejecting it (real SQL Server disallows `SET QUOTED_IDENTIFIER` inside a function body).
45+
46+
## Expression nesting limits (Msg 8631 / Msg 191)
47+
48+
Expression parsing recurses per binary operator and grouping level (`TwoSidedExpression`'s parsing ctor re-enters `Expression.Parse` for its right side), and a .NET stack overflow is uncatchable and process-fatal — unacceptable for an in-process library handed a pathological query. Two graceful limits ship instead, both probed against SQL Server 2025 (2026-07-15):
49+
50+
- **Msg 8631, Class 17** (`ServerStackLimitReached()`): `Internal error: Server stack limit has been reached. Please look for potentially deep nesting in your query, and try to simplify it.` Raised from a stack probe (`RuntimeHelpers.EnsureSufficientExecutionStack()`) at the top of `Expression.Parse` — the faithful mechanism, since real's Msg 8631 is likewise a genuine stack probe with a stack-dependent threshold (reference: a 3000-term `1 + 1 + …` chain succeeds, 6000 fails). Every recursive parse path (boolean chains, CASE, function arguments, subquery projections) passes through `Expression.Parse` at least once per nesting level, so the single guard site bounds them all, and it composes across nested proc/dynamic-SQL batches because it measures the real stack.
51+
- **Msg 191, Class 15** (`StatementNestedTooDeeply()`): `Some part of your SQL statement is nested too deeply. Rewrite the query or break it up into smaller queries.` Structural counter on grouped-expression parens (`ParserContext.GroupingDepth`, limit 512 in `Expression.MaxGroupingDepth`).
52+
53+
**Divergences**: the simulator's tolerated operator-chain depth is well below real's because its parse frames are fat — measured on a default 1 MB thread, Msg 8631 fires near 750 levels (Release) / 600 (Debug), where the reference server handles 3000+ terms. The Msg-191 paren limit (512) is likewise below real's (1000 parens succeed there, 2000 fail) so the structural error deterministically beats the stack probe on 1 MB threads. Lifting the chain-depth ceiling toward real's requires the iterative/precedence-climbing parse restructure tracked in the backlog.

0 commit comments

Comments
 (0)