Skip to content

Commit eb4a42b

Browse files
committed
BACPAC loader — deferred computed columns via ALTER TABLE ADD AS. 6 of 8 WWI computed columns now land (the 2 holdouts reference json_query, which the simulator doesn't yet model); 6 cascade-blocked extended properties also unblock (SqlExtendedProperty 89 → 83). AW2025 unaffected (still 100% row coverage; computed-column emit attempts route to Skipped with a new "Deferred: …" reason prefix that the AW resilient-loader guard knowingly ignores).
**New phase 8** in `SqlServerSimulator/Storage/Bacpac/ModelXmlReader.cs` between programmable objects (phase 7) and extended properties (renumbered to phase 9). `EmitDeferredComputedColumns(SqlTable element)` re-walks each table's `SqlComputedColumn` children — the children are nested inside `SqlTable.Columns` rather than top-level, so dispatching `("SqlTable", 8) => Run(...)` causes RunPhase to visit every SqlTable element a second time. For each computed column the emitter extracts the leaf name + `ExpressionScript` (CDATA-wrapped raw T-SQL, already parenthesized by DACFx like `(concat([X],N' ',[Y]))`) and emits `ALTER TABLE [schema].[table] ADD [col] AS expr`. **PERSISTED qualifier dropped on purpose.** The simulator's read path for PERSISTED computed columns reads bytes from row storage rather than recomputing (verified at `Selection.Execution.Joins.cs:319`). But DACFx's BCP files exclude data for computed columns regardless of PERSISTED — real `bcp.exe` behavior. So a `PERSISTED`-emitted column would have no stored bytes for existing rows and reads would deserialize garbage. Emitting without PERSISTED routes reads through the recompute branch, giving identical query semantics with no caching. The PERSISTED-bypass is called out in both the inline comment and `docs/claude/bacpac-prerequisites.md`. **Companion BCP-loader filter** in `BacpacReader.LoadRowsFromBcp`. Before this bundle the loader passed `table.Columns` (full schema) straight through to `BcpRowReader.TryReadRow` + `RowEncoder.EncodeRow`. With computed columns now landing in the schema before BCP data loads, the wire-format expectations diverge: BCP files carry N stored columns per row, but the schema thinks there are N+M (including computed). The fix filters `table.Columns` to drop entries where `Computed != null` and builds a parallel filtered `columnIsAlias[]` array; both the reader and encoder see the same N-column view. Row bytes match the BCP file's actual storage shape; subsequent reads recompute on the fly via the existing read path. **Resilience-prefix split.** `EmitDeferredComputedColumns` catches its own per-column exceptions and records them with `"Deferred: …"` rather than the outer `RunPhase`-catch's `"Load failed: …"` prefix. Rationale: `Load_AW_No_Per_Element_Failures` is a defensive guard meant to catch *regressions* on previously-working elements. AW's `dbo.ufnLeadingZeros`-referencing computed column fails at ALTER TABLE ADD AS (the simulator's column-expression parser hits a UDF-resolution gap there — separate latent issue), and WWI's `json_query` columns fail because the function isn't modeled. Both are known unmodeled-feature gaps, not regressions, so the `"Deferred:"` prefix keeps the AW guard meaningful without false alarms. **One known limitation remaining**: 3 WWI filtered indexes reference computed columns (`IX_Purchasing_SupplierTransactions_IsFinalized`, `IX_Sales_CustomerTransactions_IsFinalized`, `IX_Sales_Invoices_ConfirmedDeliveryTime`). Indexes run in phase 5, computed columns in phase 8 — the columns don't exist yet when those indexes try to bind. Tried reordering indexes to run after computed columns; AW regressed because the index reorder also moved functions to phase 6, and AW has function-referencing computed columns that surface the simulator's ALTER-TABLE-ADD-AS UDF resolution gap. Rolled the reorder back; resolving the UDF gap is a separate (and larger) piece of work. The 3 WWI indexes stay on Skipped for now. **Test updates**: - `Load_WWI_Computed_Columns_Land_With_is_computed_Set` — verifies `sys.columns.is_computed = 1` returns 6 rows after WWI load. - `Load_WWI_Persisted_Computed_Column_Evaluates_On_Read` — verifies that `Application.People.SearchName` recomputes correctly from `concat(PreferredName, ' ', FullName)` for BCP-loaded rows (validates the end-to-end load → ALTER → recompute pipeline). - `Load_WWI_Known_Gaps_Recorded_In_Skipped` — refreshed expected counts: SqlExtendedProperty 89→83, SqlComputedColumn 8→2; comments updated to call out the json_query, sysname, and DECOMPRESS dependencies as concrete next-bundle candidates. - The 2 AW tests previously asserting `IsNotEmpty(SqlComputedColumn-in-Skipped)` still pass because AW's computed columns still go to Skipped (just with the new `"Deferred:"` reason prefix instead of the old `"deferred until functions land"` message). **Doc updates**: `CLAUDE.md`'s BACPAC pointer expands to call out the new phase 8 + the BCP-filters-computed-columns contract + the `"Deferred:"` vs `"Load failed:"` reason-prefix split. `docs/claude/bacpac-prerequisites.md` gets a new step 4 covering this bundle (with the PERSISTED-bypass rationale, the BCP-filter rationale, the index-reorder rollback note, and the resilience-prefix split); step 7's WWI gap inventory updates with the new counts and identifies the next-bundle candidates (JSON_QUERY, sysname, DECOMPRESS, SCHEMABINDING/EXEC AS, index-after-computed reorder).
1 parent bb1bb84 commit eb4a42b

5 files changed

Lines changed: 170 additions & 40 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` (8 phases: DB options + schemas + UDDTs + **sequences + roles + table types** → tables → constraints → FKs → indexes → views → programmable objects → 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), 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 + 4.7M rows**, remaining Skipped categories cataloged in `Load_WWI_Known_Gaps_Recorded_In_Skipped` (89 SqlExtendedProperty, 8 SqlComputedColumn, 3 SqlProcedure, 3 SqlIndex, 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 + 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).
168168

169169
## Not modeled
170170

SqlServerSimulator.Tests.Internal/Storage/BacpacLoaderTests.cs

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -599,18 +599,55 @@ 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-
AreEqual(89, grouped["SqlExtendedProperty"], "Extended properties (mostly on computed columns) deferred.");
603-
AreEqual(8, grouped["SqlComputedColumn"], "Computed columns deferred (same as AW).");
604-
AreEqual(3, grouped["SqlProcedure"], "3 WWI procedures parse-fail; need investigation.");
605-
AreEqual(3, grouped["SqlIndex"], "3 WWI indexes fail (likely filtered indexes on bit columns).");
602+
AreEqual(83, grouped["SqlExtendedProperty"], "Extended properties on the 2 still-deferred computed columns + on table-type columns + on filegroup.");
603+
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).");
605+
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).");
608608
AreEqual(1, grouped["SqlDatabaseOptions"], "Non-default collation deferred.");
609-
AreEqual(1, grouped["SqlView"], "1 WWI view parse-fails (computed-column referent).");
610-
AreEqual(1, grouped["SqlScalarFunction"], "1 WWI scalar function parse-fails.");
609+
AreEqual(1, grouped["SqlView"], "1 WWI view (Website.VehicleTemperatures) uses DECOMPRESS (not modeled).");
610+
AreEqual(1, grouped["SqlScalarFunction"], "1 WWI scalar function uses SCHEMABINDING / WITH EXEC AS clauses beyond RETURNS NULL ON NULL INPUT.");
611611
AreEqual(1, grouped["SqlFilegroup"], "Filegroup not in dispatcher.");
612612
}
613613

614+
[TestMethod]
615+
public void Load_WWI_Computed_Columns_Land_With_is_computed_Set()
616+
{
617+
var simulation = LoadWideWorldImporters(out _);
618+
using var connection = (SimulatedDbConnection)simulation.CreateDbConnection();
619+
connection.Open();
620+
using var command = connection.CreateCommand();
621+
// 6 of WWI's 8 computed columns succeed (the 2 referencing json_query
622+
// remain deferred). Verify the 6 surface in sys.columns with
623+
// is_computed = 1.
624+
command.CommandText = "SELECT COUNT(*) FROM sys.columns WHERE is_computed = 1;";
625+
using var reader = command.ExecuteReader();
626+
IsTrue(reader.Read());
627+
AreEqual(6, reader.GetInt32(0));
628+
}
629+
630+
[TestMethod]
631+
public void Load_WWI_Persisted_Computed_Column_Evaluates_On_Read()
632+
{
633+
// Application.People.SearchName is a PERSISTED computed column
634+
// defined as concat(PreferredName, N' ', FullName). Verify the
635+
// ALTER TABLE ADD AS landed AND that the existing BCP-loaded rows
636+
// can read it (the simulator recomputes on read, since PERSISTED
637+
// is a no-op in the simulator).
638+
var simulation = LoadWideWorldImporters(out _);
639+
using var connection = (SimulatedDbConnection)simulation.CreateDbConnection();
640+
connection.Open();
641+
using var command = connection.CreateCommand();
642+
command.CommandText = "SELECT TOP 1 PreferredName, FullName, SearchName FROM Application.People WHERE PreferredName IS NOT NULL AND FullName IS NOT NULL ORDER BY PersonID;";
643+
using var reader = command.ExecuteReader();
644+
IsTrue(reader.Read());
645+
var preferred = reader.GetString(0);
646+
var full = reader.GetString(1);
647+
var search = reader.GetString(2);
648+
AreEqual($"{preferred} {full}", search);
649+
}
650+
614651
[TestMethod]
615652
public void Load_WWI_Table_Types_Land_In_sys_table_types()
616653
{

SqlServerSimulator/Storage/Bacpac/BacpacReader.cs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,32 @@ private static void LoadRowsFromBcp(Stream bcpStream, HeapTable table, string sc
101101
? flags
102102
: new bool[table.Columns.Length];
103103

104+
// BCP files don't carry data for computed columns — neither real bcp.exe
105+
// nor DACFx export them. Filter computed columns out of the per-row
106+
// wire layout (read N stored values, encode an N-column row); the
107+
// simulator's row-storage and read paths already treat computed
108+
// columns as metadata-only (recomputed on read), so an encoded row
109+
// missing the computed-column slot is the same shape any INSERT
110+
// through normal DML would produce.
111+
var storedColumns = new List<HeapColumn>(table.Columns.Length);
112+
var storedIsAlias = new List<bool>(table.Columns.Length);
113+
for (var i = 0; i < table.Columns.Length; i++)
114+
{
115+
if (table.Columns[i].Computed is not null)
116+
continue;
117+
storedColumns.Add(table.Columns[i]);
118+
storedIsAlias.Add(i < columnIsAlias.Length && columnIsAlias[i]);
119+
}
120+
var storedCols = storedColumns.ToArray();
121+
var storedAlias = storedIsAlias.ToArray();
122+
104123
var rowCount = 0;
105124
while (true)
106125
{
107-
var values = BcpRowReader.TryReadRow(bcpStream, table.Columns, columnIsAlias);
126+
var values = BcpRowReader.TryReadRow(bcpStream, storedCols, storedAlias);
108127
if (values is null)
109128
break;
110-
var rowBytes = RowEncoder.EncodeRow(table.Columns, values, table.Heap);
129+
var rowBytes = RowEncoder.EncodeRow(storedCols, values, table.Heap);
111130
_ = table.Heap.Insert(rowBytes);
112131
rowCount++;
113132
}

0 commit comments

Comments
 (0)