Skip to content

Commit 88ae7b1

Browse files
committed
Optimized system stored procedure lookup.
1 parent 878d365 commit 88ae7b1

3 files changed

Lines changed: 108 additions & 77 deletions

File tree

SqlServerSimulator/Collation.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,17 @@ private protected Collation()
4545
{
4646
}
4747

48+
/// <summary>
49+
/// Lazily-built set of the system procedure names EXEC routes to
50+
/// built-in handlers, using this collation as the comparer. Owned and
51+
/// populated by <see cref="Simulation"/>'s EXEC parser (see
52+
/// <c>Simulation.Exec.cs::ResolveSystemProcedureName</c>, including its
53+
/// note on the benign first-touch race); held here so the cache follows
54+
/// the interned collation instance's lifetime. Contents are fixed
55+
/// literals, so sharing across simulations is safe.
56+
/// </summary>
57+
internal HashSet<string>? SystemProcedureLookup;
58+
4859
public abstract string Name { get; }
4960

5061
/// <summary>

SqlServerSimulator/Simulation/Simulation.Exec.cs

Lines changed: 96 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Text;
12
using SqlServerSimulator.Parser;
23
using SqlServerSimulator.Parser.Tokens;
34
using SqlServerSimulator.Storage;
@@ -6,6 +7,78 @@ namespace SqlServerSimulator;
67

