Skip to content

Commit 9da889e

Browse files
committed
BACPAC loader — sysname keyword + BCP wire-format. All three previously-failing WWI procedures (Application.AddRoleMemberIfNonexistent, Application.CreateRoleIfNonexistent, Sequences.ReseedSequenceBeyondTableValues) — each declaring sysname parameters via DACFx's [sys].[sysname] TypeSpecifier — now CREATE successfully. WWI gap census drops by one whole category: SqlProcedure 3 → 0. AW unaffected.
**Parser fix** (`SqlServerSimulator/Storage/SqlType.cs::ResolveSimpleKeyword`). The simulator already shipped `SystemNameSqlType` (sys-schema alias over nvarchar(128) NOT NULL, with id 231 sharing nvarchar's system_type_id and user_type_id 256), but the length-7 dispatch arm in `ResolveSimpleKeyword` didn't include it. Any column / parameter declared `sysname` hit the `null` fallback and raised Msg 243 (CAST/parameter path) or Msg 2715 (column-declaration path). Added `"SYSNAME" => SystemName` to the length-7 case. **BCP wire-format fix** (`SqlServerSimulator/Storage/Bacpac/BcpRowReader.cs`). The variable-length-bounded dispatch switch didn't have a `SystemNameSqlType` arm, so a column declared as sysname would throw `NotSupportedException` ("BCP decoder doesn't yet handle type sysname") at row decode time. Added `SystemNameSqlType => ReadVarchar2(stream, type, ansi: false)` to the type-dispatch switch and `SystemNameSqlType => SqlValue.FromSystemName(text)` to the post-read materialization switch — same 2-byte LE length-prefix UTF-16 wire layout as nvarchar, which is what real SQL Server's BCP also emits for sysname columns. **Bacpac-loader workaround removed** (`SqlServerSimulator/Storage/Bacpac/ModelXmlReader.cs::NormalizeBuiltinName`). Previously expanded `[sys].[sysname]` → `nvarchar(128)` inline as a "the parser doesn't know sysname yet" workaround. With the parser fix in place, that's removed: bare `sysname` reaches the column-type parser as a keyword, and `sys.columns` now reports sysname as the type name for sysname-declared columns (previously it surfaced as nvarchar(128) due to the substitution). The XML doc on `NormalizeBuiltinName` is rewritten to match — generic sys-prefix stripping only. **Test updates**: - New `Load_WWI_Sysname_Procs_Land_In_sys_procedures` — verifies the three WWI procs are in `sys.procedures` (positive landing check). - `Load_WWI_Known_Gaps_Recorded_In_Skipped` — `IsFalse(grouped.ContainsKey("SqlProcedure"))` replaces the `AreEqual(3, …)` assertion since all 3 now load. **Doc updates**: `CLAUDE.md`'s BACPAC pointer refreshes the WWI element census (adds "42/42 procedures", drops "3 SqlProcedure" from the Skipped catalog). `docs/claude/bacpac-prerequisites.md` gets a new step 5 covering this bundle (with the rationale for removing the `nvarchar(128)` substitution, the BCP wire-format companion change, and the sys.columns fidelity win); the WWI gap inventory's SqlProcedure entry strikes through with a "shipped in step 5" note; the Status section's WWI summary picks up the 42/42 procedure count.
1 parent eb4a42b commit 9da889e

6 files changed

Lines changed: 43 additions & 17 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ Per-feature deep-dives live under `docs/claude/`. Each entry below is a trigger:
164164
- **Touching `XmlSqlType` (singleton in `Storage/XmlType.cs`, SystemTypeId=241, IsLob=true), `XmlSchemaCollection` storage class + `Schema.XmlSchemaCollections` dict + `Database.AllocateXmlCollectionId` (seeds at 65536 per probe), `HeapColumn.XmlSchemaCollection` per-column binding, the `xml`-recognizing arms of `SqlType.ResolveSimpleKeyword` / `Precedence` / `SystemTypeId`, the `xml(name)` / `xml(CONTENT name)` / `xml(DOCUMENT name)` column-type parsing in `ParseOneColumnIntoLists` (via `PeekIsXmlSchemaArgument` / `ParseXmlSchemaCollectionArgument`), `Simulation.Xml.cs` partial (CREATE XML SCHEMA COLLECTION / DROP XML SCHEMA COLLECTION / CREATE [PRIMARY] XML INDEX / FOR PATH|VALUE|PROPERTY secondary form), the `Xml` contextual keyword + `Keyword.Primary` dispatch in `TryParseCreate`, `XmlIndex` / `XmlSecondaryIndexType` storage on `HeapTable.XmlIndexes`, the `XmlMethodCall` expression in `Parser/Expressions/XmlMethodCall.cs` (closed accept-list of `value` / `nodes` / `query` / `exist` / `modify`, parses cleanly, throws `NotSupportedException` at `Run`), or the `sys.xml_schema_collections` / `sys.xml_indexes` catalog views** → [`docs/claude/bacpac-prerequisites.md`](docs/claude/bacpac-prerequisites.md) (the "`xml` data type + XML schema collections + XML methods + XML indexes" section carries the implementation detail).
165165
- **Touching `GeographySqlType` / `GeometrySqlType` (singletons in `Storage/SpatialType.cs`, both inherit `SpatialSqlType : SqlType(SqlTypeCategory.String)`, SystemTypeId=240, UserTypeId=130/129, IsLob=true), `SqlValue.FromGeography` / `FromGeometry` factories + the spatial branches in `FromString`, the `geography` / `geometry`-recognizing arms of `SqlType.ResolveSimpleKeyword` / `Precedence` / `SystemTypeId` / `UserTypeId`, the `SpatialMethodCall` expression in `Parser/Expressions/SpatialMethodCall.cs` (broad ~70-name accept-list of OGC + Microsoft-extension methods; parses cleanly + throws `NotSupportedException` at `Run` except `.ToString()` returns the stored WKT), the `SpatialStaticCall` expression in `Parser/Expressions/SpatialStaticCall.cs` (`geography::Parse(wkt)` / `geography::STGeomFromText(wkt, srid)` / `geometry::Point(x, y, srid)` work; other static methods throw), the `::` dispatch for `geography`/`geometry` in `Expression.cs` alongside `hierarchyid::`, the `.ToString()` polymorphic delegation in `HierarchyIdMethodCall.Run` (spatial receivers return WKT through the spatial path), the `SpatialIndex` storage class + `SpatialIndexKind` enum on `HeapTable.SpatialIndexes`, the `Simulation.Spatial.cs` partial (CREATE SPATIAL INDEX with USING / BOUNDING_BOX / GRIDS / CELLS_PER_OBJECT parsing + default tessellation_scheme), the `Spatial` contextual keyword routing in `TryParseCreate`, or the `sys.spatial_indexes` (23-col probe-confirmed) / `sys.spatial_index_tessellations` (16-col probe-confirmed) / `sys.spatial_reference_systems` (6-col, empty seed) catalog views** → [`docs/claude/bacpac-prerequisites.md`](docs/claude/bacpac-prerequisites.md) (the "`geography` / `geometry` data types" section carries the implementation detail).
166166
- **Adding a new top-level statement parser or changing the dispatch loop's statement-separator rules**[`docs/claude/grammar.md`](docs/claude/grammar.md) + [`docs/claude/control-flow.md`](docs/claude/control-flow.md).
167-
- **Working on BACPAC import (`Simulation.FromBacpac(string, out BacpacLoadResult)` + Stream overload, internal until externally consumed) — the model.xml dispatcher in `SqlServerSimulator/Storage/Bacpac/ModelXmlReader.cs` (9 phases: DB options + schemas + UDDTs + **sequences + roles + table types** → tables → constraints → FKs → indexes → views → programmable objects → **deferred computed columns via ALTER TABLE ADD AS** → extended properties → BCP data), the `BacpacReader` + `BcpRowReader` + `HierarchyIdWireDecoder` data path, the `BacpacLoadResult.Skipped` + `ElementCounts` + `Warnings` + `TableColumnIsAlias` diagnostics carriers, the resilient-loader contract (per-element exceptions land on Skipped with `"Load failed: …"` prefix and don't abort the load; computed-column failures use `"Deferred: …"` prefix so the AW guard stays meaningful), the **BCP-filters-computed-columns contract** (`BacpacReader.LoadRowsFromBcp` strips columns with `HeapColumn.Computed != null` before passing to `BcpRowReader`/`RowEncoder` — DACFx-emitted BCP files exclude computed columns from the wire layout regardless of PERSISTED; the loader emits computed columns *without* the PERSISTED qualifier so the simulator recomputes on every read), the wire-format matrix (fixed-raw / 1-byte-prefix / 2-byte-prefix / 8-byte-prefix tiers — see [`docs/claude/bacpac-prerequisites.md`](docs/claude/bacpac-prerequisites.md) "BCP wire format" section)** → [`docs/claude/bacpac-prerequisites.md`](docs/claude/bacpac-prerequisites.md). Loader baseline ships (2026-05-15) with AdventureWorks2025 at **100% row coverage** (760,167 / 760,167; zero BCP-file failures); WideWorldImporters-Standard loads end-to-end with **48/48 tables + 26/26 sequences + 9/9 roles + 4/4 table types + 6/8 computed columns + 4.7M rows**, remaining Skipped categories cataloged in `Load_WWI_Known_Gaps_Recorded_In_Skipped` (83 SqlExtendedProperty, 3 SqlProcedure, 3 SqlIndex, 2 SqlComputedColumn (json_query unmodeled), 2 SqlCheckConstraint, 2 SqlPermissionStatement, 1 each SqlView / SqlScalarFunction / SqlFilegroup / SqlDatabaseOptions-Collation). MAX types (varchar(MAX) / nvarchar(MAX) / varbinary(MAX)) + xml + CLR-UDT family all use the simple inline 8-byte-prefix wire shape (NOT TDS-PLP chunked form); plain `bit` uses 1-byte length prefix regardless of nullability (probe-confirmed via Production.Document.FolderFlag, the sole non-UDDT bit column in AW). Hierarchyid wire decoder covers AW's [0..79] positive-ordinal envelope via a 4-prefix order-preserving prefix code (`01` / `100` / `101` / `110`); negative ordinals + ordinals ≥ 80 + dotted sub-ordinals raise `NotSupportedException` for a follow-up bundle to extend. Geography rows load with `SpatialLocation = NULL` (WKB→WKT decoding deferred).
167+
- **Working on BACPAC import (`Simulation.FromBacpac(string, out BacpacLoadResult)` + Stream overload, internal until externally consumed) — the model.xml dispatcher in `SqlServerSimulator/Storage/Bacpac/ModelXmlReader.cs` (9 phases: DB options + schemas + UDDTs + **sequences + roles + table types** → tables → constraints → FKs → indexes → views → programmable objects → **deferred computed columns via ALTER TABLE ADD AS** → extended properties → BCP data), the `BacpacReader` + `BcpRowReader` + `HierarchyIdWireDecoder` data path, the `BacpacLoadResult.Skipped` + `ElementCounts` + `Warnings` + `TableColumnIsAlias` diagnostics carriers, the resilient-loader contract (per-element exceptions land on Skipped with `"Load failed: …"` prefix and don't abort the load; computed-column failures use `"Deferred: …"` prefix so the AW guard stays meaningful), the **BCP-filters-computed-columns contract** (`BacpacReader.LoadRowsFromBcp` strips columns with `HeapColumn.Computed != null` before passing to `BcpRowReader`/`RowEncoder` — DACFx-emitted BCP files exclude computed columns from the wire layout regardless of PERSISTED; the loader emits computed columns *without* the PERSISTED qualifier so the simulator recomputes on every read), the wire-format matrix (fixed-raw / 1-byte-prefix / 2-byte-prefix / 8-byte-prefix tiers — see [`docs/claude/bacpac-prerequisites.md`](docs/claude/bacpac-prerequisites.md) "BCP wire format" section)** → [`docs/claude/bacpac-prerequisites.md`](docs/claude/bacpac-prerequisites.md). Loader baseline ships (2026-05-15) with AdventureWorks2025 at **100% row coverage** (760,167 / 760,167; zero BCP-file failures); WideWorldImporters-Standard loads end-to-end with **48/48 tables + 26/26 sequences + 9/9 roles + 4/4 table types + 6/8 computed columns + 42/42 procedures + 4.7M rows**, remaining Skipped categories cataloged in `Load_WWI_Known_Gaps_Recorded_In_Skipped` (83 SqlExtendedProperty, 3 SqlIndex, 2 SqlComputedColumn (json_query unmodeled), 2 SqlCheckConstraint, 2 SqlPermissionStatement, 1 each SqlView / SqlScalarFunction / SqlFilegroup / SqlDatabaseOptions-Collation). MAX types (varchar(MAX) / nvarchar(MAX) / varbinary(MAX)) + xml + CLR-UDT family all use the simple inline 8-byte-prefix wire shape (NOT TDS-PLP chunked form); plain `bit` uses 1-byte length prefix regardless of nullability (probe-confirmed via Production.Document.FolderFlag, the sole non-UDDT bit column in AW). Hierarchyid wire decoder covers AW's [0..79] positive-ordinal envelope via a 4-prefix order-preserving prefix code (`01` / `100` / `101` / `110`); negative ordinals + ordinals ≥ 80 + dotted sub-ordinals raise `NotSupportedException` for a follow-up bundle to extend. Geography rows load with `SpatialLocation = NULL` (WKB→WKT decoding deferred).
168168

169169
## Not modeled
170170

SqlServerSimulator.Tests.Internal/Storage/BacpacLoaderTests.cs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,9 +599,9 @@ public void Load_WWI_Known_Gaps_Recorded_In_Skipped()
599599
var grouped = diagnostics.Skipped.GroupBy(s => s.ElementType)
600600
.ToDictionary(g => g.Key, g => g.Count());
601601
IsFalse(grouped.ContainsKey("SqlTableType"), "SqlTableType is dispatched; should not appear in Skipped.");
602+
IsFalse(grouped.ContainsKey("SqlProcedure"), "All 3 previously-failing sysname-using procs now load (sysname keyword wired into SqlType.ResolveSimpleKeyword).");
602603
AreEqual(83, grouped["SqlExtendedProperty"], "Extended properties on the 2 still-deferred computed columns + on table-type columns + on filegroup.");
603604
AreEqual(2, grouped["SqlComputedColumn"], "2 WWI computed columns invoke json_query (not modeled); both routed via 'Deferred:' prefix so the AW guard stays meaningful.");
604-
AreEqual(3, grouped["SqlProcedure"], "3 WWI procedures parse-fail (sysname system-type modeling deferred).");
605605
AreEqual(3, grouped["SqlIndex"], "3 filtered indexes reference computed columns that don't yet exist at phase 5 (computed columns land in phase 8). Reordering indexes to a later phase tripped an unrelated UDF-resolution gap in ALTER TABLE ADD AS for AW; deferred.");
606606
AreEqual(2, grouped["SqlCheckConstraint"], "2 JSON check constraints deferred.");
607607
AreEqual(2, grouped["SqlPermissionStatement"], "GRANT/REVOKE wire-up deferred (same as AW).");
@@ -648,6 +648,33 @@ public void Load_WWI_Persisted_Computed_Column_Evaluates_On_Read()
648648
AreEqual($"{preferred} {full}", search);
649649
}
650650

651+
[TestMethod]
652+
public void Load_WWI_Sysname_Procs_Land_In_sys_procedures()
653+
{
654+
// The three WWI procs taking sysname parameters
655+
// ([Application].[AddRoleMemberIfNonexistent],
656+
// [Application].[CreateRoleIfNonexistent],
657+
// [Sequences].[ReseedSequenceBeyondTableValues]) now load — sysname
658+
// is recognized as a keyword. Verify they're in sys.procedures.
659+
var simulation = LoadWideWorldImporters(out _);
660+
using var connection = (SimulatedDbConnection)simulation.CreateDbConnection();
661+
connection.Open();
662+
using var command = connection.CreateCommand();
663+
command.CommandText = """
664+
SELECT name FROM sys.procedures
665+
WHERE name IN ('AddRoleMemberIfNonexistent', 'CreateRoleIfNonexistent', 'ReseedSequenceBeyondTableValues')
666+
ORDER BY name;
667+
""";
668+
using var reader = command.ExecuteReader();
669+
var names = new List<string>();
670+
while (reader.Read())
671+
names.Add(reader.GetString(0));
672+
HasCount(3, names);
673+
AreEqual("AddRoleMemberIfNonexistent", names[0]);
674+
AreEqual("CreateRoleIfNonexistent", names[1]);
675+
AreEqual("ReseedSequenceBeyondTableValues", names[2]);
676+
}
677+
651678
[TestMethod]
652679
public void Load_WWI_Table_Types_Land_In_sys_table_types()
653680
{

SqlServerSimulator/Storage/Bacpac/BcpRowReader.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ private static SqlValue DecodeColumn(PushbackStream stream, HeapColumn column, b
122122
{
123123
VarcharSqlType => ReadVarchar2(stream, type, ansi: true),
124124
NVarcharSqlType => ReadVarchar2(stream, type, ansi: false),
125+
SystemNameSqlType => ReadVarchar2(stream, type, ansi: false),
125126
NCharSqlType => ReadVarchar2(stream, type, ansi: false),
126127
CharSqlType => ReadVarchar2(stream, type, ansi: true),
127128
VarbinarySqlType => ReadVarbinary2(stream, type),
@@ -330,6 +331,7 @@ private static SqlValue ReadVarchar2(PushbackStream stream, SqlType type, bool a
330331
{
331332
VarcharSqlType => SqlValue.FromVarchar(text),
332333
NVarcharSqlType => SqlValue.FromNVarchar(text),
334+
SystemNameSqlType => SqlValue.FromSystemName(text),
333335
NCharSqlType => SqlValue.FromNChar(type, text),
334336
CharSqlType => SqlValue.FromChar(type, text),
335337
_ => throw new InvalidOperationException(),

SqlServerSimulator/Storage/Bacpac/ModelXmlReader.cs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -640,20 +640,15 @@ private static string TranslateTypeSpecifier(XElement typeSpec)
640640
/// <summary>
641641
/// Strips the brackets and any <c>[sys].</c> qualifier from a built-in
642642
/// type reference. <c>[int]</c> → <c>int</c>, <c>[sys].[hierarchyid]</c>
643-
/// → <c>hierarchyid</c>. The simulator's <see cref="SqlType.GetByName"/>
644-
/// keyword table doesn't include <c>sysname</c> (it's a sys-schema alias
645-
/// over nvarchar(128) rather than a parser keyword); the loader expands
646-
/// the alias inline so DACFx's <c>[sys].[sysname]</c> reaches the parser
647-
/// as <c>nvarchar(128)</c>. Surface fidelity loss: <c>sys.columns</c>
648-
/// reports nvarchar(128) instead of sysname for the affected columns —
649-
/// acceptable for the loader baseline (storage shape is identical).
643+
/// → <c>hierarchyid</c>, <c>[sys].[sysname]</c> → <c>sysname</c>. The
644+
/// returned bare name is what the simulator's parser sees in a column /
645+
/// parameter type position.
650646
/// </summary>
651647
private static string NormalizeBuiltinName(string bracketedName)
652648
{
653649
var lastDot = bracketedName.LastIndexOf('.');
654650
var lastSegment = lastDot < 0 ? bracketedName : bracketedName[(lastDot + 1)..];
655-
var bare = lastSegment.Trim('[', ']');
656-
return string.Equals(bare, "sysname", StringComparison.OrdinalIgnoreCase) ? "nvarchar(128)" : bare;
651+
return lastSegment.Trim('[', ']');
657652
}
658653

659654
private static bool ReadBoolProperty(XElement element, string name, bool defaultValue) =>

SqlServerSimulator/Storage/SqlType.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -842,6 +842,7 @@ private static (SqlType Type, int? MaxLength) ResolveFixedString(int? declaredMa
842842
{
843843
"TINYINT" => TinyInt,
844844
"VARCHAR" => Varchar,
845+
"SYSNAME" => SystemName,
845846
_ => null,
846847
},
847848
8 => upper switch

0 commit comments

Comments
 (0)