From 60f16ffc2e68ece56606d3d2cf6b4b1f5c1dee71 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 08:15:17 +0100 Subject: [PATCH 01/41] Start the Dapper/Dapper.AOT parity accounting under notes/ The goal on record: enable Dapper.AOT in the Dapper test suite, announce types via attributes, and have it swallow everything - AOT-clean. - parity.md: the feature table, with impact/complexity per gap (several 'gaps' score zero because the concept doesn't exist under AOT, e.g. the ref-emit plan cache) - tokens.md: @ids expansion, {=literal}, ?foo? pseudo-positional, param filtering - type-vs-generic.md: the announced-types design space for Type-based APIs - test-suite-audit.md: the Dapper tests as acceptance corpus, sequenced --- notes/README.md | 50 +++++++++++++++ notes/parity.md | 128 ++++++++++++++++++++++++++++++++++++++ notes/test-suite-audit.md | 82 ++++++++++++++++++++++++ notes/tokens.md | 110 ++++++++++++++++++++++++++++++++ notes/type-vs-generic.md | 68 ++++++++++++++++++++ 5 files changed, 438 insertions(+) create mode 100644 notes/README.md create mode 100644 notes/parity.md create mode 100644 notes/test-suite-audit.md create mode 100644 notes/tokens.md create mode 100644 notes/type-vs-generic.md diff --git a/notes/README.md b/notes/README.md new file mode 100644 index 00000000..695d679c --- /dev/null +++ b/notes/README.md @@ -0,0 +1,50 @@ +# Dapper / Dapper.AOT unification — working notes + +These notes are the working set for closing the gap between vanilla Dapper and Dapper.AOT, +with the eventual goal of retiring ref-emit entirely. + +## The success criterion + +> We turn Dapper.AOT "on" in the **Dapper test suite** (`Dapper/tests/Dapper.Tests`), announce +> the types in play via (new) attributes, and Dapper.AOT swallows *everything* — every call site +> intercepted, all tests green, with Dapper.AOT doing all the work. Bonus: the project compiles +> in AOT mode without warnings. + +That makes the Dapper test suite the **acceptance corpus**: parity is not "the API list looks +covered", it is "these tests pass through generated code". The corpus conveniently already +exercises the obscure corners (list expansion, literals, pseudo-positional parameters, type +handlers, multi-map, GridReader, dynamic rows, output parameters, ...), across many providers. + +"Announce the types" is the one concession we ask of consumers: the `Type`-based (non-generic) +APIs and anything else that discovers types at runtime need the candidate types stated at +build time. See [type-vs-generic.md](type-vs-generic.md). + +## The documents + +| doc | contents | +| --- | --- | +| [parity.md](parity.md) | the feature parity table: Dapper's surface vs Dapper.AOT today | +| [tokens.md](tokens.md) | special string-token handling: `@ids` expansion, `{=literal}`, `?foo?`, etc | +| [type-vs-generic.md](type-vs-generic.md) | `Type`-based vs `` APIs, and the "announce your types" design space | +| [test-suite-audit.md](test-suite-audit.md) | the Dapper test files as acceptance corpus, and what blocks each | + +## Honesty rules + +- A status in these tables is only worth having if it was **verified against code** — the + interceptor's dispatch switch, a test fixture, or the Dapper source. Anything inferred or + remembered is marked ❓ until checked. +- "Compiles" is not "works": a call site left un-intercepted still compiles and passes tests + on a JIT runtime via vanilla Dapper. The corpus only measures us when interception is + *confirmed* (DAP000 counts, or Dapper is removed from the runtime closure). +- Statuses: ✅ supported (verified) · ⚠️ partial/constrained · ❌ not supported today · + 🚫 deliberate non-goal (decision recorded) · ❓ unverified. + +## Where the current status came from + +- Dapper.AOT's supported-method set: the dispatch switch in + `src/Dapper.AOT.Analyzers/Internal/Inspection.cs` (`IsDapperMethod`, `OperationFlags`). +- Fixture evidence: `test/Dapper.AOT.Test/Interceptors/*.input.cs` — a fixture with an + `.output.cs` generates; one without (e.g. `QueryMultiple`, `QueryMultiType`, + `DynamicParameters`) does not. +- Dapper's surface: `Dapper/Dapper/PublicAPI.Shipped.txt` plus source (`SqlMapper.cs` for the + token machinery). diff --git a/notes/parity.md b/notes/parity.md new file mode 100644 index 00000000..318befc7 --- /dev/null +++ b/notes/parity.md @@ -0,0 +1,128 @@ +# Feature parity: Dapper vs Dapper.AOT + +Status legend: ✅ supported (verified) · ⚠️ partial/constrained · ❌ not supported today · +🚫 deliberate non-goal (decision recorded) · ❓ unverified. See [README.md](README.md) for +where the evidence comes from. + +**Impact** = usefulness toward the goal (corpus density + real-world usage), zero/low/med/high. +Zero means the *concept does not exist* under AOT — e.g. pruning the ref-emit plan cache is a +zero on any scale, because there is no ref-emit plan cache in AOT. **Complexity** = initial +perceived effort to close the gap; "—" for rows already done. Both are first-pass estimates +for prioritization, expected to be revised. + +"Not supported today" means the call site is left on vanilla Dapper (works under JIT, fails +under AOT) — the generator's dispatch switch marks it `NotAotSupported`, or the parameter / +result shape makes generation bail. + +Two levers change several complexity scores and are worth naming up front: + +- **We own Dapper too.** Where interception is blocked by Dapper's own types (e.g. an + interceptor must *return* `SqlMapper.GridReader`, whose construction is internal), the fix + can be an extension point added to Dapper itself — virtual members, an accessible ctor, an + AOT-friendly interface — rather than heroics on the AOT side. +- **`[UnsafeAccessor]` (net8+)** reaches non-public members/ctors from generated C# without + reflection, exactly as the protobuf-net AOT generator does. Several "generated C# cannot do + what ref-emit did" limits (DAP017-adjacent) soften to "net8+ can, down-level cannot". + +## 1. Core API surface (`SqlMapper` extension methods) + +| Dapper API | AOT status | impact | complexity | notes | +| --- | --- | --- | --- | --- | +| `Query` / `QueryAsync` | ✅ | — | — | includes buffered/unbuffered flag | +| `QueryUnbufferedAsync` (`IAsyncEnumerable`) | ✅ | — | — | | +| `QueryFirst/Single[OrDefault]` + async | ✅ | — | — | row-count guidance via DAP229/230 | +| `Query` (non-generic → `dynamic` rows) | ✅ | — | — | see §3 dynamic-row fidelity | +| `Query` / untyped | ✅ | — | — | `QueryUntyped` fixture | +| `Query(Type, sql, ...)` + `First/Single[OrDefault]` + async | ❌ | med | med | needs announced types; see [type-vs-generic.md](type-vs-generic.md) | +| `Query` multi-map (2–7 + splitOn) | ❌ | **high** | med-high | `Arity > 1` → `NotAotSupported`. New read shape (splitOn slicing, per-type readers, user delegate), all sync/async/buffered variants | +| `Query(sql, Type[] types, Func map, ...)` | ❌ | low-med | low* | *after* multi-map + announced types land; incremental on both | +| `QueryMultiple` / `QueryMultipleAsync` (`GridReader`) | ❌ | **high** | high | interceptor must return Dapper's `GridReader` → needs a Dapper-side extension point (subclassable GridReader) or an AOT-owned grid API; then per-`Read` typing is a second problem (instance calls, not interceptable — likely: announced types + runtime dispatch) | +| `Execute` / `ExecuteAsync` | ✅ | — | — | | +| `Execute` with `IEnumerable` (multi-exec) | ✅ | — | low (verify) | AOT batches (`DbBatch`, `[BatchSize]`) — *better*; verify semantics match Dapper (order, transaction, partial failure, total rowcount) | +| `ExecuteScalar` / `ExecuteScalar` + async | ✅ | — | — | conversion fidelity in §3 | +| `ExecuteReader` / `ExecuteReaderAsync` | ❌ | med | low-med | command setup already generated; return the (wrapped) reader; `WrappedReader`/`IWrappedDataReader` disposal semantics | +| `GetRowParser(reader)` | ✅ | — | — | | +| `GetRowParser(reader, Type concreteType, ...)` | ❌ | med | low* | discriminator/polymorphism pattern; dictionary lookup once types are announced | +| `Parse` / `Parse(Type)` / `Parse` (dynamic) | ❌ ❓ | low | low | same reader machinery, different entry point | +| `AsTableValuedParameter` (`DataTable` / `SqlDataRecord`) | ❓ | med | med | SQL Server crowd; pairs with `ICustomQueryParameter` | +| `AsList` | n/a | — | — | trivial helper; confirm it doesn't count as a candidate site | +| `GetTypeDeserializer` / `CreateParamInfoGenerator` / `ReadChar` etc | 🚫 | **zero** | — | reflection-era machinery; no meaning under AOT | +| `PurgeQueryCache` / `GetCachedSQL*` / `GetHashCollissions` / `QueryCachePurged` | 🚫 | **zero** | — | there is no ref-emit plan cache in AOT | +| `Format` / `ReplaceLiterals` | ❓ | low | low | falls out of the literal-injection work (see [tokens.md](tokens.md)) | + +## 2. Parameters (input side) + +| feature | AOT status | impact | complexity | notes | +| --- | --- | --- | --- | --- | +| anonymous types / concrete POCOs | ✅ | — | — | | +| fields as members | ❓ | low | low | verify | +| `DynamicParameters` | ❌ | **high** | high | densest single gap: templates, `AddDynamicParams`, per-param DbType/direction/size/precision/scale, `Get` post-execute, output callbacks. Candidate design: a real (hand-written, AOT-safe) implementation in the runtime lib that generated commands consume — generation only needed for template objects | +| `SqlMapper.IDynamicParameters` (custom impls) | ❌ | low-med | med | interface receives the `IDbCommand`, so callable directly — blocked on `Identity` (Dapper-internal) in the signature; owning Dapper permits an AOT-friendly overload | +| `SqlMapper.ICustomQueryParameter` | ❌ ❓ | med | low | interface is `AddParameter(IDbCommand, string)` — generated code can just call it | +| `IParameterLookup` / `IParameterCallbacks` | ❌ ❓ | low | low-med | obscure but public | +| `DbString` | ✅ | — | — | DAP048 nudges to `[DbValue]`; keep the Dapper spelling, the corpus uses it | +| output / return params via `[DbValue(Direction=...)]` | ⚠️ | — | — | AOT spelling works; Dapper spelling rides on `DynamicParameters` above | +| list expansion (`in @ids`) | ❌ ❓ | **high** | med | ubiquitous. Runtime rewrite helper (per-invocation list size); analyzer already detects the shape. [tokens.md](tokens.md) §2 | +| literal injection (`{=name}`) | ❌ ❓ | med | low-med | formatting rules compile-time decidable. [tokens.md](tokens.md) §3 | +| pseudo-positional (`?foo?`) | ❌ ❓ | low | med | OleDb/Access corner. [tokens.md](tokens.md) §4 | +| enum / nullable / `char` / `Guid` params | ⚠️❓ | med | low | verify edge conversions vs Dapper | +| param filtering (only bind members named in SQL) + `SupportLegacyParameterTokens` | ❓ | med | low | AOT currently *includes* + warns (DAP236); on strict providers that's an error, so may need parity not preference | +| UDTs (`UdtTypeHandler`, geo types) | ❌ ❓ | low | med | provider-specific | +| XML types (`XmlDocument`/`XDocument`/`XElement`) | ❌ ❓ | low-med | low | treat as known types with fixed handlers | +| `CommandDefinition` incl. `CommandFlags.Pipelined` | ❓ | med | low-med | `NoCache` is **zero** (no cache to bypass); `Buffered` covered; `Pipelined` is a perf feature to verify | +| `commandTimeout` / `transaction` / `commandType` args | ✅ ❓ | — | — | verify `TableDirect` | +| `CancellationToken` | ✅ | — | — | AOT extends Dapper here (DAP044/045) | + +## 3. Results (output side) + +| feature | AOT status | impact | complexity | notes | +| --- | --- | --- | --- | --- | +| POCO binding: props, case-insensitive | ✅ | — | — | | +| non-public members / private types | ⚠️ | med | med | ref-emit could bypass accessibility; generated C# cannot — but `[UnsafeAccessor]` (net8+) covers members; private *types* stay hard | +| constructor binding, `[ExplicitConstructor]` | ✅ | — | — | plus factory methods (AOT extension) | +| `required` / init-only members | ✅ | — | — | `RequiredProperties` fixture | +| fields | ❓ | low | low | verify | +| `dynamic` rows (`DapperRow` fidelity) | ⚠️ | high | med | Dapper's row is `IDictionary` + `IReadOnlyDictionary`, tolerates add/remove, specific null/missing semantics — pin the matrix with tests | +| tuple results | ❌ | med | med | DAP013; design already framed by `[BindTupleByName]` | +| enum results (string→enum case-insens., widening, `ShortEnum`) | ⚠️❓ | high | low-med | Dapper recently changed precedence (prefer type handlers, #2200) — match the *new* behavior | +| `MatchNamesWithUnderscores` | ❓ | med-high | low | snake_case databases; needs a compile-time equivalent (global option/attr) | +| `SetTypeMap` / `CustomPropertyTypeMap` / `ITypeMap` / `TypeMapProvider` | ❌ 🚫? | med | med | runtime config by definition; AOT spelling is `[Column]`+`[UseColumnAttribute]`. Proposal: declare 🚫 for the runtime API, ship attribute equivalents + migration guidance | +| `AddTypeHandler` / `TypeHandler` / `StringTypeHandler` | ❌→⚠️ | **high** | med-high | AOT has its own `TypeHandler`; needs the unification story (how does a *Dapper* handler registration become an AOT one?) | +| `AddTypeMap` / `RemoveTypeMap` (scalar DbType map) | ❌ ❓ | low-med | low | e.g. `DateTime`→`DateTime2`; global compile-time option | +| `Settings.ApplyNullValues` | ❓ | low | low | | +| coercion matrix (`char`, `Nullable`, `Convert.ChangeType` fidelity) | ❓ | high | med | silent-wrongness risk; test-driven, differential against Dapper | +| column-level error reporting (`ThrowDataException` names column+value) | ❓ | med | low | DX parity worth keeping | +| `ExecuteScalar` conversions (null→default, enums, handlers) | ❓ | med | low | | + +## 4. Configuration & settings + +| feature | AOT status | impact | complexity | notes | +| --- | --- | --- | --- | --- | +| `Settings.CommandTimeout` (global default) | ❓ | med | low | AOT has per-site args + `[CommandProperty]`; needs a global knob | +| `Settings.InListStringSplitCount` | ❌ | med | low* | *after* list expansion; SQL Server plan-stability win | +| `Settings.PadListExpansions` | ❌ | low-med | low* | same | +| `Settings.UseSingleResult/UseSingleRowOptimization` | ❓ | low | low | AOT picks `CommandBehavior` itself; verify equivalence | +| `Settings.FetchSize` (Oracle) | ⚠️ | low | low | `GlobalFetchSize` exists; verify | +| `SqlMapper.ConnectionStringComparer` | 🚫? | **zero-ish** | — | exists to partition the runtime identity/cache — a concept AOT doesn't have | +| `FeatureSupport` (per-provider null-array quirks) | ❓ | low | low | folds into list-expansion helper | + +## 5. Sibling packages in the Dapper repo + +Scope decision needed per package: unify, ignore, or leave on vanilla Dapper. + +| package | impact | complexity | notes | +| --- | --- | --- | --- | +| `Dapper.SqlBuilder` | med | low* | runtime SQL + `DynamicParameters` — mostly falls out of the DynamicParameters work | +| `Dapper.Rainbow` | low | high | heavy runtime typing; candidate 🚫 | +| `Dapper.EntityFramework` | low | med | `DbGeography` handlers; rides on type-handler story | +| `Dapper.ProviderTools` | low | — | largely orthogonal to serialization; probably nothing to do | +| `Dapper.StrongName` etc | — | — | packaging variants, not features | + +## 6. AOT-only extensions (the "plus some") + +Recorded so the unified story stays a superset, not a port: batch execution (`DbBatch`, +`[BatchSize]`), cancellation via parameter members, named-tuple *parameter* binding +(`[BindTupleByName]`), factory-method construction, `[StrictTypes]`, `[QueryColumns]`, +`[CacheCommand]`, `[CommandProperty]` (provider-specific command props), `[RowCount]` / +`[RowCountHint]`, `TypeAccessor` + `SqlBulkCopy` bridge, `[IncludeLocation]`, deep TSQL +analysis (DAP2xx), `[SqlSyntax]`. diff --git a/notes/test-suite-audit.md b/notes/test-suite-audit.md new file mode 100644 index 00000000..b4fd747b --- /dev/null +++ b/notes/test-suite-audit.md @@ -0,0 +1,82 @@ +# The Dapper test suite as acceptance corpus + +Goal restated: enable Dapper.AOT in `Dapper/tests/Dapper.Tests`, announce types where the +`Type`-based APIs need it, and have every call site intercepted with all tests green — +ideally AOT-publishable without warnings. + +## Practicalities + +- The suite needs live databases. `tests/docker-compose.yml` exists in the Dapper repo; + installing SQL Server Developer Edition locally or using docker images is fine on this + machine. SQL Server is the core dependency (`TestBase` runs everything against both + `System.Data.SqlClient` and `Microsoft.Data.SqlClient`); the provider tests add SQLite, + MySQL, Postgres, Firebird, DuckDB, Snowflake, OleDb, Linq2Sql, EF. +- Interception requires C# ≥11 + net8 SDK and ``; the test + project must opt in. +- **Measurement**: DAP000 reports "handled N of M possible call-sites". The corpus number to + drive to 100% is that ratio (per test assembly), then the test pass rate, then the AOT + publish warning count — in that order. A passing test with an un-intercepted call site is + measuring vanilla Dapper, not us. +- Some tests assert *Dapper internals* (cache counts, `Identity`, deserializer internals); + those need triage into "port", "skip under AOT", or "equivalent AOT assertion". + +## Per-file first-pass audit + +Status here = "expected blockers when AOT is enabled", from file names + known contents; +refine by actually running with interception enabled and reading DAP000/DAP001 output. + +| test file | exercises | expected blockers today | +| --- | --- | --- | +| `QueryMultipleTests` | `QueryMultiple`/`GridReader` incl. multi-map reads, unbuffered grids | ❌ QueryMultiple wholesale | +| `MultiMapTests` | `Query`, splitOn variants, `Type[]` overload | ❌ multi-map wholesale | +| `ParameterTests` | DynamicParameters (templates, output, callbacks), list expansion incl. padding + string_split, TVPs, `ICustomQueryParameter`, `DbString`, UDTs, pseudo-positional | ❌ most of it: the single densest gap file | +| `LiteralTests` | `{=name}` injection, enums/bools as literals, in-list + literal combos | ❌ literal injection | +| `TypeHandlerTests` | `AddTypeHandler`, `StringTypeHandler`, `ITypeHandler`, `RemoveTypeMap`/`AddTypeMap` | ❌ runtime registration model | +| `PreferTypeHandlersForEnumsTests` | new enum/type-handler precedence (#2200) | ❌ same | +| `EnumTests` | enum coercions: string→enum, nullable, `ShortEnum`, in-list of enums | ⚠️ verify coercion matrix | +| `ConstructorTests` | ctor binding, `[ExplicitConstructor]`, mixed ctor+setter | ✅ mostly; verify edge rules match | +| `TupleTests` | tuple results and parameters | ❌ tuple results (DAP013/014) | +| `AsyncTests` | full async surface incl. unbuffered, cancellation, `Pipelined` flag | ⚠️ mostly ✅; `CommandFlags.Pipelined`/`CommandDefinition` paths ❓ | +| `SingleRowTests` | First/Single[OrDefault] semantics incl. empty/over-read | ✅ verify exception parity | +| `NullTests` | null handling, `ApplyNullValues` | ❓ | +| `DecimalTests`, `DateTimeOnlyTests` | numeric coercion; `DateOnly`/`TimeOnly` | ⚠️ AOT has DateOnly fixture; verify | +| `XmlTests` | XmlDocument/XDocument/XElement params+results | ❌ inbuilt XML handlers | +| `DataReaderTests` | `ExecuteReader`, `GetRowParser` (incl. `Type`-based discriminator), `Parse` | ❌ ExecuteReader, Type-based parser | +| `WrappedReaderTests` | `IWrappedDataReader` unwrap behavior | ❌ rides on ExecuteReader | +| `TransactionTests` | transaction plumbing incl. `TransactedConnection` (custom `IDbConnection` wrapper!) | ⚠️ custom `IDbConnection` impls — interceptor targets extension calls on the interface, so should be fine, but the *generated* code paths for non-`DbConnection` need checking | +| `ProcedureTests` | stored procs, output/return params via DynamicParameters | ❌ DynamicParameters.Get pattern | +| `MiscTests` | the grab-bag: dynamic rows semantics, `AsList`, huge param counts, field binding, private members, generic helper methods calling Dapper (`TestSubsequentQueriesSuccess` style) | ⚠️ several: **generic helper methods** are a known structural gap (DAP016: generic type parameters not supported) — indirect/helper usage is explicitly "not today" in the FAQ | +| `TypeAttributeTests`* / column mapping (in Misc/others) | `SetTypeMap`, `CustomPropertyTypeMap`, `MatchNamesWithUnderscores` | ❌ runtime type-map APIs | +| `SqlBuilderTests` | Dapper.SqlBuilder templates | ❌ rides on DynamicParameters | +| `Providers/*` | SQLite/MySQL/Postgres/Firebird/DuckDB/Snowflake/OleDb/EF/Linq2Sql | ⚠️ per-provider: `$`/`:`/`?` prefixes, pseudo-positional (OleDb), `DbGeography` (EF), provider-specific types | +| `ProviderTests` | `IDbConnection` vs `DbConnection` API split, `GetRowParser` polymorphism | ⚠️/❌ | + +\* exact file/test names to be confirmed when the suite is run with interception on — this +table was drawn from file names and Dapper source knowledge, not yet from a run. + +## Suggested sequencing (by corpus unblocked per unit of work) + +1. **DynamicParameters** — unblocks `ParameterTests`, `ProcedureTests`, `SqlBuilderTests`; + biggest single win and forces the output-parameter design. +2. **List expansion + literals + pseudo-positional** (one work item: the SQL-rewrite runtime + helper) — unblocks `LiteralTests`, much of `ParameterTests`, provider corpora. +3. **QueryMultiple/GridReader** — needs an AOT-owned GridReader equivalent; unblocks + `QueryMultipleTests` and parts of others. +4. **Multi-map** — `MultiMapTests`; generic arities first, `Type[]` after announcements land. +5. **Announced types** ([type-vs-generic.md](type-vs-generic.md)) — `Type`-based APIs, + `GetRowParser` discriminator, `Parse`. +6. **Type handlers unification** — decide the runtime-registration story; unblocks + `TypeHandlerTests`, `XmlTests`, EF geo types. +7. **ExecuteReader/WrappedReader**, tuple results, `ApplyNullValues`, settings parity — the + tail. + +Items 1–2 are also exactly where "obscure API usage" concentrates, which is why the corpus +ordering and the feature ordering agree. + +## Indirect usage (helper methods) + +The corpus (like real code) wraps Dapper in generic helpers (`Get`, repository patterns). +Interceptors work per-call-site with concrete types, so `helper` bodies calling +`connection.Query` are today's DAP016. The "announce your types" mechanism may double as +the answer here (instantiate the helper's generated code per announced type) — worth keeping +the two designs joined up. diff --git a/notes/tokens.md b/notes/tokens.md new file mode 100644 index 00000000..d31f3438 --- /dev/null +++ b/notes/tokens.md @@ -0,0 +1,110 @@ +# Special string-token handling + +Dapper rewrites the command text and/or parameter set in several documented-but-obscure ways. +Every one of these is observable behavior that real code depends on; each needs an explicit +AOT position: **replicate**, **replicate with constraints**, or **refuse loudly** (analyzer +error, never silent divergence). + +Source of truth: `Dapper/Dapper/SqlMapper.cs` (`PackListParameters`, `GetInListRegex`, +`GetLiteralTokens`, `ReplaceLiterals`, `SanitizeParameterValue`, the pseudo-positional +rewrite) and `SqlMapper.Settings`. Line refs are as of `72a54c4`. + +## 1. Parameter prefixes + +Dapper matches parameters in SQL with any of the prefixes `@`, `:`, `$`, `?` (regex class +`[?@:$]`). AOT's SQL analysis is TSQL-centric; the non-`@` prefixes matter for Postgres / +Oracle / MySQL / OleDb corpora. + +## 2. List expansion (`in @ids`) + +When a parameter member's value is an `IEnumerable` (and not string/byte[]/special-cased), +Dapper rewrites the SQL and explodes the parameter: + +- token grammar: `([?@:$]ids)(?!\w)` with an optional trailing `unknown` keyword (regex in + `GetInListRegex`, ~`SqlMapper.cs:2150`); there is also a *positional* variant `?ids?` for + pseudo-positional providers; +- non-empty list → `(@ids1, @ids2, ...)`, one `DbParameter` each; note the SQL text mutates + **per list size**, which is exactly why the plan-cache-pollution features below exist; +- empty list → a always-false/no-row construct (plus provider-aware null semantics via + `FeatureSupport`) rather than invalid `()` syntax; +- `Settings.PadListExpansions` → repeat the last value to round the count up, reducing + distinct SQL shapes; +- `Settings.InListStringSplitCount` (SQL Server, string-typed lists ≥ N) → rewrites to + `(select cast([value] as ) from string_split(@ids, ','))` with a *single* joined + parameter (~`SqlMapper.cs:2349`); +- the `... in @ids unknown` suffix opts a single expansion out of smart handling. + +**AOT today: nothing.** No trace in the generator or runtime library. + +**AOT design note:** the *detection* is static (member type is enumerable, token appears in +SQL — the analyzer already sees both), but the *rewrite* is inherently per-invocation (list +size). So generated code needs a runtime helper that takes (sql, values) and produces the +rewritten text + parameters — an AOT-friendly library routine, not codegen per size. The +`string_split` path is a per-provider decision the generator can bake in when the syntax is +known. + +## 3. Literal injection (`{=name}`) + +`{=name}` (regex `(?`: the non-generic APIs, and announcing types + +## The problem + +The generic APIs (`Query`) tell the generator the row type at the call site; those are +supported. The `Type`-based APIs do not: + +| API | shape | +| --- | --- | +| `Query(Type type, string sql, ...)` + `First/Single[OrDefault]` + async | row type is a runtime argument | +| `Query(sql, Type[] types, Func map, string splitOn, ...)` | arbitrary-arity multi-map over runtime types | +| `GetRowParser(reader, Type concreteType, ...)` | per-row polymorphism: read a discriminator column, pick a parser — the canonical use is *dynamic dispatch by design* | +| `ExecuteScalar` (untyped `object`) | mild version of the same | + +Today every one of these is `NotAotSupported` and the call is left on vanilla Dapper — i.e. +broken under AOT, silently fine under JIT. This mirrors the protobuf-net AOT problem +(`Serializer.NonGeneric`, `PBN3011`): a call site nobody can resolve statically is exactly +the kind that fails only after trimming. + +## The plan: announce the types + +We can't resolve `typeof(x)` flowing through variables, but we don't have to: the consumer +**declares the closed set of candidate types**, and the generator emits per-type handlers +plus a `Type → handler` dispatch map. The `Type` argument then becomes a dictionary key, not +a reflection subject. + +Sketch (naming tbd): + +```csharp +// global: these types participate in Type-based Dapper calls anywhere in the assembly +[module: DapperTypes(typeof(Foo), typeof(Bar), typeof(Blah))] + +// or local: scoped to a method/type, for call-site-adjacent declarations +[DapperTypes(typeof(Foo))] +public IEnumerable GetThings(Type type) => + connection.Query(type, "select ..."); +``` + +Design points to settle: + +- **miss behavior**: a `Type` not in the announced set must throw a clear runtime error + ("type X was not announced; add [DapperTypes]"), never fall back to reflection — same + principle as protobuf-net's "no serializer" backstop: incomplete model, loud failure. +- **granularity**: module-level is the easy 90%; per-call-site scoping only matters if + distinct call sites need distinct column-binding for the same type (probably: not v1). +- **reuse**: the per-type row-readers already exist for the generic path (`RowFactory`); + the announcement only adds the dispatch map, so the marginal cost per announced type is + small. +- **analyzer support**: a `Type`-based call with no announcement in scope should get a + diagnostic + code fix that scaffolds the attribute — the `PBN3010`/lightbulb pattern. + When the argument is a `typeof(Foo)` literal, the fix (or the generator itself) can + resolve it directly with no announcement needed; that sub-case is statically knowable + and should just work. +- **multi-map `Type[]`**: with announced types, the object[]-map overload becomes emittable + (the map function is user code taking `object[]`; we only need per-type readers + splitOn + handling). It rides on the multi-map work, not on anything type-specific. +- **`GetRowParser(reader, Type)`**: the discriminator pattern is the one place the *runtime* + choice is the whole point; announced types make it a supported dictionary lookup. This + should be a headline scenario for the feature, with an example in docs. +- **serialization-adjacent traps**: announced types need the same closure rules as generic + usage (constructor selection, member binding) — reuse the existing machinery, don't fork + it. + +## Relationship to `dynamic` + +`Query` (non-generic, no `Type` argument → `dynamic` rows) is already generated and does +**not** need announcements — the row shape comes from the resultset, not from a type. Keep +these distinct in docs; people conflate "non-generic" with "dynamic". From f00bdbdd723f6f0d34082a30a8addfbade3810bb Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 08:20:06 +0100 Subject: [PATCH 02/41] GetTypeDeserializer is valid API, not cache plumbing With announced types it's the same dispatch map (boxed materializer), and its generic strengthening already exists as GetRowParser. The real hole is the write side: CreateParamInfoGenerator has no generic counterpart - recorded the GetParameterBinder proposal, and the question of blessing CommandFactory/RowFactory as the supported surface. ReadChar and friends are plain AOT-safe statics, nothing to do. --- notes/parity.md | 4 +++- notes/type-vs-generic.md | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/notes/parity.md b/notes/parity.md index 318befc7..778428f9 100644 --- a/notes/parity.md +++ b/notes/parity.md @@ -46,7 +46,9 @@ Two levers change several complexity scores and are worth naming up front: | `Parse` / `Parse(Type)` / `Parse` (dynamic) | ❌ ❓ | low | low | same reader machinery, different entry point | | `AsTableValuedParameter` (`DataTable` / `SqlDataRecord`) | ❓ | med | med | SQL Server crowd; pairs with `ICustomQueryParameter` | | `AsList` | n/a | — | — | trivial helper; confirm it doesn't count as a candidate site | -| `GetTypeDeserializer` / `CreateParamInfoGenerator` / `ReadChar` etc | 🚫 | **zero** | — | reflection-era machinery; no meaning under AOT | +| `GetTypeDeserializer(Type, reader, startBound, length, ...)` | ❌ | low-med | low* | a valid raw-materializer API, not mere plumbing: with announced types it's the same dispatch map, returning a boxed `Func`. Its generic strengthening **already exists**: `GetRowParser` (same slicing knobs), which AOT supports | +| `CreateParamInfoGenerator(Identity, ...)` | ❌ | low | med | the raw parameter-binder factory; **no generic counterpart exists in Dapper** — see "Strengthened APIs" in [type-vs-generic.md](type-vs-generic.md) for the proposed `` form | +| `ReadChar` / `ReadNullableChar` / `SanitizeParameterValue` | ✅ | — | — | plain static helpers, AOT-safe as-is; nothing to intercept | | `PurgeQueryCache` / `GetCachedSQL*` / `GetHashCollissions` / `QueryCachePurged` | 🚫 | **zero** | — | there is no ref-emit plan cache in AOT | | `Format` / `ReplaceLiterals` | ❓ | low | low | falls out of the literal-injection work (see [tokens.md](tokens.md)) | diff --git a/notes/type-vs-generic.md b/notes/type-vs-generic.md index 52ef9147..1ae5b191 100644 --- a/notes/type-vs-generic.md +++ b/notes/type-vs-generic.md @@ -61,6 +61,35 @@ Design points to settle: usage (constructor selection, member binding) — reuse the existing machinery, don't fork it. +## Strengthened APIs: generic counterparts for the raw surface + +Announced types make the `Type`-based APIs *work*, but the better long-term answer for the +common case is that people never touch `Type` at all: every raw API should have a generic +counterpart, so `Type` is only for genuinely-runtime flows (discriminators, plugin-ish code). +The inventory today is lopsided: + +| raw (`Type`-based) API | generic counterpart | status | +| --- | --- | --- | +| `Query(Type, ...)` etc | `Query` etc | exists, supported | +| `Parse(Type)` / `Parse` | `Parse` | exists | +| `GetTypeDeserializer(Type, reader, startBound, length, returnNullIfFirstMissing)` | `GetRowParser(reader, startIndex, length, returnNullIfFirstMissing)` | **exists** — same knobs, better shape; AOT supports it. `GetTypeDeserializer` survives as the boxed form via announced types | +| `CreateParamInfoGenerator(Identity, ...)` → `Action` | *(none)* | **missing** — the write-side hole | + +So the concrete "do we need a new generic API?" answer is: **yes, one — the parameter +binder.** Something like `GetParameterBinder(sql?)` → `Action` (name tbd; +the `sql` argument exists because binding is SQL-dependent — list expansion, literals, and +member filtering all key off the command text). Notably `Identity` — the awkward part of the +raw API's signature — exists to key the *runtime cache*, which is exactly the concept AOT +deletes; the strengthened API should not carry it. + +There is a second candidate answer already in the codebase: the Dapper.AOT runtime's own +`CommandFactory` / `RowFactory` *are* the strengthened pair, but the FAQ currently +disclaims them ("we might radically change that API at any time"). Part of unification is +deciding whether to **bless that surface as the supported, documented API** — in which case +the legacy raw APIs (`GetTypeDeserializer`, `CreateParamInfoGenerator`) become thin +announced-type shims over it, and the new generic API lands in Dapper (or Dapper.AOT) as its +public face. + ## Relationship to `dynamic` `Query` (non-generic, no `Type` argument → `dynamic` rows) is already generated and does From 050d63c3b9bf720c6d32db796fae85f20c513c33 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 08:21:49 +0100 Subject: [PATCH 03/41] Scope the accounting to the public API and observable behavior PublicAPI.Shipped.txt is the checklist; the contract is what reaches the provider and what comes back, never Dapper's internals. Cuts both ways: the dynamic row needs behavioral fidelity only (the type is internal), while the public infrastructure statics ARE in scope because extenders call them. --- notes/README.md | 14 ++++++++++++++ notes/parity.md | 3 ++- notes/tokens.md | 8 +++++--- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/notes/README.md b/notes/README.md index 695d679c..1983c1e1 100644 --- a/notes/README.md +++ b/notes/README.md @@ -28,6 +28,20 @@ build time. See [type-vs-generic.md](type-vs-generic.md). | [type-vs-generic.md](type-vs-generic.md) | `Type`-based vs `` APIs, and the "announce your types" design space | | [test-suite-audit.md](test-suite-audit.md) | the Dapper test files as acceptance corpus, and what blocks each | +## Scope: the public API, by observable behavior + +Parity is defined over Dapper's **public API** (`Dapper/PublicAPI.Shipped.txt` is the +checklist) and the **observable behavior** of each member: the SQL text and parameter set +that reach the provider, and the values that come back. How Dapper implements any of it — +internal types, regexes, caches — is irrelevant, except as *evidence* of the observable +behavior. Two consequences: + +- internal implementation types (e.g. the dynamic-row class) need behavioral fidelity (the + returned object's public contracts), never type fidelity; +- public-but-infrastructure members (`PackListParameters`, `FindOrAddParameter`, + `TypeHandlerCache`, ...) are in scope *because they are public* — Contrib-style + extenders call them — even though they exist to serve Dapper's own generated IL. + ## Honesty rules - A status in these tables is only worth having if it was **verified against code** — the diff --git a/notes/parity.md b/notes/parity.md index 778428f9..33c44209 100644 --- a/notes/parity.md +++ b/notes/parity.md @@ -51,6 +51,7 @@ Two levers change several complexity scores and are worth naming up front: | `ReadChar` / `ReadNullableChar` / `SanitizeParameterValue` | ✅ | — | — | plain static helpers, AOT-safe as-is; nothing to intercept | | `PurgeQueryCache` / `GetCachedSQL*` / `GetHashCollissions` / `QueryCachePurged` | 🚫 | **zero** | — | there is no ref-emit plan cache in AOT | | `Format` / `ReplaceLiterals` | ❓ | low | low | falls out of the literal-injection work (see [tokens.md](tokens.md)) | +| public infrastructure statics: `PackListParameters`, `FindOrAddParameter`, `LookupDbType`, `HasTypeHandler`, `GetTypeName`/`SetTypeName`, `SetDbType`, `TypeHandlerCache.Parse/SetValue`, `ThrowDataException`, `ThrowNullCustomQueryParameter` | ❓ | low | low | in scope because they are public (Contrib-style extenders call them), even though they exist to serve Dapper's generated IL. Mostly plain AOT-safe statics; the `Type`-keyed ones (`LookupDbType`, `TypeHandlerCache`) fold into announced types / the type-handler story | ## 2. Parameters (input side) @@ -84,7 +85,7 @@ Two levers change several complexity scores and are worth naming up front: | constructor binding, `[ExplicitConstructor]` | ✅ | — | — | plus factory methods (AOT extension) | | `required` / init-only members | ✅ | — | — | `RequiredProperties` fixture | | fields | ❓ | low | low | verify | -| `dynamic` rows (`DapperRow` fidelity) | ⚠️ | high | med | Dapper's row is `IDictionary` + `IReadOnlyDictionary`, tolerates add/remove, specific null/missing semantics — pin the matrix with tests | +| `dynamic` rows — behavioral fidelity | ⚠️ | high | med | the row *type* is internal and irrelevant; the observable contract is: `dynamic` member access, `IDictionary` + `IReadOnlyDictionary`, tolerates add/remove, specific null/missing semantics — pin that matrix with tests against AOT's row | | tuple results | ❌ | med | med | DAP013; design already framed by `[BindTupleByName]` | | enum results (string→enum case-insens., widening, `ShortEnum`) | ⚠️❓ | high | low-med | Dapper recently changed precedence (prefer type handlers, #2200) — match the *new* behavior | | `MatchNamesWithUnderscores` | ❓ | med-high | low | snake_case databases; needs a compile-time equivalent (global option/attr) | diff --git a/notes/tokens.md b/notes/tokens.md index d31f3438..1a949335 100644 --- a/notes/tokens.md +++ b/notes/tokens.md @@ -5,9 +5,11 @@ Every one of these is observable behavior that real code depends on; each needs AOT position: **replicate**, **replicate with constraints**, or **refuse loudly** (analyzer error, never silent divergence). -Source of truth: `Dapper/Dapper/SqlMapper.cs` (`PackListParameters`, `GetInListRegex`, -`GetLiteralTokens`, `ReplaceLiterals`, `SanitizeParameterValue`, the pseudo-positional -rewrite) and `SqlMapper.Settings`. Line refs are as of `72a54c4`. +The contract here is **observable behavior only**: the SQL text and parameter set that reach +the provider. The regexes and rewrite internals cited below (from `Dapper/Dapper/SqlMapper.cs` +— `PackListParameters`, `GetInListRegex`, `GetLiteralTokens`, `SanitizeParameterValue`, the +pseudo-positional rewrite — line refs as of `72a54c4`) are *evidence* of that behavior, not +something to replicate structurally. AOT is free to implement all of this however it likes. ## 1. Parameter prefixes From 570fcdf570f9401b167f2909bc301bccdd03fdc0 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 08:24:19 +0100 Subject: [PATCH 04/41] Record the decision: internals-asserting tests get adjusted, not maintained --- notes/test-suite-audit.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/notes/test-suite-audit.md b/notes/test-suite-audit.md index b4fd747b..9236aeaf 100644 --- a/notes/test-suite-audit.md +++ b/notes/test-suite-audit.md @@ -17,8 +17,11 @@ ideally AOT-publishable without warnings. drive to 100% is that ratio (per test assembly), then the test pass rate, then the AOT publish warning count — in that order. A passing test with an un-intercepted call site is measuring vanilla Dapper, not us. -- Some tests assert *Dapper internals* (cache counts, `Identity`, deserializer internals); - those need triage into "port", "skip under AOT", or "equivalent AOT assertion". +- **Decided:** tests that assert *Dapper internals* (cache counts, `Identity`, deserializer + internals) are expected to be **adjusted, not maintained** — those side-effect numbers are + not part of the contract, and several describe machinery AOT doesn't have. Per test: skip + under AOT, or replace with an assertion about the observable behavior it was standing in + for. "100% of the corpus" means 100% of the *behavioral* corpus. ## Per-file first-pass audit From 36d7942b2f9e07e2cf9ffa4f43a19f4c0c717f60 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 08:25:50 +0100 Subject: [PATCH 05/41] New work item: warn (new DAP id) on use of the has-no-meaning APIs Plan-cache surface, CommandFlags.NoCache, possibly ConnectionStringComparer: supported-and-meaningless under AOT, which is a different statement to DAP001's unsupported-but-meaningful. Warning, not error - the code runs. --- notes/parity.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/notes/parity.md b/notes/parity.md index 33c44209..52c830b2 100644 --- a/notes/parity.md +++ b/notes/parity.md @@ -49,7 +49,7 @@ Two levers change several complexity scores and are worth naming up front: | `GetTypeDeserializer(Type, reader, startBound, length, ...)` | ❌ | low-med | low* | a valid raw-materializer API, not mere plumbing: with announced types it's the same dispatch map, returning a boxed `Func`. Its generic strengthening **already exists**: `GetRowParser` (same slicing knobs), which AOT supports | | `CreateParamInfoGenerator(Identity, ...)` | ❌ | low | med | the raw parameter-binder factory; **no generic counterpart exists in Dapper** — see "Strengthened APIs" in [type-vs-generic.md](type-vs-generic.md) for the proposed `` form | | `ReadChar` / `ReadNullableChar` / `SanitizeParameterValue` | ✅ | — | — | plain static helpers, AOT-safe as-is; nothing to intercept | -| `PurgeQueryCache` / `GetCachedSQL*` / `GetHashCollissions` / `QueryCachePurged` | 🚫 | **zero** | — | there is no ref-emit plan cache in AOT | +| `PurgeQueryCache` / `GetCachedSQL*` / `GetHashCollissions` / `QueryCachePurged` | 🚫 | **zero** | — | there is no ref-emit plan cache in AOT — but usage should *warn*, see §7 | | `Format` / `ReplaceLiterals` | ❓ | low | low | falls out of the literal-injection work (see [tokens.md](tokens.md)) | | public infrastructure statics: `PackListParameters`, `FindOrAddParameter`, `LookupDbType`, `HasTypeHandler`, `GetTypeName`/`SetTypeName`, `SetDbType`, `TypeHandlerCache.Parse/SetValue`, `ThrowDataException`, `ThrowNullCustomQueryParameter` | ❓ | low | low | in scope because they are public (Contrib-style extenders call them), even though they exist to serve Dapper's generated IL. Mostly plain AOT-safe statics; the `Type`-keyed ones (`LookupDbType`, `TypeHandlerCache`) fold into announced types / the type-handler story | @@ -129,3 +129,15 @@ Recorded so the unified story stays a superset, not a port: batch execution (`Db `[CacheCommand]`, `[CommandProperty]` (provider-specific command props), `[RowCount]` / `[RowCountHint]`, `TypeAccessor` + `SqlBulkCopy` bridge, `[IncludeLocation]`, deep TSQL analysis (DAP2xx), `[SqlSyntax]`. + +## 7. Work items arising + +- **New diagnostic: warn on "has no meaning" APIs.** When AOT is enabled, detect usage of + the APIs whose *concept* doesn't exist under AOT — the plan-cache surface + (`PurgeQueryCache`, `GetCachedSQL`, `GetCachedSQLCount`, `GetHashCollissions`, + `QueryCachePurged`), `CommandFlags.NoCache`, and (if confirmed zero) + `SqlMapper.ConnectionStringComparer` — and emit a **warning**: the call is harmless but + inert, and its presence usually signals code written to manage a runtime that is no longer + there. Not an error: the code still runs. Next free id in the library block is DAP050 + (DAP049 is the highest taken). Distinct from DAP001 (unsupported-but-meaningful): this is + *supported-and-meaningless*. From 728a3bedfa6413cbc7953cb3abab9f26135990d3 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 08:29:54 +0100 Subject: [PATCH 06/41] Measurement caveat: build-time DAP counts are an upper bound Some failure modes are silent until executed - handled means intercepted, not correct. Only the DB-backed test run catches silent divergence. --- notes/test-suite-audit.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/notes/test-suite-audit.md b/notes/test-suite-audit.md index 9236aeaf..121d9b5f 100644 --- a/notes/test-suite-audit.md +++ b/notes/test-suite-audit.md @@ -17,6 +17,13 @@ ideally AOT-publishable without warnings. drive to 100% is that ratio (per test assembly), then the test pass rate, then the AOT publish warning count — in that order. A passing test with an un-intercepted call site is measuring vanilla Dapper, not us. +- **The DAP list is useful but incomplete: some failure modes are silent until executed.** + "Handled" at build time means an interceptor was emitted, not that it behaves like Dapper — + a generated call can bind the wrong member, format a value differently, or fail only on a + particular data shape, with no diagnostic anywhere. So the build-time ratio is an *upper + bound*; only the test run catches silent divergence, which is why the DB-backed run is part + of the measurement and not an optional extra. (This is the same lesson as protobuf-net's + AOT differential: every serious generator bug there compiled cleanly and wrote wrong bytes.) - **Decided:** tests that assert *Dapper internals* (cache counts, `Identity`, deserializer internals) are expected to be **adjusted, not maintained** — those side-effect numbers are not part of the contract, and several describe machinery AOT doesn't have. Per test: skip From ee006d91a2267659c42f4a2a61c1dfc54a735193 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 08:33:58 +0100 Subject: [PATCH 07/41] First harness baseline: 'handled 396 of 396' alongside 96 compile errors Two root-cause generator bugs (array-of-anonymous parameter emits the display string and wrecks the parse; inaccessible row types are emitted rather than refused), plus two scorecard honesty problems (the denominator excludes unattempted APIs; handled does not mean compiles) and a zero- analyzer-diagnostics anomaly to re-check once the compile is clean. --- notes/README.md | 1 + notes/harness-baseline.md | 72 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 notes/harness-baseline.md diff --git a/notes/README.md b/notes/README.md index 1983c1e1..36cf1f24 100644 --- a/notes/README.md +++ b/notes/README.md @@ -27,6 +27,7 @@ build time. See [type-vs-generic.md](type-vs-generic.md). | [tokens.md](tokens.md) | special string-token handling: `@ids` expansion, `{=literal}`, `?foo?`, etc | | [type-vs-generic.md](type-vs-generic.md) | `Type`-based vs `` APIs, and the "announce your types" design space | | [test-suite-audit.md](test-suite-audit.md) | the Dapper test files as acceptance corpus, and what blocks each | +| [harness-baseline.md](harness-baseline.md) | real numbers from the suite with AOT enabled (Dapper repo, `aot-harness` branch) | ## Scope: the public API, by observable behavior diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md new file mode 100644 index 00000000..cfa7a913 --- /dev/null +++ b/notes/harness-baseline.md @@ -0,0 +1,72 @@ +# Harness baseline: Dapper.AOT enabled in the Dapper test suite + +First real numbers, 2026-08-18. Setup lives on the `aot-harness` branch of the **Dapper** +repo (sibling checkout): local package feed at `../DapperAOT/artifacts` (pack with +`NBGV_GitEngine=Disabled dotnet pack src/Dapper.AOT/Dapper.AOT.csproj -c Release -o artifacts`, +giving the stable harness version `1.0.0-g`; purge `~/.nuget/packages/dapper.aot/1.0.0-g` +between repacks), `[module: DapperAot]` in `DapperAotEnable.cs`, interceptors enabled, and a +`.globalconfig` raising DAP000 (Hidden by default) to warning. + +Build: `dotnet build tests/Dapper.Tests/Dapper.Tests.csproj -f net10.0` (net481 and net8.0 +legs not yet measured). + +## The headline, and why it is wrong + +> `DAP000: Dapper.AOT handled 396 of 396 possible call-sites using 143 interceptors, 61 +> commands and 15 readers` + +…reported alongside **~96 compile errors, all in the generated file**. Two separate honesty +problems, both now work items: + +1. **The denominator excludes what was never attempted.** The suite plainly uses + `QueryMultiple`, multi-map, `ExecuteReader`, … (whole test files of them) — "396 of 396" + counts only call-sites the generator considers candidates, so the unsupported surface + vanishes from the score. The scorecard should count *every* Dapper call-site, splitting + into handled / unsupported (with ids) / failed. +2. **"Handled" does not even mean "compiles"**, let alone "behaves like Dapper" — the + emitted code failed to build. (And behavioral divergence is silent until executed — see + the measurement caveat in [test-suite-audit.md](test-suite-audit.md).) + +## Error classes → root causes + +Raw tally (log lines; MSBuild double-lists, so ratios matter more than counts): CS0122 ×76, +CS1527 ×70, CS0708 ×24, CS0246 ×12, CS0102 ×6, CS0115 ×2, CS0052 ×2. + +Actual distinct bugs: **two**. + +- **Bug A — array-of-anonymous-type parameter wrecks the file.** `MiscTests.cs:1217`-ish: + a parameter object with a member `MyModels = new[] { new { XXXId = 1, ... }, ... }`. The + unary anonymous-type path correctly emits `CommandFactory` + a + `Cast(args, static () => new { ... })` shape-witness (visible in the generated + `CommandFactory26`); the enumerable path instead renders the *display string* into source: + `CommandFactory[]>` — which is not C#. The `<`/`>` + in that string break the parse tree, so every subsequent class in the file lands at + namespace scope: the CS1527/CS0708/CS0102/CS0115/CS0246/CS0052 flood (and the CS0122s + naming `CommandFactoryNN`) are **all cascade from this one bug**. It also emits + `args.XXXId` where `args` is the array — the body is wrong as well as unnameable. +- **Bug B — inaccessible types are referenced instead of refused.** `PostgresqlTests.Cat` is + a `private` nested class used as a row type; the generator emitted + `RowFactory<...PostgresqlTests.Cat>` anyway → genuine CS0122. DAP017 ("non-accessible + type … not currently supported") exists for exactly this, but the *generator* doesn't + enforce it — it should drop the call-site (leave it on vanilla Dapper) with the diagnostic, + never emit code that cannot compile. Same principle as protobuf-net's PBN3002: refuse + beats emitting a build break in a file the consumer never wrote. + +## Anomaly: zero analyzer diagnostics + +The build shows **no** DAP diagnostics at all besides DAP000 — no DAP001 (unsupported +method: the suite is full of them), no DAP2xx SQL analysis, and no AD0001 (so the analyzer +didn't crash). Hypothesis: the broken generated code poisons binding, and the analyzer's +operation callbacks bail on error symbols. Re-check once Bug A is fixed; if the silence +persists with a clean compile, that's its own bug. + +## Scoreboard (to update as things land) + +| leg | possible (honest) | handled | compiles | tests green | AOT publish | +| --- | --- | --- | --- | --- | --- | +| net10.0 | > 396 (denominator understated) | 396 claimed | ❌ (2 root-cause bugs) | — | — | +| net8.0 | not yet measured | | | | | +| net481 | not yet measured | | | | | + +Next: fix Bug A and Bug B in the generator (both look small), rebuild, re-tally — expect the +DAP001/DAP2xx picture to appear once the compile is clean, which gives the real gap list. From 8c41bf0fc26b46ab46ef7eab9f0abe9217738aae Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 08:37:42 +0100 Subject: [PATCH 08/41] Generator audit: the capture model snapshots Roslyn nodes; fix first Both generators' cached SourceState hold IMethodSymbol/ITypeSymbol/ Location (MemberMap even holds an IOperation), and the pipeline combines the raw CompilationProvider into the source output - so it behaves as a full-recompute generator with a memory leak. Recorded as a sequencing gate ahead of the gap-closing features, with the fix shape that worked for protobuf-net (plain equatable model, span-based locations, separate diagnostics branch, shape-enforcing test). --- notes/README.md | 1 + notes/generator-audit.md | 72 +++++++++++++++++++++++++++++++++++++++ notes/test-suite-audit.md | 4 +++ 3 files changed, 77 insertions(+) create mode 100644 notes/generator-audit.md diff --git a/notes/README.md b/notes/README.md index 36cf1f24..4f9521d6 100644 --- a/notes/README.md +++ b/notes/README.md @@ -28,6 +28,7 @@ build time. See [type-vs-generic.md](type-vs-generic.md). | [type-vs-generic.md](type-vs-generic.md) | `Type`-based vs `` APIs, and the "announce your types" design space | | [test-suite-audit.md](test-suite-audit.md) | the Dapper test files as acceptance corpus, and what blocks each | | [harness-baseline.md](harness-baseline.md) | real numbers from the suite with AOT enabled (Dapper repo, `aot-harness` branch) | +| [generator-audit.md](generator-audit.md) | **fix-first gate**: the capture model snapshots Roslyn symbols/nodes — retention + cache defeat | ## Scope: the public API, by observable behavior diff --git a/notes/generator-audit.md b/notes/generator-audit.md new file mode 100644 index 00000000..0be21ba4 --- /dev/null +++ b/notes/generator-audit.md @@ -0,0 +1,72 @@ +# Generator audit: the capture model snapshots Roslyn nodes + +**Work item, and a sequencing gate: fix this before building new generator features**, so the +multi-map / DynamicParameters / QueryMultiple work is written against the fixed model rather +than needing a second pass over brand-new code. + +## The rule being violated + +An incremental generator's cached pipeline values must be **plain, equatable data**. Holding +`ISymbol` / `SyntaxNode` / `IOperation` / `Location` / `Compilation` in them causes two silent +failures at once: + +1. **retention** — a symbol pins its entire `Compilation` (and transitively the syntax trees) + alive for as long as the driver caches the value: in a long-running IDE session, that is + whole Roslyn trees held non-collectible; +2. **cache defeat** — symbol/location equality does not hold across compilations, so the + incremental cache never hits anyway; you pay the leak *and* get none of the benefit. + +## Findings (2026-08-18, `main` @ `e3b1037`) + +Verified by reading the code, not assumed: + +- **`DapperInterceptorGenerator.SuccessSourceState`** (`DapperInterceptorGenerator.cs:1505`) + holds `IMethodSymbol Method`, `ITypeSymbol? ResultType`, `ITypeSymbol? ParameterType`, and + `Location` — this is the cached per-call-site value (`CreateSyntaxProvider(PreFilter, Parse)`). + `CommonComparer` groups with `SymbolEqualityComparer`, which also does not hold across + compilations. +- **`TypeAccessorInterceptorGenerator.SourceState`** (`TypeAccessorInterceptorGenerator.cs:184`) + same shape: `Location` + `ITypeSymbol` + `IMethodSymbol`. +- **`MemberMap`** (`Internal/MemberMap.cs`) holds `ITypeSymbol`s, `IMethodSymbol`s, a + `Location`, and an **`IOperation`** — and is reachable from `AdditionalCommandState`, which + rides inside `SuccessSourceState`. Audit everything transitively reachable from the cached + states; a single symbol anywhere in the graph is enough to pin the compilation. +- **The pipeline combines the raw compilation**: `DapperInterceptorGenerator.cs:58-61` — + `context.CompilationProvider.Combine(nodes.Collect())` feeding `RegisterSourceOutput`. Even + with a clean node model, this re-runs the generate step on *every* compilation change + (every keystroke); the compilation must not be an input to the output step. (This is the + documented Roslyn anti-pattern; RS1041-family guidance.) +- `FaultSourceState` holds `Exception` + `Location` — same treatment (message/type name + + span data). + +Net: the generator currently behaves as a **full-recompute generator with a memory leak**, +not an incremental one. Correctness is unaffected — which is why nothing ever flagged it. + +## The fix shape (proven in protobuf-net's AOT generator) + +This exact trap sank earlier attempts at protobuf-net's generator; the working pattern there: + +- model types are hand-written **equatable plain-data values** — strings, enums, packed + flags, hand-rolled `EquatableArray` (note `ImmutableArray` equality is + reference-based and silently defeats caching too — don't swap one trap for another); +- locations are stored as Roslyn **value** types (`TextSpan` + `LinePositionSpan` + path — + plain data), reconstituted into a `Location` only at report/emit time (`PlanLocation`); +- symbols are fully **projected during parse**: everything emit needs (qualified type names, + member lists, flags) is extracted into the model while the `SemanticModel` is in hand; +- diagnostics ride a **separate** pipeline branch from the emit model, because they carry + locations that shift with every edit and the emit model should not; +- a **shape-enforcing test** walks the model types by reflection and fails on any field of a + Roslyn reference type (`ProtoModelPlanShapeTests` pattern) — the constraint has to have + teeth or it erodes; it eroding silently is precisely how it got here. + +The interceptor output itself needs `InterceptsLocation` data — that is file/position values, +not `Location` objects, so it survives projection fine. + +## Why fix-first is the right order + +Every gap-closing feature in [parity.md](parity.md) §1–§3 adds parse-time state (multi-map +adds per-type splits, DynamicParameters adds member graphs, announced types add a type +inventory). Built on the current model, each addition deepens the symbol snapshot and has to +be re-done when the model is fixed. Built after the fix, each lands on plain data from day +one. The harness ([harness-baseline.md](harness-baseline.md)) is unaffected — it measures +behavior, not model shape — so the two tracks can run in parallel. diff --git a/notes/test-suite-audit.md b/notes/test-suite-audit.md index 121d9b5f..e8a4115e 100644 --- a/notes/test-suite-audit.md +++ b/notes/test-suite-audit.md @@ -66,6 +66,10 @@ table was drawn from file names and Dapper source knowledge, not yet from a run. ## Suggested sequencing (by corpus unblocked per unit of work) +0. **Fix the generator capture model first** ([generator-audit.md](generator-audit.md)): the + cached pipeline values hold Roslyn symbols/nodes (retention + cache defeat). Everything + below adds parse-time state; build it on the fixed model, not twice. + 1. **DynamicParameters** — unblocks `ParameterTests`, `ProcedureTests`, `SqlBuilderTests`; biggest single win and forces the output-parameter design. 2. **List expansion + literals + pseudo-positional** (one work item: the SQL-rewrite runtime From b77dd8368c3470faff317187828852faededf1c0 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 08:40:39 +0100 Subject: [PATCH 09/41] Record the agreed plan: gap table, then generator model, then features The line that resolves the phase-1/2 tension: nothing that adds parse-time state lands before the model rework completes; refusals and scorecard fixes are allowed ahead of it, which is what lets phase 1 see. --- notes/README.md | 1 + notes/plan.md | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 notes/plan.md diff --git a/notes/README.md b/notes/README.md index 4f9521d6..8e7edea7 100644 --- a/notes/README.md +++ b/notes/README.md @@ -23,6 +23,7 @@ build time. See [type-vs-generic.md](type-vs-generic.md). | doc | contents | | --- | --- | +| [plan.md](plan.md) | **the agreed plan**: complete the gap table → fix the generator → close the gaps | | [parity.md](parity.md) | the feature parity table: Dapper's surface vs Dapper.AOT today | | [tokens.md](tokens.md) | special string-token handling: `@ids` expansion, `{=literal}`, `?foo?`, etc | | [type-vs-generic.md](type-vs-generic.md) | `Type`-based vs `` APIs, and the "announce your types" design space | diff --git a/notes/plan.md b/notes/plan.md new file mode 100644 index 00000000..3e29058a --- /dev/null +++ b/notes/plan.md @@ -0,0 +1,52 @@ +# The plan + +Agreed 2026-08-18. Three phases, in order. The rule that resolves the phase-1/phase-2 +tension: **nothing that adds parse-time state lands before phase 2 completes** — refusals +and scorecard changes are fine, features are not. + +## Phase 1 — complete the gap table (empirically, not by reading) + +The parity table's remaining ❓s are resolved by the harness, which is currently blind. So +phase 1 includes the minimum generator work needed to make the instrument read: + +- fix **Bug A** (array-of-anonymous parameter emits the display string and wrecks the parse) + and **Bug B** (inaccessible types emitted rather than refused with DAP017) — see + [harness-baseline.md](harness-baseline.md). Both are parse/refusal decisions that survive + the phase-2 model rework, which is why they are allowed ahead of it; +- fix the **DAP000 scorecard honesty**: count *every* Dapper call-site, split + handled / unsupported (with ids) / failed. That scorecard is the gap table's data source, + so it is phase-1 work, not phase-3; +- re-run the harness, confirm the analyzer diagnostics appear once the compile is clean + (the zero-DAP001 anomaly), harvest the flood, and replace the guessed rows in + [test-suite-audit.md](test-suite-audit.md) and the ❓s in [parity.md](parity.md) with + measured ones; +- in parallel: stand up SQL Server (docker or Developer Edition — both fine on this + machine) and get the **vanilla** suite green as the control, so the behavioral instrument + exists before phase 3 needs it. + +Exit criteria: suite compiles with AOT on; honest scorecard numbers for all three TFM legs; +parity table has no ❓ that the harness could have answered. + +## Phase 2 — fix the generator capture model + +[generator-audit.md](generator-audit.md) is the spec. Requirement: **byte-identical +generated output before/after**, checked two ways for free — the interceptor golden +fixtures (`*.output.cs`), and a diff of the harness's emitted `Dapper.Tests.generated.cs`. + +The same change adds the **shape-enforcing reflection test** (no Roslyn reference types in +the cached model, Roslyn value types allowed) — the constraint has to have teeth in CI or +it erodes again; the test landing with the fix is what made the rule stick in protobuf-net. + +Exit criteria: model is plain equatable data; `CompilationProvider` no longer feeds the +output step; shape test in CI; output byte-identical. + +## Phase 3 — close the gaps, in the sequenced order + +Order as drafted in [test-suite-audit.md](test-suite-audit.md): DynamicParameters → token +rewrites (list expansion / literals / pseudo-positional) → QueryMultiple/GridReader → +multi-map → announced types → type handlers → the tail. Revisit the order once phase 1's +measured numbers land — it was drafted from estimated impact. + +**Definition of done per feature**: compiles + intercepted + **DB-backed tests green**, not +just build-time clean — the test run is the only net that catches silent divergence (see +the measurement caveat in [test-suite-audit.md](test-suite-audit.md)). From 62720a11cd925ffbf8d146718cf579f8d5e60ce4 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 08:57:44 +0100 Subject: [PATCH 10/41] Round 2 numbers, and log the modern-interceptor-syntax work item --- notes/harness-baseline.md | 32 ++++++++++++++++++++++++++++++-- notes/parity.md | 10 ++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index cfa7a913..03909f61 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -68,5 +68,33 @@ persists with a clean compile, that's its own bug. | net8.0 | not yet measured | | | | | | net481 | not yet measured | | | | | -Next: fix Bug A and Bug B in the generator (both look small), rebuild, re-tally — expect the -DAP001/DAP2xx picture to appear once the compile is clean, which gives the real gap list. +## Round 2 (same day): both bugs fixed (PRs #180, #181), repack, re-measure + +Predictions held: the zero-analyzer-diagnostics anomaly was cascade from Bug A — with the +parse wreck gone the full picture appeared, and the scorecard moved honestly (396 → **394 of +394**: the refused sites left the denominator too, which is the dishonesty already on +record). + +Warnings: DAP027 ×402, DAP028 ×44, DAP012 ×40 (tuple-name guidance), DAP018 ×26 (params not +detected in SQL), DAP048 ×16 (DbString→DbValue). Remaining build breaks, i.e. the next +bug/gap batch: + +- **types nested in generic classes leak `TProvider`** (CS0246 ×16 + CS0122 ×8): + `ParameterTests.IntCustomParam` renders with the open type parameter into the + generated (non-generic) scope. `InvolvesGenericTypeParameter` has the same blind spot + `IsPublicOrAssemblyLocal` had (PR #180): it walks containment but not type arguments. + Same fix shape; note the accessibility check *also* missed these (private nested), so + check ordering/coverage while there; +- **CS1503 ×2**: generated code passes `IDataReader` where `DbDataReader` is required + (`GetRowParser`-adjacent, generated line 497) — emit bug, uninvestigated; +- **DAP037 as a build error on scalar-ish results** (×16): `Query`, `Query`, + `Query`, `Query` (renders as `''`), `Query` — all fine in + vanilla Dapper, all "no settable members" *errors* here. The generator lacks scalar + result-type handling for enums/char/arrays and mishandles `dynamic` as a generic argument; + an analyzer **error** on a vanilla-supported shape also breaks the build outright, which + is worth revisiting as a severity question independent of the feature gap; +- **DAP036 as a build error** (×4): `Query`, `Query` — BCL structs as + results hit the constructor-ambiguity error. Same family as above. + +The scalar-result gaps are phase-3 features (they add parse-time state); the `TProvider` +leak is a phase-1 refusal fix; the CS1503 needs sizing. diff --git a/notes/parity.md b/notes/parity.md index 52c830b2..d71f680c 100644 --- a/notes/parity.md +++ b/notes/parity.md @@ -132,6 +132,16 @@ analysis (DAP2xx), `[SqlSyntax]`. ## 7. Work items arising +- **Migrate to the modern interceptor syntax.** The generator emits the legacy + `[InterceptsLocation(path, line, column)]` form, deprecated in current SDKs (`CS9270` — the + generated header even carries a pragma for it, and the `SqliteUsage` snapshot warns today); + the replacement is the `InterceptableLocation`-based version+data form. **A working + implementation that does not need an SDK/Roslyn bump exists in protobuf-net** (just done): + `GrpcProxyGenerator` obtains the location payload by reflecting into the host's + `GetInterceptableLocation` (Roslyn 4.11+) so the shipped baseline can stay low, and + `docs/aot-grpc.md` there records the encoding — reverse-engineered and proven by hand — as + the fallback if the reflection ever stops working. Port that approach. + - **New diagnostic: warn on "has no meaning" APIs.** When AOT is enabled, detect usage of the APIs whose *concept* doesn't exist under AOT — the plan-cache surface (`PurgeQueryCache`, `GetCachedSQL`, `GetCachedSQLCount`, `GetHashCollissions`, From 01ed04b28383aa6cfb5cf76183ab311d99384740 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 09:08:01 +0100 Subject: [PATCH 11/41] Round 3: the suite compiles with AOT enabled (4 fix PRs + 2 severity downgrades) --- notes/harness-baseline.md | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index 03909f61..dd9ae433 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -60,11 +60,27 @@ didn't crash). Hypothesis: the broken generated code poisons binding, and the an operation callbacks bail on error symbols. Re-check once Bug A is fixed; if the silence persists with a clean compile, that's its own bug. +## Round 3 (same day): four fixes in, the suite compiles + +With PRs #180 (generic args in accessibility), #181 (enumerable params outside Execute), +#182 (member types checked like the parameter type), #183 (GetRowParser over IDataReader), +plus a harness-side severity downgrade of DAP036/DAP037 (they fire as *refusals* on the +scalar-result feature gaps — enums, char, TimeSpan, arrays, dynamic — so for measurement +they are gap markers, not build breaks): + +> **exit 0.** `Dapper.AOT handled 387 of 387 possible call-sites using 137 interceptors, +> 55 commands and 15 readers` — and zero compiler errors. + +The 550 DAP warnings are now the measured gap surface (DAP027 ×402, DAP028 ×44, DAP012 ×40, +DAP018 ×26, DAP048 ×16, DAP036/037 ×20 as downgraded errors). Next instruments: the honest +scorecard (the 387 denominator still excludes never-attempted APIs), then actually *running* +the suite against a database, which is where silent divergence shows. + ## Scoreboard (to update as things land) | leg | possible (honest) | handled | compiles | tests green | AOT publish | | --- | --- | --- | --- | --- | --- | -| net10.0 | > 396 (denominator understated) | 396 claimed | ❌ (2 root-cause bugs) | — | — | +| net10.0 | > 387 (denominator understated) | 387 claimed | ✅ (with 4 fix-PRs + 2 severity downgrades) | — | — | | net8.0 | not yet measured | | | | | | net481 | not yet measured | | | | | From 468317bc0a2054df90d7ce35fcf91e5926292c9e Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 10:30:04 +0100 Subject: [PATCH 12/41] Scoreboard: all three TFM legs compile; local SQL Server available --- notes/harness-baseline.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index dd9ae433..7c92fb84 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -80,9 +80,16 @@ the suite against a database, which is where silent divergence shows. | leg | possible (honest) | handled | compiles | tests green | AOT publish | | --- | --- | --- | --- | --- | --- | -| net10.0 | > 387 (denominator understated) | 387 claimed | ✅ (with 4 fix-PRs + 2 severity downgrades) | — | — | -| net8.0 | not yet measured | | | | | -| net481 | not yet measured | | | | | +| net10.0 | > 387 (denominator understated) | 387 claimed | ✅ | — | — | +| net8.0 | > 387 | 387 claimed | ✅ | — | — | +| net481 | > 405 | 405 claimed | ✅ (needs PR #184) | — | — | + +All three legs compile as of 2026-08-18 evening, with PRs #180–#184 (the net481 leg — EF +spatial, Linq2Sql — was the finder for #184: `DbGeography`/`Binary` result types emitted +uncompilable construction; now refused with the new DAP050) plus the two harness severity +downgrades. Environment note: **SQL Server 2022 Developer is installed and running locally** +(default instance, matching the suite's default `Data Source=.` connection string), so the +vanilla control run needs no docker. ## Round 2 (same day): both bugs fixed (PRs #180, #181), repack, re-measure From e2e19d42a1c41415bc067ed1720e0f12c96f21a1 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 11:00:16 +0100 Subject: [PATCH 13/41] Work item: [UnsafeAccessor] may lift the accessibility refusals (net8+) --- notes/parity.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/notes/parity.md b/notes/parity.md index d71f680c..1845ea51 100644 --- a/notes/parity.md +++ b/notes/parity.md @@ -132,6 +132,20 @@ analysis (DAP2xx), `[SqlSyntax]`. ## 7. Work items arising +- **Investigate `[UnsafeAccessor]` (net8+) to lift the accessibility-class refusals.** The + protobuf-net AOT generator uses it extensively in generated code, gated on target framework: + it covers **construction** (`UnsafeAccessorKind.Constructor`), non-public **properties/setters**, + **fields**, and even get-only auto-properties (writing the compiler-generated backing field) — + and unlike reflection it is resolved at publish time, so it stays AOT-safe (protobuf-net proved + it under ILC, including `initonly` backing fields; struct targets take `ref`). Candidates here: + the DAP050 shapes where the *type* is accessible but its constructor is not (`DbGeography`'s + internal ctor is exactly this), non-public setters on result members, and get-only auto-props. + Two limits to respect, both learned in protobuf-net: the accessor's signature must still *name* + the target type, so non-public **types** (DAP017) stay refused regardless; and it is net8.0+, + so down-level targets keep the refusal path — protobuf-net's pattern is "smaller model with + warnings naming the fix, not a broken build" (`DownLevelSmoke`). Probe for the attribute + rather than assuming TFM. + - **Migrate to the modern interceptor syntax.** The generator emits the legacy `[InterceptsLocation(path, line, column)]` form, deprecated in current SDKs (`CS9270` — the generated header even carries a pragma for it, and the `SqliteUsage` snapshot warns today); From 423a07edb010023016540ec6864e53cf4a674db2 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 11:07:48 +0100 Subject: [PATCH 14/41] Round 4: the honest scorecard says 53%, not 100% --- notes/harness-baseline.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index 7c92fb84..59099aa4 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -76,6 +76,22 @@ DAP018 ×26, DAP048 ×16, DAP036/037 ×20 as downgraded errors). Next instrument scorecard (the 387 denominator still excludes never-attempted APIs), then actually *running* the suite against a database, which is where silent divergence shows. +## Round 4: the honest scorecard (PR #185) + +With DAP000 counting every enabled call-site (unsupported APIs and diagnostic-refused sites +included), the truth behind the former "100%": + +| leg | handled | of | ratio | unsupported API | skipped via diagnostics | +| --- | --- | --- | --- | --- | --- | +| net10.0 | 387 | 725 | **53.4%** | 82 | 256 | +| net8.0 | 387 | 725 | 53.4% | 82 | 256 | +| net481 | 405 | 757 | 53.5% | 84 | 268 | + +So the corpus number to drive to 100% starts at **53%**. The 82 unsupported-API sites are the +`QueryMultiple`/multi-map/`ExecuteReader` families (parity.md §1); the 256 diagnostic skips +are dominated by the DAP012/DAP015/DAP016/DAP017/DAP050 shapes plus untyped parameters — +harvesting that breakdown per-id into the audit table is the remaining phase-1 analysis step. + ## Scoreboard (to update as things land) | leg | possible (honest) | handled | compiles | tests green | AOT publish | From 17c080ec9d9b73f71fd64c96fccea4da8309c7bb Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 11:16:34 +0100 Subject: [PATCH 15/41] Harvest the skip breakdown; flag the DAP016 corpus-shape decision --- notes/harness-baseline.md | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index 59099aa4..d59f813e 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -87,10 +87,29 @@ included), the truth behind the former "100%": | net8.0 | 387 | 725 | 53.4% | 82 | 256 | | net481 | 405 | 757 | 53.5% | 84 | 268 | -So the corpus number to drive to 100% starts at **53%**. The 82 unsupported-API sites are the -`QueryMultiple`/multi-map/`ExecuteReader` families (parity.md §1); the 256 diagnostic skips -are dominated by the DAP012/DAP015/DAP016/DAP017/DAP050 shapes plus untyped parameters — -harvesting that breakdown per-id into the audit table is the remaining phase-1 analysis step. +So the corpus number to drive to 100% starts at **53%**. Harvested breakdown (net10.0, +diagnostic occurrences; the refusal ids default to *Info* and needed elevating in the harness +globalconfig to be visible at all — worth knowing before trusting any warning tally): + +| bucket | count | meaning | +| --- | --- | --- | +| DAP016 | 272 | **types nested inside `XxxTests`** — the DTO itself is generic-free; only its containment involves the type parameter. A corpus-shape artifact, not real-world weight: see the decision below | +| DAP015 | 100 | untyped parameters (`DynamicParameters` / `object`) — confirms DynamicParameters as phase-3 #1 | +| DAP001: `QueryMultiple(Async)` | 54 | unsupported API | +| DAP001: multi-map `Query<...>` | 52 | unsupported API | +| DAP001: `ExecuteReader(Async)` | 42 | unsupported API | +| DAP013/DAP014 | 40 | tuple results/params | +| DAP017 | 22 | non-public types (private DTOs in the test classes) | +| DAP037/DAP036 | 20 | construction gaps (scalar-ish results, type-handler territory) | + +**Decision needed on the DAP016 bucket** (the single largest): the honest options are (a) +adjust the *suite* — move the nested DTOs to namespace scope, which changes zero observable +behavior and is in the same spirit as "announce your types"; (b) generator support for +call-sites whose types are open over the *containing* class's type parameters — hard, and +possibly not expressible with interceptors at all (the interceptor method cannot see the +enclosing class's type parameters); or (c) accept the ceiling. (a) looks right, but it is a +change to Dapper's test layout, so it is an explicit call, not something to slip in. +Similarly DAP017's 22 sites are private test DTOs, where `internal` would do. ## Scoreboard (to update as things land) From a1267820c4061cb1a7dabd9fb02b8c7aa1c6b8dc Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 11:19:32 +0100 Subject: [PATCH 16/41] Round 5: first behavioral run - 84 failures, every one compiled clean --- notes/harness-baseline.md | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index d59f813e..bae76ab9 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -111,11 +111,36 @@ enclosing class's type parameters); or (c) accept the ceiling. (a) looks right, change to Dapper's test layout, so it is an explicit call, not something to slip in. Similarly DAP017's 22 sites are private test DTOs, where `internal` would do. +## Round 5: the first behavioral run (local SQL Server 2022) + +Control (vanilla, `main` worktree): **760 passed, 0 failed, 29 skipped** (unavailable +providers), 22s. Same suite with AOT interception on: **676 passed, 84 failed** — 42 per SQL +Server provider, perfectly symmetric. + +**Every one of the 84 compiled clean.** This is the measurement caveat made flesh: the +call-sites were "handled", and the failure only exists at runtime. Breakdown by class: + +- **list expansion** (`in @ids`, empty arrays, string_split, padding): the largest group — + `ArgumentException: No mapping exists from object type System.Int32[]...` — the generated + code binds the array member as a raw parameter value; ParameterTests/AsyncTests/MiscTests; +- **literals** (`{=name}`): LiteralTests + LiteralInAsync etc; +- **TVPs / `SqlDataRecord` / `ICustomQueryParameter`**: bound raw, same ArgumentException shape; +- **type handlers** (AnsiString default, `RemoveTypeMap`, IEnumerable handler): 4/provider; +- **a small coercion tail worth individual triage**: OverflowException ×2, InvalidCastException + ×2, RuntimeBinderException ×2 (dynamic single-row), one transaction-inheritance case, one + DataReader case — these may be genuine behavioral divergences at handled sites rather than + known feature gaps, i.e. exactly the class nothing but this run can find. + +The encouraging half: 676 tests pass *with 387 interceptions live*, so interception itself is +broadly sound — failures concentrate precisely where the parity table said the gaps are, which +also re-validates the phase-3 order (tokens and type handlers carry real runtime weight, not +just call-site counts). + ## Scoreboard (to update as things land) | leg | possible (honest) | handled | compiles | tests green | AOT publish | | --- | --- | --- | --- | --- | --- | -| net10.0 | > 387 (denominator understated) | 387 claimed | ✅ | — | — | +| net10.0 | 725 honest | 387 (53.4%) | ✅ | 676/760 (84 fail, all runtime-only) | — | | net8.0 | > 387 | 387 claimed | ✅ | — | — | | net481 | > 405 | 405 claimed | ✅ (needs PR #184) | — | — | From 476fd01d497a6a92534059ea4c0325e5abce3ab4 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 11:33:36 +0100 Subject: [PATCH 17/41] Note that aot-harness is deliberately local-only --- notes/harness-baseline.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index bae76ab9..0d0dc8c7 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -1,7 +1,8 @@ # Harness baseline: Dapper.AOT enabled in the Dapper test suite First real numbers, 2026-08-18. Setup lives on the `aot-harness` branch of the **Dapper** -repo (sibling checkout): local package feed at `../DapperAOT/artifacts` (pack with +repo (sibling checkout; deliberately **local-only, not pushed** — it is a measurement rig, +not work-in-progress on the public repo): local package feed at `../DapperAOT/artifacts` (pack with `NBGV_GitEngine=Disabled dotnet pack src/Dapper.AOT/Dapper.AOT.csproj -c Release -o artifacts`, giving the stable harness version `1.0.0-g`; purge `~/.nuget/packages/dapper.aot/1.0.0-g` between repacks), `[module: DapperAot]` in `DapperAotEnable.cs`, interceptors enabled, and a From 096fdf7f3591c2cc6086cc6beee2e302dba10123 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 11:37:27 +0100 Subject: [PATCH 18/41] Phase 2 log: approach and increments --- notes/phase2-log.md | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 notes/phase2-log.md diff --git a/notes/phase2-log.md b/notes/phase2-log.md new file mode 100644 index 00000000..5f5d70a4 --- /dev/null +++ b/notes/phase2-log.md @@ -0,0 +1,42 @@ +# Phase 2 log: generator capture-model rework + +Running log, newest entries at the bottom. Written as-I-go (sessions can be lost); decisions +that need after-the-fact review are marked **[DECISION]**. The spec is +[generator-audit.md](generator-audit.md); the safety net is byte-identical generated output +(golden fixtures + a hash of the Dapper suite's generated file). + +## Approach + +The full job — plain equatable model, no `CompilationProvider` into the output step, shape +test in CI — is multi-session. Order of attack, chosen so every increment is independently +verifiable and shippable: + +1. **Baseline**: hash the harness's `Dapper.Tests.generated.cs` off current `main`; goldens + are already in git. Any increment that changes either is a bug (or a consciously recorded + exception). +2. **`TypeAccessorInterceptorGenerator` first** — the small generator (state: `Location` + + `ITypeSymbol` + `IMethodSymbol`), converted end-to-end to prove the pattern cheaply. +3. **Interceptor generator, by axis**, each a separate commit with the byte-identical check: + a. `Location` → span-based plain struct (also needed: project the interceptor file path at + parse time, since emit currently asks the `SourceTree` for it); + b. `IMethodSymbol Method` → projected signature model (emit only needs text: return type, + parameter list, name, arity; grouping needs equality, which becomes string equality); + c. `ResultType`/`ParameterType`/`AdditionalCommandState` → the big projection: everything + `WriteRowFactory`/command-factory emission reads from symbols (members, db types, + nullability, constructor choice) moves to parse-time plan data. This is the protobuf-net + "plan model" equivalent and is the bulk of the work. +4. **Remove `CompilationProvider` from the output step** — what Generate still needs from the + compilation (DbCommand type discovery, `AllowUnsafe`, language version) gets its own + projected provider(s) with equatable outputs. +5. **Shape test** (reflection over the model namespace, no Roslyn reference types allowed) + lands with the increment that completes 3c, and gates CI from then on. + +**[DECISION]** Two-stage risk framing: the *retention* harm (cached values pinning +compilations) is fixed by 2-3; the *recompute* harm needs 4 as well. If the session ends +mid-way, 2-3 alone are still worth merging — a full-recompute generator without the leak +beats today's state. + +## Log + +- (start) Branch `phase2-model` from `main` (42ad705 + merges). Baseline hash of the harness + generated file to be recorded below. From d4193e95bbce8fcecf2f500ea72c9ff7b0588b4a Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 11:47:14 +0100 Subject: [PATCH 19/41] Phase 2 log: increment 1 done --- notes/phase2-log.md | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/notes/phase2-log.md b/notes/phase2-log.md index 5f5d70a4..9e91fdd6 100644 --- a/notes/phase2-log.md +++ b/notes/phase2-log.md @@ -38,5 +38,21 @@ beats today's state. ## Log -- (start) Branch `phase2-model` from `main` (42ad705 + merges). Baseline hash of the harness - generated file to be recorded below. +- (start) Branch `phase2-model` from `main`. Baseline hash of the harness generated file: + `f8d3a61f81455c8ef0bcecccbfc348a1584a6bab` (copy kept at `/tmp/baseline-generated.cs` for + diffing; regenerate with `-p:EmitCompilerGeneratedFiles=true + -p:CompilerGeneratedFilesOutputPath=obj/gen` — under `obj/`, or the default glob compiles + the stale copies as source, which cost an hour earlier). +- **Increment 1 done** (pushed on `phase2-model`): `TypeAccessorInterceptorGenerator` + converted end-to-end. New `Dapper.CodeAnalysis.Model` namespace: `LocationSnapshot`, + `EquatableArray`, `TypeAccessorModel`/`AccessorMember`/`ForwarderMethod`, + `GenerationEnvironment` (AllowUnsafe + AssemblyName + has-InterceptsLocationAttribute — the + only three facts the output step needed from the `Compilation`). `ModelShapeTests` enforces + no-Roslyn-fields + IEquatable over the namespace by reflection. Byte-identical: Accessors + goldens unchanged; full suite green net10/net48. + **[DECISION]** `PreGeneratedCodeWriter` keeps its `Compilation` ctor alongside the new + bool one until the big generator converts — avoids touching it out of order. + **[DECISION]** `ForwarderMethod`/type-name projection replicates `CodeWriter.Append(ITypeSymbol)` + exactly (anonymous → MinimallyQualified, else GetTypeName) so output cannot shift. +- Next: increment 3a — `LocationSnapshot` into the interceptor generator's `SuccessSourceState` + (plus projecting the interceptor file path at parse, since emit asks the `SourceTree` for it). From 320c4422df1988d9df5dea09ec95b79283732df4 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 11:53:50 +0100 Subject: [PATCH 20/41] Phase 2 log: 3a done --- notes/phase2-log.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/notes/phase2-log.md b/notes/phase2-log.md index 9e91fdd6..8bd5e3d2 100644 --- a/notes/phase2-log.md +++ b/notes/phase2-log.md @@ -54,5 +54,12 @@ beats today's state. bool one until the big generator converts — avoids touching it out of order. **[DECISION]** `ForwarderMethod`/type-name projection replicates `CodeWriter.Append(ITypeSymbol)` exactly (anonymous → MinimallyQualified, else GetTypeName) so output cannot shift. -- Next: increment 3a — `LocationSnapshot` into the interceptor generator's `SuccessSourceState` +- **Increment 3a done** (pushed): `Location` out of all interceptor-generator cached states. + `LocationSnapshot` gained MappedPath/MappedStartLine (the IncludeLocation SQL comment used + `GetMappedLineSpan`); the interceptor file path and language version are projected at parse + (emit used to reach through `Location.SourceTree` for both). Byte-identical: goldens + unchanged + harness hash equal (`f8d3a61f`). + **[NOTE]** the Bash harness un-escapes backslashes in commands, which broke two heredocs + before being identified — long edit scripts now go via a file, not a heredoc. +- Next was: increment 3a — `LocationSnapshot` into the interceptor generator's `SuccessSourceState` (plus projecting the interceptor file path at parse, since emit asks the `SourceTree` for it). From 44b234abc52e881b5e21c387d3c7770eca93ac8a Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 11:57:03 +0100 Subject: [PATCH 21/41] Phase 2 log: 3b done --- notes/phase2-log.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/notes/phase2-log.md b/notes/phase2-log.md index 8bd5e3d2..b22728d2 100644 --- a/notes/phase2-log.md +++ b/notes/phase2-log.md @@ -61,5 +61,16 @@ beats today's state. unchanged + harness hash equal (`f8d3a61f`). **[NOTE]** the Bash harness un-escapes backslashes in commands, which broke two heredocs before being identified — long edit scripts now go via a file, not a heredoc. +- **Increment 3b done** (pushed): `IMethodSymbol Method` out of the cached state - + `InterceptedMethod`/`MethodParam` plain models; grouping by structural equality; + `CodeWriter.GetAppendTypeName` added as the canonical projection helper. Byte-identical + both ways again (`f8d3a61f`). +- Remaining in the cached model: `ITypeSymbol? ResultType`, `ITypeSymbol? ParameterType`, + and `AdditionalCommandState` (which reaches MemberMap: symbols + an IOperation). That is + increment 3c - the big projection (row factories, command factories, member maps) - plus + 4 (CompilationProvider out of the output step: DbCommand type discovery, AllowUnsafe, + assembly name, pre-generated helpers). 3c is protobuf-net-plan-scale work; the shape test + already guards everything in the Model namespace, and 2+3a+3b are independently mergeable + if the session ends here. - Next was: increment 3a — `LocationSnapshot` into the interceptor generator's `SuccessSourceState` (plus projecting the interceptor file path at parse, since emit asks the `SourceTree` for it). From 9ea89c37a711f41d2214e2c8d8949f7030f01526 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 12:03:12 +0100 Subject: [PATCH 22/41] Phase 2 log: 3c-i done; two cached symbols remain --- notes/phase2-log.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/notes/phase2-log.md b/notes/phase2-log.md index b22728d2..8003fadb 100644 --- a/notes/phase2-log.md +++ b/notes/phase2-log.md @@ -72,5 +72,20 @@ beats today's state. assembly name, pre-generated helpers). 3c is protobuf-net-plan-scale work; the shape test already guards everything in the Model namespace, and 2+3a+3b are independently mergeable if the session ends here. +- **Increment 3c-i done** (pushed): `AdditionalCommandState`/`CommandProperty` plain and + moved into the Model namespace (now under the shape test); member-exists and is-DbCommand + probes run at parse; QueryColumns/CommandProperties on `EquatableArray`, which had to learn + default-vs-empty (QueryColumns semantics distinguish them). Byte-identical again. + **[CORRECTION]** an earlier chat message called QueryColumns equality a latent reference- + equality bug - wrong: `AdditionalCommandState` had proper element-wise static helpers; the + conversion is for shape-consistency, not a bug fix. + **[NOTE]** `HasPublicSettableInstanceMember` returns true for *readonly* fields + (`field.IsReadOnly` looks inverted) - preserved as-is for byte-identical output; flag for + a separate look. +- Remaining cached symbols after 3c-i: exactly two - `ITypeSymbol? ResultType` and + `ITypeSymbol? ParameterType` on `SuccessSourceState`. Everything else in the cached model + is plain. Those two feed the row-factory/command-factory emitters, which consume symbols + deeply (MemberMap/ElementMember); projecting them is the protobuf-net-plan-scale increment + (3c-ii), best started fresh rather than at a session tail. - Next was: increment 3a — `LocationSnapshot` into the interceptor generator's `SuccessSourceState` (plus projecting the interceptor file path at parse, since emit asks the `SourceTree` for it). From 19acc0b7449ae2e81bdc6a7ebffb980e513f0c08 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 12:11:05 +0100 Subject: [PATCH 23/41] Phase 2 log: result-side plan done; one symbol left --- notes/phase2-log.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/notes/phase2-log.md b/notes/phase2-log.md index 8003fadb..ee9a183f 100644 --- a/notes/phase2-log.md +++ b/notes/phase2-log.md @@ -87,5 +87,14 @@ beats today's state. is plain. Those two feed the row-factory/command-factory emitters, which consume symbols deeply (MemberMap/ElementMember); projecting them is the protobuf-net-plan-scale increment (3c-ii), best started fresh rather than at a session tail. +- **Increment 3c-ii result side done** (pushed on stacked branch `phase2-model-plans`; + checkpoint PR #187 covers everything before it): `ResultType` → `RowPlan`/`RowMember` - + the full row-factory projection (member types/db-names/reader-methods, ctor/factory + choice, deferred construction, inbuilt-helper detection, query-column mapping). + `RowReaderState` now de-dupes on plan equality. Byte-identical (`f8d3a61f`) on first run. + **One cached symbol remains in the entire model: `ParameterType`** - the command-factory + side (WriteCommandFactory ~450 lines: member accessors, DbType/size/direction facts, + anonymous shape witness, multi-exec element/cast, UnsafeAccessor-adjacent bits). Same + recipe; roughly twice the surface of the row side. - Next was: increment 3a — `LocationSnapshot` into the interceptor generator's `SuccessSourceState` (plus projecting the interceptor file path at parse, since emit asks the `SourceTree` for it). From 577cabfc1cd3673867ed4ae86c79438e0a582959 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 12:19:06 +0100 Subject: [PATCH 24/41] Phase 2 log: cached model fully plain; only increment 4 remains --- notes/phase2-log.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/notes/phase2-log.md b/notes/phase2-log.md index ee9a183f..0c9bcfdd 100644 --- a/notes/phase2-log.md +++ b/notes/phase2-log.md @@ -96,5 +96,16 @@ beats today's state. side (WriteCommandFactory ~450 lines: member accessors, DbType/size/direction facts, anonymous shape witness, multi-exec element/cast, UnsafeAccessor-adjacent bits). Same recipe; roughly twice the surface of the row side. +- **Increment 3c-ii parameter side done** (pushed): `ParameterType` → `ParamPlan`/`ParamMember` + (command-factory emission incl. Add-mode sizing decisions, DbString/cancellation, the + anonymous shape witness, multi-exec element plan). `CommandFactoryState` keys on plan + equality. **Zero Roslyn objects remain in the cached model.** The states also gained + structural equality (reference equality was still defeating the node-level cache), and the + shape test now covers the SourceState family. Byte-identical throughout (`f8d3a61f`). +- **Remaining for phase 2**: increment 4 only - the interceptor generator's output step still + combines the raw `CompilationProvider` (used for: DbCommand type discovery/ + GetSpecialCommandFlags, GetCommandFactory, AllowUnsafe, assembly name, PreGeneratedCodeWriter, + and the CommandFactoryState systemObject plan). Project those into an equatable environment + (the TypeAccessor generator shows the pattern) and the recompute half is done too. - Next was: increment 3a — `LocationSnapshot` into the interceptor generator's `SuccessSourceState` (plus projecting the interceptor file path at parse, since emit asks the `SourceTree` for it). From cc94b53cdc6eec449290fba12869c2263ba92064 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 12:22:57 +0100 Subject: [PATCH 25/41] Phase 2 log: complete - PRs #187 + #188 --- notes/phase2-log.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/notes/phase2-log.md b/notes/phase2-log.md index 0c9bcfdd..8563baf7 100644 --- a/notes/phase2-log.md +++ b/notes/phase2-log.md @@ -107,5 +107,17 @@ beats today's state. GetSpecialCommandFlags, GetCommandFactory, AllowUnsafe, assembly name, PreGeneratedCodeWriter, and the CommandFactoryState systemObject plan). Project those into an equatable environment (the TypeAccessor generator shows the pattern) and the recompute half is done too. +- **Increment 4 done — PHASE 2 COMPLETE** (PR #188, stacked on #187): `InterceptorEnvironment` + projected via `Select` (AllowUnsafe, assembly name, InterceptsLocationAttribute presence, + the DbCommand special-types sweep pre-filtered, module-level `[CommandFactory]`, the + object fallback plan); the analyzer bridge builds the same environment. All four exit + criteria met: plain equatable cached model (zero Roslyn objects, shape-test-enforced + including the SourceState family), no CompilationProvider into either output step, shape + test in CI, output byte-identical (`f8d3a61f` unchanged across the entire rework; goldens + green net10/net48 at every increment). +- Review pointers for the PRs: the states now have structural equality *as well as* plain + fields — both halves were needed for node-level caching; the `HasPublicSettableInstanceMember` + readonly-field quirk is preserved-as-was and flagged; ordering of the DbCommand special + types follows the same HashSet enumeration as before (projection just moved it earlier). - Next was: increment 3a — `LocationSnapshot` into the interceptor generator's `SuccessSourceState` (plus projecting the interceptor file path at parse, since emit asks the `SourceTree` for it). From 8febfb986b2eee200ad3ccbb0a45f64ebfc3d106 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 14:27:34 +0100 Subject: [PATCH 26/41] Phase 2 log: caching tests landed --- notes/phase2-log.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/notes/phase2-log.md b/notes/phase2-log.md index 8563baf7..a5607a31 100644 --- a/notes/phase2-log.md +++ b/notes/phase2-log.md @@ -119,5 +119,12 @@ beats today's state. fields — both halves were needed for node-level caching; the `HasPublicSettableInstanceMember` readonly-field quirk is preserved-as-was and flagged; ordering of the DbCommand special types follows the same HashSet enumeration as before (projection just moved it earlier). +- **Incremental-caching tests added** (on #188, at review request): three driver-tracked + cases - unrelated-file edit → all output steps Cached; same-file edit *below* the + call-site → cached (this is the case that specifically needs the states' structural + equality, since Parse re-runs and yields fresh instances); real shape edit → re-runs and + output changes. Finding while writing it: editing the SQL *literal* does not change the + generated text (SQL flows through as an argument), so the "real edit" probe must change + shape, not SQL. - Next was: increment 3a — `LocationSnapshot` into the interceptor generator's `SuccessSourceState` (plus projecting the interceptor file path at parse, since emit asks the `SourceTree` for it). From 03fb6f2315fdd597826926899c2de8549ed19ba9 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 14:53:29 +0100 Subject: [PATCH 27/41] Phase 2 log: readonly-field quirk fixed (#190) --- notes/phase2-log.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/notes/phase2-log.md b/notes/phase2-log.md index a5607a31..1ed2f15a 100644 --- a/notes/phase2-log.md +++ b/notes/phase2-log.md @@ -126,5 +126,10 @@ beats today's state. output changes. Finding while writing it: editing the SQL *literal* does not change the generated text (SQL flows through as an argument), so the "real edit" probe must change shape, not SQL. +- **The readonly-field quirk is fixed** (PR #190, stacked on #188, approved by Marc): the + `[CommandProperty]` member probe said readonly fields were assignable and mutable ones were + not; now inverted to sense, with a theory covering all five member shapes (the two field + cases fail without the fix). Kept out of the rework PRs deliberately - behavior change vs + byte-identical contract. Review stack: **#187 → #188 → #190**. - Next was: increment 3a — `LocationSnapshot` into the interceptor generator's `SuccessSourceState` (plus projecting the interceptor file path at parse, since emit asks the `SourceTree` for it). From ed9881d5afed92b68a79761f6c65ac3d0560a441 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 15:38:04 +0100 Subject: [PATCH 28/41] Round 6: DAP051 + restructure takes interception to 68.1% --- notes/harness-baseline.md | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index 0d0dc8c7..b0fe9b2d 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -137,11 +137,28 @@ broadly sound — failures concentrate precisely where the parity table said the also re-validates the phase-3 order (tokens and type handlers carry real runtime weight, not just call-site counts). +## Round 6: DAP051 + the corpus restructure (decision: pre-a then a) + +Marc's call: first make the nested-DTO problem obvious to users - **DAP051** (PR #192) splits +the "generic only by containment" shape out of DAP016, names the culprit container in the +message, links a docs page with the concrete before/after (move `Dog` out), and is a +*warning* (Info is invisible in MSBuild output). Then restructure the suite: every such DTO +(84 types, 12 files) moved into per-file non-generic `XxxTestsTypes` containers with +using-static imports - zero behavioral change, unqualified usages intact. NullTests and +ProcedureTests stay nested deliberately as pinned DAP051-ceiling representatives. + +Result: **494 of 725 handled (68.1%)**, up from 53.4%; DAP051 collapses 272 → 4 (the pins). +The sweep immediately exposed a new generator bug on a first-time shape - a `dynamic`-typed +member emitted illegal `typeof(dynamic)` (CS1962) - fixed as PR #193 with a golden fixture. +Remaining skip buckets: DAP015 ×114 (untyped params - DynamicParameters is now unambiguously +phase-3 #1), DAP013/14 ×40 (tuples), DAP016 ×28 (genuinely-generic incl. helper methods), +DAP017 ×22 (private types), DAP037 ×20 + DAP050 ×12 (construction), DAP036 ×4. + ## Scoreboard (to update as things land) | leg | possible (honest) | handled | compiles | tests green | AOT publish | | --- | --- | --- | --- | --- | --- | -| net10.0 | 725 honest | 387 (53.4%) | ✅ | 676/760 (84 fail, all runtime-only) | — | +| net10.0 | 725 honest | 494 (68.1%) | ✅ | behavioral re-run pending | — | | net8.0 | > 387 | 387 claimed | ✅ | — | — | | net481 | > 405 | 405 claimed | ✅ (needs PR #184) | — | — | From bf9de1b219e9e666dab0d875402f31169480199b Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 15:47:52 +0100 Subject: [PATCH 29/41] Round 6b: 612/760 behavioral; failures track interception growth honestly --- notes/harness-baseline.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index b0fe9b2d..34143151 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -154,11 +154,26 @@ Remaining skip buckets: DAP015 ×114 (untyped params - DynamicParameters is now phase-3 #1), DAP013/14 ×40 (tuples), DAP016 ×28 (genuinely-generic incl. helper methods), DAP017 ×22 (private types), DAP037 ×20 + DAP050 ×12 (construction), DAP036 ×4. +## Round 6b: behavioral run on the restructured suite + +**612 passed / 148 failed** (74 per provider, symmetric; vanilla control remains 760/0). +Failures *rose* from 84 as interception rose from 387 to 494 - which is the honest direction: +the newly-intercepted call-sites exercise runtime gaps that were previously hidden behind +DAP051 refusals. The growth is concentrated exactly where expected: TypeHandlerTests 4 → 16 +per provider (the DTOs now generate, and `SqlMapper.AddTypeHandler` registrations are +ignored by generated code - the phase-3 type-handler story), MiscTests 10 → 16, plus the +existing token/TVP groups. Breakdown: ArgumentException ×56 (raw-bound lists/TVPs), +assertion failures ×30 (behavioral divergence at handled sites - the coercion-matrix tail, +worth triage), InvalidCast ×8, SqlException ×16, NotSupported ×8. + +**[NOTE]** run duration jumped 22s → 9m26s - some newly-intercepted failing tests appear to +burn full command timeouts; worth a look during phase 3 (it makes the behavioral loop slow). + ## Scoreboard (to update as things land) | leg | possible (honest) | handled | compiles | tests green | AOT publish | | --- | --- | --- | --- | --- | --- | -| net10.0 | 725 honest | 494 (68.1%) | ✅ | behavioral re-run pending | — | +| net10.0 | 725 honest | 494 (68.1%) | ✅ | 612/760 (148 fail, runtime-only) | — | | net8.0 | > 387 | 387 claimed | ✅ | — | — | | net481 | > 405 | 405 claimed | ✅ (needs PR #184) | — | — | From 3ac35dc082580c7996099d5a369ee6f23933b1ae Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 17:07:39 +0100 Subject: [PATCH 30/41] DynamicParameters design: delegate to the bag; needs one small Dapper API --- notes/README.md | 1 + notes/dynamicparameters-design.md | 73 +++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 notes/dynamicparameters-design.md diff --git a/notes/README.md b/notes/README.md index 8e7edea7..8dfdcb03 100644 --- a/notes/README.md +++ b/notes/README.md @@ -29,6 +29,7 @@ build time. See [type-vs-generic.md](type-vs-generic.md). | [type-vs-generic.md](type-vs-generic.md) | `Type`-based vs `` APIs, and the "announce your types" design space | | [test-suite-audit.md](test-suite-audit.md) | the Dapper test files as acceptance corpus, and what blocks each | | [harness-baseline.md](harness-baseline.md) | real numbers from the suite with AOT enabled (Dapper repo, `aot-harness` branch) | +| [dynamicparameters-design.md](dynamicparameters-design.md) | phase 3 item 1: delegate to the bag's own vanilla protocol; needs a small Dapper-side API | | [generator-audit.md](generator-audit.md) | **fix-first gate**: the capture model snapshots Roslyn symbols/nodes — retention + cache defeat | ## Scope: the public API, by observable behavior diff --git a/notes/dynamicparameters-design.md b/notes/dynamicparameters-design.md new file mode 100644 index 00000000..97c9d041 --- /dev/null +++ b/notes/dynamicparameters-design.md @@ -0,0 +1,73 @@ +# Phase 3, item 1: DynamicParameters under Dapper.AOT + +The largest measured gap: DAP015 ×114 call-sites, plus it underlies most of the +ParameterTests/ProcedureTests/SqlBuilderTests behavioral failures. + +## The key insight: delegate to the bag itself + +`DynamicParameters` *is* runtime state - no generator can know its members. But it also +already knows how to apply itself to a command: `IDynamicParameters.AddParameters(command, +identity)` is the whole vanilla protocol, including per-parameter DbType/direction/size, +templates, literal replacement, `RemoveUnused` filtering, and storing the attached +`DbParameter`s so `Get` reads outputs afterwards. + +So the generated command factory for a `DynamicParameters`-typed argument should not try to +understand the bag - it should **call the bag's own vanilla implementation**. That gives +byte-exact behavioral parity for the entire DynamicParameters surface in one stroke, because +it *is* the vanilla implementation: + +- `Add(name, value, dbType, direction, size, precision, scale)` - full fidelity; +- output/return parameters + `Get` - automatic (the bag holds the attached parameters); +- `IParameterCallbacks.OnCompleted` - generated `PostProcess` calls it; +- templates (`new DynamicParameters(template)`) - work on JIT exactly as vanilla (it is + vanilla's code path); under **native AOT** they fail inside Dapper's own ref-emit, which + is the same failure vanilla has there - no regression, honestly documented; +- `AddDynamicParams`, `ReplaceLiterals` - along for the ride. + +## The blocker, and the Dapper-side fix **[NEEDS SIGN-OFF]** + +`AddParameters(command, identity)` consumes `identity.Sql` on its first line (literal +tokens), so a null identity NREs - and `Identity`'s constructor is internal, so generated +code cannot build one. There is also no public way to *reimplement* the protocol from +outside: the per-parameter metadata lives in private `ParamInfo`. + +Proposed: a small public overload on `DynamicParameters` (or a `SqlMapper` static) in +**Dapper** itself: + +```csharp +public void AddParameters(IDbCommand command) + => AddParameters(command, /* identity built from command.CommandText */); +``` + +i.e. self-apply against the command's own SQL - which is what `identity.Sql` is in practice. +Useful beyond AOT (extenders have wanted a way to invoke bags directly). No release needed +yet: the harness consumes Dapper by project reference, so this can sit on a Dapper branch +until the next ship. + +**Consumer-version safety**: generated code calling a new Dapper API would break consumers +on older Dapper, so the generator must *probe for the overload symbol* and only take this +path when present - otherwise the call-site keeps DAP015 and stays on vanilla (the +protobuf-net probe-for-`UnsafeAccessor` pattern). + +## Generator/runtime shape + +- Recognition: parameter type is `DynamicParameters`, or implements + `SqlMapper.IDynamicParameters` (the interface case needs the same overload story - start + with the concrete type, where the new overload is definitely present). +- Emit: `CommandFactory` whose `AddParameters` calls + `args.AddParameters(cmd.UnderlyingCommand)`; `RequirePostProcess => true` with + `PostProcess` invoking `((SqlMapper.IParameterCallbacks)args).OnCompleted()` when the bag + implements it (runtime test - implementers are runtime state); +- **No command caching / no prepare** for these factories: re-applying a bag to a reused + command would double-add parameters, and prepare needs statically-known types. Refuse + `[CacheCommand]` combination (diagnostic) rather than misbehave; +- `UpdateParameters` for batch reuse: not applicable (a bag is a single-command concept; + multi-exec over `IEnumerable` stays unsupported for now); +- DAP015 splits: `DynamicParameters`-typed args become supported; `object`/`dynamic`-typed + stay DAP015 (that is announced-types territory - the runtime type is unknowable). + +## Scope check against the corpus + +Statically-`DynamicParameters` sites cover the bulk of the 114; the remainder are +`object`-typed pass-throughs and helper indirection, which stay refused with an honest +diagnostic. Measure the split after implementation rather than guessing it now. From 5bffe780c586c623aed98f5e4e9330c182552fcc Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 17:36:45 +0100 Subject: [PATCH 31/41] Round 7: DynamicParameters at 73.5%; First-pipeline drain divergence found --- notes/harness-baseline.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index 34143151..ffcd87f0 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -169,6 +169,28 @@ worth triage), InvalidCast ×8, SqlException ×16, NotSupported ×8. **[NOTE]** run duration jumped 22s → 9m26s - some newly-intercepted failing tests appear to burn full command timeouts; worth a look during phase 3 (it makes the behavioral loop slow). +## Round 7: DynamicParameters lands (delegate-to-the-bag) + +Design in [dynamicparameters-design.md](dynamicparameters-design.md); the Dapper-side +`AddParameters(IDbCommand)` overload sits on the local `dynamicparameters-apply` branch +(API shape awaiting Marc's sign-off), and the generator support (probe-gated: older Dapper +keeps the DAP015 refusal, goldens untouched) is on `dynamicparameters-support`. + +**533 of 725 handled (73.5%)**, up from 68.1%; DAP015 falls 114 → 30 (the rest are +object-typed args - announced-types territory). First behavioral run caught a real bug the +unit suite could not: stored procedures took `ParameterMode.All` before the dynamic-bag +Defer check, so proc+bag sites got the parameterless fallback factory ("expects parameter +@ID, which was not supplied") - fixed; ProcedureTests goes 16 failures → 0 bag-related +(the 2 left are the known list-expansion gap). + +**New triage item, found by the suite (parity.md §4 said "verify" - now verified as a real +divergence): the First/Single pipeline's CommandBehavior/drain semantics.** +`QueryFirst("select * from #mydata; raiserror(...)")` over 500k rows: vanilla surfaces the +trailing DbException; Dapper.AOT does not (Assert.ThrowsAny: no exception thrown), and the +run burns ~4.5 minutes per provider apparently draining pending rows on reader close. Both +halves point at how the generated First path picks CommandBehavior vs vanilla's deliberate +choices (`Settings.UseSingleRowOptimization` exists precisely because of this trap). + ## Scoreboard (to update as things land) | leg | possible (honest) | handled | compiles | tests green | AOT publish | From df1fc4e0d41b14ba6083ab1f5c67ae2a52427671 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 17:46:40 +0100 Subject: [PATCH 32/41] Round 7b: 612/762; every failure class maps to a planned feature --- notes/harness-baseline.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index ffcd87f0..0bea7091 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -191,11 +191,19 @@ run burns ~4.5 minutes per provider apparently draining pending rows on reader c halves point at how the generated First path picks CommandBehavior vs vanilla's deliberate choices (`Settings.UseSingleRowOptimization` exists precisely because of this trap). +## Round 7b: confirmation run with the proc-mode fix + +**612 passed / 150 failed** (down from 170; the proc-mode fix recovered 20). Every remaining +failure class maps onto a planned phase-3 feature: ParameterTests ×23/provider (tokens: +list expansion, TVPs, custom params), TypeHandlerTests ×16 (type-handler story), MiscTests +×16 (coercions + tokens), Async/Literal (literals), plus the First-pipeline drain pair and +the small tail. Nothing unexplained. + ## Scoreboard (to update as things land) | leg | possible (honest) | handled | compiles | tests green | AOT publish | | --- | --- | --- | --- | --- | --- | -| net10.0 | 725 honest | 494 (68.1%) | ✅ | 612/760 (148 fail, runtime-only) | — | +| net10.0 | 725 honest | 533 (73.5%) | ✅ | 612/762 (150 fail, runtime-only) | — | | net8.0 | > 387 | 387 claimed | ✅ | — | — | | net481 | > 405 | 405 claimed | ✅ (needs PR #184) | — | — | From 0d79ccb2ebe9cd2cdeb4a2b38ab0b7c475850e19 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 18:08:18 +0100 Subject: [PATCH 33/41] Interceptor-syntax migration: soft-target requirement recorded --- notes/parity.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/notes/parity.md b/notes/parity.md index 1845ea51..c88e43e8 100644 --- a/notes/parity.md +++ b/notes/parity.md @@ -155,6 +155,14 @@ analysis (DAP2xx), `[SqlSyntax]`. `GetInterceptableLocation` (Roslyn 4.11+) so the shipped baseline can stay low, and `docs/aot-grpc.md` there records the encoding — reverse-engineered and proven by hand — as the fallback if the reflection ever stops working. Port that approach. + **Requirement (Marc, 2026-08-18): soft-target.** Do not force SDK/compiler updates: emit + the new version+data form only when the *hosting* compiler supports it, and keep emitting + the old path/line/column form on down-level. The clean fact making this safe: generation + and consumption happen in the same csc invocation, so reflecting on the running Roslyn for + `GetInterceptableLocation` (added Roslyn 4.11 / VS 17.11; the encoded form itself landed in + 4.10) is exactly the right capability probe — no SDK/LangVersion sniffing needed (both + attribute forms compile under the same C# 11+ interceptors gate). The emitted + `InterceptsLocationAttribute` polyfill needs both constructors. - **New diagnostic: warn on "has no meaning" APIs.** When AOT is enabled, detect usage of the APIs whose *concept* doesn't exist under AOT — the plan-cache surface From eb632e36c04ceb1efcd07dc10fb00714be63cc4d Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 18:10:59 +0100 Subject: [PATCH 34/41] Tokens: runtime-SQL design - per-factory memoized role scan --- notes/tokens.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/notes/tokens.md b/notes/tokens.md index 1a949335..e60e38d8 100644 --- a/notes/tokens.md +++ b/notes/tokens.md @@ -99,6 +99,26 @@ Dapper and AOT both infer text-vs-proc when `commandType` is unspecified (`WhitespaceOrReserved` heuristic — shared regex, `DapperAotExtensions.cs:59`). Believed equivalent; pin with a corpus test. +## Runtime-provided SQL (Marc's question, 2026-08-18) + +The member set is always known at build time even when the SQL is not; only each member's +*role* varies (bind / inject / expand / unused). Three-tier emission: + +1. constant SQL with tokens → fully baked injection/expansion code, zero runtime decision; +2. constant SQL without tokens → today's code, zero overhead; +3. runtime SQL → keep the statically-typed per-member code for each possible role, gated by + a runtime role-assignment scan of the SQL, **memoized per call-site factory** with a + single-slot cache (`ReferenceEquals` then string equality) - the dominant "runtime" + string is built once and reused, so the steady state is one reference compare. + +Precedent: deferred-map call-sites already emit runtime `Include(sql, commandType, name)` +tests - runtime SQL interrogation is an existing pattern here, extended not invented. Never +worse than vanilla (which runtime-scans too, cached per Identity in a global dictionary; +per-composed-call SQL misses that cache as well). Cost named honestly: tier-3 factories +carry both role implementations for members that *could* be literal (value-typed) or +expandable (enumerable), so runtime-SQL call-sites get fatter factories; members that can +never play those roles pay nothing. + ## Decision table (to fill in as designs land) | token | AOT position | design sketch | From a257808dd867c76f3fe4b85a77b87bf92f643f95 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 18:57:54 +0100 Subject: [PATCH 35/41] Record the feature-detection rule (DAP052) in the design note --- notes/dynamicparameters-design.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/notes/dynamicparameters-design.md b/notes/dynamicparameters-design.md index 97c9d041..fb250b44 100644 --- a/notes/dynamicparameters-design.md +++ b/notes/dynamicparameters-design.md @@ -44,10 +44,13 @@ Useful beyond AOT (extenders have wanted a way to invoke bags directly). No rele yet: the harness consumes Dapper by project reference, so this can sit on a Dapper branch until the next ship. -**Consumer-version safety**: generated code calling a new Dapper API would break consumers -on older Dapper, so the generator must *probe for the overload symbol* and only take this -path when present - otherwise the call-site keeps DAP015 and stays on vanilla (the -protobuf-net probe-for-`UnsafeAccessor` pattern). +**Consumer-version safety (the rule, per Marc)**: generated code calling a new Dapper API +would break consumers on older Dapper, so for *every* Dapper-side dependency the generator +must (a) probe for the symbol, (b) never emit code that cannot compile against the +referenced Dapper, and (c) refuse with a diagnostic that names exactly which API is missing +and what it enables - **DAP052** ("Feature requires a newer Dapper"), not a generic message, +and never a baffling compiler error. Implemented; the DAP052 verifier runs against the +packaged (older) Dapper, which is exactly the scenario it exists for. ## Generator/runtime shape From eb7b4a5206fc31c388ca761e71c45deb3e15ed56 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 19:22:57 +0100 Subject: [PATCH 36/41] Round 8: CommandBehavior parity fix (PR #196) --- notes/harness-baseline.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index 0bea7091..06f5b587 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -199,6 +199,22 @@ list expansion, TVPs, custom params), TypeHandlerTests ×16 (type-handler story) ×16 (coercions + tokens), Async/Literal (literals), plus the First-pipeline drain pair and the small tail. Nothing unexplained. +## Round 8: the First-pipeline divergence, root-caused and fixed (PR #196) + +The suite's `QueryFirst_PerformanceAndCorrectness` failure decomposed into two bugs with one +root cause, pinned by a side-by-side repro (vanilla and AOT on the same connection): +Dapper.AOT hardcoded `CommandBehavior.SingleResult` (+`SingleRow` on First), where vanilla +strips **both by default** (`Settings.AllowedCommandBehaviors = ~(SingleResult|SingleRow)`; +the optimizations are opt-in) - because with those flags SqlClient cancels the remainder of +the batch on close: `select *; raiserror(...)` produced *no exception* under AOT while +vanilla threw `SqlException`, and the flags interplay made async one-row reads ~10x slower +(568ms → 57ms at 100k rows in the repro; minutes at the suite's 500k). Fix: SequentialAccess +only, matching vanilla's effective default, with the opt-in knob location noted in code. + +Lesson worth keeping: parity.md §4 had this exact row as "AOT chooses behaviors itself; +probably fine, verify" - the behavioral suite is what turned "probably fine" into two +confirmed bugs and a 10x. + ## Scoreboard (to update as things land) | leg | possible (honest) | handled | compiles | tests green | AOT publish | From 6d4ee7627097a9f7274d241b48ed9eb39a21487a Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 19:23:58 +0100 Subject: [PATCH 37/41] Round 8b: 616/762, suite loop 17s --- notes/harness-baseline.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index 06f5b587..1209697f 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -199,6 +199,13 @@ list expansion, TVPs, custom params), TypeHandlerTests ×16 (type-handler story) ×16 (coercions + tokens), Async/Literal (literals), plus the First-pipeline drain pair and the small tail. Nothing unexplained. +## Round 8b: confirmation - suite duration 9m28s → **17 seconds** + +With #195+#196 combined: 616 passed / 146 failed. The behavior fix recovered the +SingleRowTests pair plus two async dynamic reads, and gave the whole behavioral loop its +speed back. Remaining failures unchanged in shape: tokens (ParameterTests ×23/provider, +Literal ×5, chunks of Misc/Async), type handlers (×16), coercion tail. + ## Round 8: the First-pipeline divergence, root-caused and fixed (PR #196) The suite's `QueryFirst_PerformanceAndCorrectness` failure decomposed into two bugs with one @@ -219,7 +226,7 @@ confirmed bugs and a 10x. | leg | possible (honest) | handled | compiles | tests green | AOT publish | | --- | --- | --- | --- | --- | --- | -| net10.0 | 725 honest | 533 (73.5%) | ✅ | 612/762 (150 fail, runtime-only) | — | +| net10.0 | 725 honest | 533 (73.5%) | ✅ | 616/762 (146 fail; suite runs in 17s) | — | | net8.0 | > 387 | 387 claimed | ✅ | — | — | | net481 | > 405 | 405 claimed | ✅ (needs PR #184) | — | — | From 4ef5a9ab25bcacdb0fe51d5596ed7569347bfde7 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 19:42:22 +0100 Subject: [PATCH 38/41] Round 9: list expansion lands (PR #197), 638/762 --- notes/harness-baseline.md | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index 1209697f..f740563e 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -199,6 +199,26 @@ list expansion, TVPs, custom params), TypeHandlerTests ×16 (type-handler story) ×16 (coercions + tokens), Async/Literal (literals), plus the First-pipeline drain pair and the small tail. Nothing unexplained. +## Round 9: list expansion (PR #197) - 638/762, in-list group cleared + +With #195+#196+#197 combined: **638 passed / 124 failed** (up from 616/146; the in-list +group recovered 22 across both providers). Interception count **unchanged at 533/725**, +which is the expected shape: expandable members were already intercepted - binding the +list as a single raw parameter, which fails at execution - so this round is correctness +at existing sites, not coverage. The two new parse-side skips (multi-exec over expandable +elements; expandable alongside an output/return parameter, whose PostProcess read-back is +by index) cost the suite nothing. + +Design note carried on the PR: the generated code calls the public-but-\[Obsolete\] +`SqlMapper.PackListParameters` (per-line CS0618 pragma), which works against every +shipped Dapper with no feature-detection needed; the alternative is a fresh non-obsolete +Dapper wrapper, probe-gated like the DynamicParameters overload. Marc's call at review. + +Remaining failure classes, all mapped to planned features: TypeHandlerTests x16/provider, +MiscTests x15 (coercions + tokens), ParameterTests x14 (TVPs/DataTable/ +ICustomQueryParameter/SqlDecimal), Literal x5, Async x5, small tail (Constructor x2, +Xml/Transaction/enum-handler x1s). + ## Round 8b: confirmation - suite duration 9m28s → **17 seconds** With #195+#196 combined: 616 passed / 146 failed. The behavior fix recovered the @@ -226,7 +246,7 @@ confirmed bugs and a 10x. | leg | possible (honest) | handled | compiles | tests green | AOT publish | | --- | --- | --- | --- | --- | --- | -| net10.0 | 725 honest | 533 (73.5%) | ✅ | 616/762 (146 fail; suite runs in 17s) | — | +| net10.0 | 725 honest | 533 (73.5%) | ✅ | 638/762 (124 fail; suite runs in 18s) | — | | net8.0 | > 387 | 387 claimed | ✅ | — | — | | net481 | > 405 | 405 claimed | ✅ (needs PR #184) | — | — | From 7c8046ed43b1088c2028a94bc5f1cadc4e262464 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 19:58:35 +0100 Subject: [PATCH 39/41] Round 10: custom parameters + the two bugs they uncovered, 658/793 --- notes/harness-baseline.md | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index f740563e..0177a322 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -199,6 +199,36 @@ list expansion, TVPs, custom params), TypeHandlerTests ×16 (type-handler story) ×16 (coercions + tokens), Async/Literal (literals), plus the First-pipeline drain pair and the small tail. Nothing unexplained. +## Round 10: custom parameters (PR #198) + two real bugs it uncovered - 658/793 + +ICustomQueryParameter members (PR #198, stacked on #197): the value adds itself via +`AddParameter(command, name)`, null reference members throw with vanilla's message, struct +members skip the null test, and the #197 guards generalise to "self-binding members" +(expandable or custom). Cleared the TVP/custom-param group. A bare `DataTable` member is +deliberately out of scope - vanilla routes it through its default-registered +`DataTableHandler`, so it belongs to the type-handler story. + +Making custom parameters work at all exposed two real bugs the unit suites could not see: + +- **Dapper.AOT never cleared `cmd.Parameters` on teardown** (PR #199). Vanilla's finally + blocks all do `cmd?.Parameters.Clear()` ("Add-tastic"), and it is load-bearing: a + caller-supplied `DbParameter` otherwise stays owned by the dead command's collection and + the next use throws. First fix in `UnifiedCommand.Cleanup` was insufficient - the query + and execute pipelines dispose via `SyncCommandState`/`AsyncCommandState`, which never + pass through it - so the state Dispose paths clear too. Recycled commands are nulled out + of the state first, so their parameters are kept for in-place update, unchanged. +- **the #2225 overload dispatched statically** (amended on that PR). A subclass that hides + `AddParameters` and re-implements `IDynamicParameters` - the `DynamicParameterWithIntTVP` + pattern in the suite itself - was skipped by the direct call; it now routes via + `((SqlMapper.IDynamicParameters)this)`, matching vanilla execution. Test added there. + +**658 passed / 106 failed** (616 -> 638 -> 652 -> 658 this evening; denominator grew to 793 +with the new #2225 tests). Interception still 533/725. Remaining ParameterTests x5/provider: +DataTable pair (type-handler story), SqlDecimal (read-side coercion), legacy `?` token, +SupportInit (ISupportInitialize, read-side). Larger classes: TypeHandler x16, Misc x15 +(dynamic-row DBNull/mutability, privates/fields, coercions), Literal x5 + Async literal x3 +(coordinate with external #191), Async dynamic x2. + ## Round 9: list expansion (PR #197) - 638/762, in-list group cleared With #195+#196+#197 combined: **638 passed / 124 failed** (up from 616/146; the in-list @@ -246,7 +276,7 @@ confirmed bugs and a 10x. | leg | possible (honest) | handled | compiles | tests green | AOT publish | | --- | --- | --- | --- | --- | --- | -| net10.0 | 725 honest | 533 (73.5%) | ✅ | 638/762 (124 fail; suite runs in 18s) | — | +| net10.0 | 725 honest | 533 (73.5%) | ✅ | 658/793 (106 fail; suite runs in ~18s) | — | | net8.0 | > 387 | 387 claimed | ✅ | — | — | | net481 | > 405 | 405 claimed | ✅ (needs PR #184) | — | — | From a084a827005e420fda795490dd3c8214ac2df2e0 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 18 Aug 2026 20:08:12 +0100 Subject: [PATCH 40/41] Round 11: dynamic-record fidelity, 672/793 --- notes/harness-baseline.md | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/notes/harness-baseline.md b/notes/harness-baseline.md index 0177a322..d3672230 100644 --- a/notes/harness-baseline.md +++ b/notes/harness-baseline.md @@ -199,6 +199,27 @@ list expansion, TVPs, custom params), TypeHandlerTests ×16 (type-handler story) ×16 (coercions + tokens), Async/Literal (literals), plus the First-pipeline drain pair and the small tail. Nothing unexplained. +## Round 11: dynamic-record fidelity (PR #200) - 672/793 + +`DynamicRecord` diverged from vanilla's `DapperRow` three ways, all caught by the suite +and none by the unit tests: a null column came back as `DBNull` from the dynamic and +dictionary surfaces (vanilla hands back null; `(int?)row.A` threw RuntimeBinderException); +dynamic records refused mutation (vanilla supports member set, add and remove - the DLR +detail is that a set-binding must *yield* the assigned value, since assignment is an +expression); and a missing member threw `KeyNotFoundException` where vanilla's indexer is +TryGetValue-and-return - null, with the *value-type cast* being what throws, from the +binder. The `DbDataRecord` surface keeps its ADO.NET contract (`GetValue` restores +`DBNull`; the string indexer still throws), since vanilla has no counterpart there. +Runtime-lib only, branched from main - no stacking. The fields array is shared per shape +(it is the Tokenize state), so structural mutation copies before diverging. + +**672 passed / 92 failed.** The evening's arc: 616 -> 638 (#197) -> 652 (#198) -> 658 +(#199 + the #2225 amendment) -> 672 (#200). Remaining classes: TypeHandler x16/provider, +Misc x11 (privates/fields, inheritance, Int16/Int32 coercions, nullable char in/out, +unexpected-data message, multi-exec object[]), Literal x5 + async x3, ParameterTests x5 +(DataTable pair = type-handler story, SqlDecimal read-side, legacy `?` token, +ISupportInitialize), Constructor x2, small tail. + ## Round 10: custom parameters (PR #198) + two real bugs it uncovered - 658/793 ICustomQueryParameter members (PR #198, stacked on #197): the value adds itself via @@ -276,7 +297,7 @@ confirmed bugs and a 10x. | leg | possible (honest) | handled | compiles | tests green | AOT publish | | --- | --- | --- | --- | --- | --- | -| net10.0 | 725 honest | 533 (73.5%) | ✅ | 658/793 (106 fail; suite runs in ~18s) | — | +| net10.0 | 725 honest | 533 (73.5%) | ✅ | 672/793 (92 fail; suite runs in ~18s) | — | | net8.0 | > 387 | 387 claimed | ✅ | — | — | | net481 | > 405 | 405 claimed | ✅ (needs PR #184) | — | — | From 4001e87fb9c8cffeff25dec90ac9ab154a3faac6 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Wed, 19 Aug 2026 10:13:15 +0100 Subject: [PATCH 41/41] Sync with main; point parity rows at their open PRs The table lands on main via #186; from here each feature PR flips its own cells, so the table and the merge history cannot drift apart. Rows with an open PR say so, and the flip to a settled status is that PR's job. --- notes/parity.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/notes/parity.md b/notes/parity.md index c88e43e8..2f7a31c4 100644 --- a/notes/parity.md +++ b/notes/parity.md @@ -44,7 +44,7 @@ Two levers change several complexity scores and are worth naming up front: | `GetRowParser(reader)` | ✅ | — | — | | | `GetRowParser(reader, Type concreteType, ...)` | ❌ | med | low* | discriminator/polymorphism pattern; dictionary lookup once types are announced | | `Parse` / `Parse(Type)` / `Parse` (dynamic) | ❌ ❓ | low | low | same reader machinery, different entry point | -| `AsTableValuedParameter` (`DataTable` / `SqlDataRecord`) | ❓ | med | med | SQL Server crowd; pairs with `ICustomQueryParameter` | +| `AsTableValuedParameter` (`DataTable` / `SqlDataRecord`) | ❓ | med | med | **PR #198 open** covers it (the result *is* an `ICustomQueryParameter`); a bare `DataTable` member rides the type-handler story instead (vanilla registers `DataTableHandler` by default) | | `AsList` | n/a | — | — | trivial helper; confirm it doesn't count as a candidate site | | `GetTypeDeserializer(Type, reader, startBound, length, ...)` | ❌ | low-med | low* | a valid raw-materializer API, not mere plumbing: with announced types it's the same dispatch map, returning a boxed `Func`. Its generic strengthening **already exists**: `GetRowParser` (same slicing knobs), which AOT supports | | `CreateParamInfoGenerator(Identity, ...)` | ❌ | low | med | the raw parameter-binder factory; **no generic counterpart exists in Dapper** — see "Strengthened APIs" in [type-vs-generic.md](type-vs-generic.md) for the proposed `` form | @@ -59,13 +59,13 @@ Two levers change several complexity scores and are worth naming up front: | --- | --- | --- | --- | --- | | anonymous types / concrete POCOs | ✅ | — | — | | | fields as members | ❓ | low | low | verify | -| `DynamicParameters` | ❌ | **high** | high | densest single gap: templates, `AddDynamicParams`, per-param DbType/direction/size/precision/scale, `Get` post-execute, output callbacks. Candidate design: a real (hand-written, AOT-safe) implementation in the runtime lib that generated commands consume — generation only needed for template objects | +| `DynamicParameters` | ❌ | **high** | high | **PR #195 open**: delegate to the bag's own protocol (pairs with Dapper #2225); covers subclasses via interface dispatch. Templates ride on the same path | | `SqlMapper.IDynamicParameters` (custom impls) | ❌ | low-med | med | interface receives the `IDbCommand`, so callable directly — blocked on `Identity` (Dapper-internal) in the signature; owning Dapper permits an AOT-friendly overload | -| `SqlMapper.ICustomQueryParameter` | ❌ ❓ | med | low | interface is `AddParameter(IDbCommand, string)` — generated code can just call it | +| `SqlMapper.ICustomQueryParameter` | ❌ ❓ | med | low | **PR #198 open**: generated code calls it, with vanilla's null semantics; uncovered a teardown bug (**PR #199**: parameters must be cleared on dispose, as vanilla does) | | `IParameterLookup` / `IParameterCallbacks` | ❌ ❓ | low | low-med | obscure but public | | `DbString` | ✅ | — | — | DAP048 nudges to `[DbValue]`; keep the Dapper spelling, the corpus uses it | | output / return params via `[DbValue(Direction=...)]` | ⚠️ | — | — | AOT spelling works; Dapper spelling rides on `DynamicParameters` above | -| list expansion (`in @ids`) | ❌ ❓ | **high** | med | ubiquitous. Runtime rewrite helper (per-invocation list size); analyzer already detects the shape. [tokens.md](tokens.md) §2 | +| list expansion (`in @ids`) | ❌ ❓ | **high** | med | **PR #197 open**: delegates to vanilla's `PackListParameters`, which owns the whole contract — no runtime rewrite helper needed after all. [tokens.md](tokens.md) §2 | | literal injection (`{=name}`) | ❌ ❓ | med | low-med | formatting rules compile-time decidable. [tokens.md](tokens.md) §3 | | pseudo-positional (`?foo?`) | ❌ ❓ | low | med | OleDb/Access corner. [tokens.md](tokens.md) §4 | | enum / nullable / `char` / `Guid` params | ⚠️❓ | med | low | verify edge conversions vs Dapper | @@ -85,7 +85,7 @@ Two levers change several complexity scores and are worth naming up front: | constructor binding, `[ExplicitConstructor]` | ✅ | — | — | plus factory methods (AOT extension) | | `required` / init-only members | ✅ | — | — | `RequiredProperties` fixture | | fields | ❓ | low | low | verify | -| `dynamic` rows — behavioral fidelity | ⚠️ | high | med | the row *type* is internal and irrelevant; the observable contract is: `dynamic` member access, `IDictionary` + `IReadOnlyDictionary`, tolerates add/remove, specific null/missing semantics — pin that matrix with tests against AOT's row | +| `dynamic` rows — behavioral fidelity | ⚠️ | high | med | **PR #200 open**: null (not DBNull) on the dynamic surface, mutation (set/add/remove), missing member is null with the cast throwing — the whole matrix the suite pins | | tuple results | ❌ | med | med | DAP013; design already framed by `[BindTupleByName]` | | enum results (string→enum case-insens., widening, `ShortEnum`) | ⚠️❓ | high | low-med | Dapper recently changed precedence (prefer type handlers, #2200) — match the *new* behavior | | `MatchNamesWithUnderscores` | ❓ | med-high | low | snake_case databases; needs a compile-time equivalent (global option/attr) | @@ -104,7 +104,7 @@ Two levers change several complexity scores and are worth naming up front: | `Settings.CommandTimeout` (global default) | ❓ | med | low | AOT has per-site args + `[CommandProperty]`; needs a global knob | | `Settings.InListStringSplitCount` | ❌ | med | low* | *after* list expansion; SQL Server plan-stability win | | `Settings.PadListExpansions` | ❌ | low-med | low* | same | -| `Settings.UseSingleResult/UseSingleRowOptimization` | ❓ | low | low | AOT picks `CommandBehavior` itself; verify equivalence | +| `Settings.UseSingleResult/UseSingleRowOptimization` | ❓ | low | low | **PR #196 open**: verification found a real divergence (AOT hardcoded the opt-in flags; swallowed trailing errors, 10x slower async) — now matches vanilla's default | | `Settings.FetchSize` (Oracle) | ⚠️ | low | low | `GlobalFetchSize` exists; verify | | `SqlMapper.ConnectionStringComparer` | 🚫? | **zero-ish** | — | exists to partition the runtime identity/cache — a concept AOT doesn't have | | `FeatureSupport` (per-provider null-array quirks) | ❓ | low | low | folds into list-expansion helper |