78
partial class Simulation
89
{
10+
/// <summary>
11+
/// Matches <paramref name="leaf"/> against the system procedure names
12+
/// under the database collation's equality, returning the canonical
13+
/// as-declared name (so the dispatch switch in <see cref="ParseExec"/>
14+
/// can match ordinal string constants) or null when the name is not a
15+
/// system procedure. Matching is collation-aware — probe-confirmed
16+
/// (2026-05-21): system proc names follow the database collation, so
17+
/// <c>SP_EXECUTESQL</c> on a case-sensitive database misses here and
18+
/// falls through to "procedure not found".
19+
/// </summary>
20+
private static string? ResolveSystemProcedureName(Collation collation, string leaf)
21+
{
22+
// Lazily built once per interned collation instance (stable across
23+
// ALTER DATABASE COLLATE, which swaps the database's field to a
24+
// different interned instance). A concurrent first-touch race is
25+
// benign: both threads build identical sets and the reference
26+
// assignment is atomic.
27+
var lookup = collation.SystemProcedureLookup ??= new HashSet<string>(
28+
[
29+
// sp_executesql is a built-in proc with a special argument
30+
// shape (param-defs and OUTPUT writeback to @-variables in
31+
// the caller's scope) parsed by ParseSpExecuteSql rather
32+
// than the generic EXEC-argument grammar.
33+
"sp_executesql",
34+
// Extended-property sprocs — all 3 share argument parsing +
35+
// target resolution via InvokeSpExtendedProperty. The
36+
// bacpac loader emits `EXEC sp_addextendedproperty …` for
37+
// every `<SqlExtendedProperty>` element in model.xml; the
38+
// update/drop variants round out the API.
39+
"sp_addextendedproperty",
40+
"sp_updateextendedproperty",
41+
"sp_dropextendedproperty",
42+
// Linked-server sprocs — sp_addlinkedserver / sp_dropserver
43+
// carry semantic effect (activating / deactivating an entry
44+
// in Simulation.ActiveLinkedServers); sp_addlinkedsrvlogin /
45+
// sp_droplinkedsrvlogin / sp_serveroption parse-and-discard
46+
// since the simulator has no principal-mapping or
47+
// per-server-option model but BACPAC / migration scripts
48+
// often emit them.
49+
"sp_addlinkedserver",
50+
"sp_dropserver",
51+
"sp_addlinkedsrvlogin",
52+
"sp_droplinkedsrvlogin",
53+
"sp_serveroption",
54+
"sp_set_session_context",
55+
"sp_getapplock",
56+
"sp_releaseapplock",
57+
],
58+
collation);
59+
if (lookup.TryGetValue(leaf, out var canonical))
60+
return canonical;
61+
62+
// A set miss is definitive only when the probe hashes consistently
63+
// with the stored names. SqlLatin1Cp1CiAsCollation.GetHashCode
64+
// hashes in-repertoire strings by SQL sort weights but
65+
// out-of-repertoire strings via its inner Windows collation, so an
66+
// out-of-repertoire spelling that IS Equals-equal to a stored name
67+
// (fullwidth sp_executesql — regime-1 fullwidth folding) hashes
68+
// differently and misses the set. Every stored name is ASCII and
69+
// therefore in-repertoire, so a pure-ASCII probe shares their
70+
// hashing scheme and its miss is exact; anything else re-checks by
71+
// direct equality.
72+
if (Ascii.IsValid(leaf))
73+
return null;
74+
foreach (var name in lookup)
75+
{
76+
if (collation.Equals(leaf, name))
77+
return name;
78+
}
79+
return null;
80+
}
81+
982
/// <summary>
1083
/// Parses an <c>[@rc =] EXEC[UTE] target [args]</c> statement where
1184
/// <c>target</c> is either a procedure name (regular EXEC), a
@@ -74,85 +147,31 @@ private IEnumerable<SimulatedStatementOutcome> ParseExec(BatchContext batch)
74147
var procName = BatchContext.ParseObjectName(context);
75148
context.MoveNextOptional();
76149

77-
// sp_executesql is a built-in proc with a special argument shape
78-
// (param-defs and OUTPUT writeback to @-variables in the caller's
79-
// scope). Route to its own parser before falling through to the
80-
// generic-procedure path. Probe-confirmed (2026-05-21): system proc
81-
// names follow the database collation, so `SP_EXECUTESQL` on a CS
82-
// database raises "procedure not found" — that's what falls through
83-
// here when the case mismatch causes a dispatch miss.
84-
var dbCollation = batch.CurrentDatabase.Collation;
85-
if (dbCollation.Equals(procName.Leaf, "sp_executesql"))
150+
// System procedures route to built-in handlers before generic
151+
// resolution. ResolveSystemProcedureName does the collation-aware
152+
// match and hands back the canonical as-declared name, so the
153+
// switch arms below match ordinary string constants regardless of
154+
// the SQL text's casing. A null falls through to user-procedure
155+
// resolution.
156+
var systemProcName = ResolveSystemProcedureName(batch.CurrentDatabase.Collation, procName.Leaf);
157+
var systemProc = systemProcName switch
86158
{
87-
foreach (var outcome in ParseSpExecuteSql(batch, returnCodeVar))
88-
yield return outcome;
89-
yield break;
90-
}
91-
92-
// Extended-property sprocs — all 3 share argument parsing + target
93-
// resolution via InvokeSpExtendedProperty. The bacpac loader emits
94-
// `EXEC sp_addextendedproperty …` for every `<SqlExtendedProperty>`
95-
// element in model.xml; the update/drop variants round out the API.
96-
if (dbCollation.Equals(procName.Leaf, "sp_addextendedproperty"))
97-
{
98-
foreach (var outcome in InvokeSpExtendedProperty(batch, ExtendedPropertyOp.Add))
99-
yield return outcome;
100-
yield break;
101-
}
102-
if (dbCollation.Equals(procName.Leaf, "sp_updateextendedproperty"))
103-
{
104-
foreach (var outcome in InvokeSpExtendedProperty(batch, ExtendedPropertyOp.Update))
105-
yield return outcome;
106-
yield break;
107-
}
108-
if (dbCollation.Equals(procName.Leaf, "sp_dropextendedproperty"))
109-
{
110-
foreach (var outcome in InvokeSpExtendedProperty(batch, ExtendedPropertyOp.Drop))
111-
yield return outcome;
112-
yield break;
113-
}
114-
115-
// Linked-server sprocs — sp_addlinkedserver / sp_dropserver carry
116-
// semantic effect (activating / deactivating an entry in
117-
// Simulation.ActiveLinkedServers); sp_addlinkedsrvlogin /
118-
// sp_droplinkedsrvlogin / sp_serveroption parse-and-discard since
119-
// the simulator has no principal-mapping or per-server-option model
120-
// but BACPAC / migration scripts often emit them.
121-
if (dbCollation.Equals(procName.Leaf, "sp_addlinkedserver"))
122-
{
123-
foreach (var outcome in InvokeSpAddLinkedServer(batch))
124-
yield return outcome;
125-
yield break;
126-
}
127-
if (dbCollation.Equals(procName.Leaf, "sp_dropserver"))
128-
{
129-
foreach (var outcome in InvokeSpDropServer(batch))
130-
yield return outcome;
131-
yield break;
132-
}
133-
if (dbCollation.Equals(procName.Leaf, "sp_set_session_context"))
134-
{
135-
foreach (var outcome in InvokeSpSetSessionContext(batch))
136-
yield return outcome;
137-
yield break;
138-
}
139-
if (dbCollation.Equals(procName.Leaf, "sp_getapplock"))
140-
{
141-
foreach (var outcome in InvokeSpGetAppLock(batch, returnCodeVar))
142-
yield return outcome;
143-
yield break;
144-
}
145-
if (dbCollation.Equals(procName.Leaf, "sp_releaseapplock"))
146-
{
147-
foreach (var outcome in InvokeSpReleaseAppLock(batch, returnCodeVar))
148-
yield return outcome;
149-
yield break;
150-
}
151-
if (dbCollation.Equals(procName.Leaf, "sp_addlinkedsrvlogin")
152-
|| dbCollation.Equals(procName.Leaf, "sp_droplinkedsrvlogin")
153-
|| dbCollation.Equals(procName.Leaf, "sp_serveroption"))
159+
null => null,
160+
"sp_addextendedproperty" => InvokeSpExtendedProperty(batch, ExtendedPropertyOp.Add),
161+
"sp_addlinkedserver" => InvokeSpAddLinkedServer(batch),
162+
"sp_addlinkedsrvlogin" or "sp_droplinkedsrvlogin" or "sp_serveroption" => InvokeSpLinkedServerNoOp(batch),
163+
"sp_dropextendedproperty" => InvokeSpExtendedProperty(batch, ExtendedPropertyOp.Drop),
164+
"sp_dropserver" => InvokeSpDropServer(batch),
165+
"sp_executesql" => ParseSpExecuteSql(batch, returnCodeVar),
166+
"sp_getapplock" => InvokeSpGetAppLock(batch, returnCodeVar),
167+
"sp_releaseapplock" => InvokeSpReleaseAppLock(batch, returnCodeVar),
168+
"sp_set_session_context" => InvokeSpSetSessionContext(batch),
169+
"sp_updateextendedproperty" => InvokeSpExtendedProperty(batch, ExtendedPropertyOp.Update),
170+
_ => throw new InvalidOperationException($"{systemProcName} is in SystemProcedureNames but has no dispatch arm."),
171+
};
172+
if (systemProc is not null)
154173
{
155-
foreach (var outcome in InvokeSpLinkedServerNoOp(batch))
174+
foreach (var outcome in systemProc)
156175
yield return outcome;
157176
yield break;
158177
}

docs/claude/backlog.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ Real bugs / limitations against shipped behavior — fixes are concrete work, no
4646

4747
- **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.
4848
- **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).
49+
- **`SqlLatin1Cp1CiAsCollation.GetHashCode` inconsistent with `Equals` across the repertoire boundary** — the hybrid default collation hashes in-repertoire strings by SQL sort weights but out-of-repertoire strings via its inner Windows collation, so a pair that `Equals` deems equal across the boundary (fullwidth `schema` vs `schema`, the regime-1 fullwidth fold) hashes to different buckets and misses any hash container keyed by the collation. Probe-confirmed divergences (2026-07-13, default collation): real SQL Server resolves fullwidth spellings of user **table / schema / procedure names** (simulator: Msg 208 / "could not find stored procedure" — `Database.Schemas` / `Schema.HeapTables` / `Procedures` dictionary hash-miss), resolves fullwidth **`@variable` references** (simulator: "must declare" — `BatchContext.Variables` hash-miss), and raises **Msg 8143** for a duplicate EXEC named argument spelled `@a` + fullwidth `@a` (simulator: silently binds both — `Simulation.Exec.cs` `seenNames` hash-miss). Column references, named-arg→parameter matching, and `#temp` references already match (linear `Equals` paths). The system-procedure dispatch in `ParseExec` works around it locally (ASCII probes hash consistently with the all-ASCII stored names, so only non-ASCII probes take a linear `Equals` fallback — see `ResolveSystemProcedureName`); that trick doesn't generalize to user-named keys, which can themselves be out-of-repertoire. A general fix means making `GetHashCode` constant on cross-boundary equivalence classes — e.g. hashing everything through the inner Windows collation, which is safe only if weight-equality implies inner-equality for in-repertoire pairs (false collisions are harmless, false splits are not; the ligature/ignorable tiebreaks are the suspect cases) — and needs its own probe-backed bundle since the hash is load-bearing for GROUP BY / DISTINCT / seek caches. `CultureCollation` and the binary collations are self-consistent and unaffected.
4950
- **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).
5051

5152
## Design choices to revisit

0 commit comments

Comments
 (0)