diff --git a/notes/README.md b/notes/README.md new file mode 100644 index 00000000..8dfdcb03 --- /dev/null +++ b/notes/README.md @@ -0,0 +1,68 @@ +# 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 | +| --- | --- | +| [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 | +| [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 + +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 + 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/dynamicparameters-design.md b/notes/dynamicparameters-design.md new file mode 100644 index 00000000..fb250b44 --- /dev/null +++ b/notes/dynamicparameters-design.md @@ -0,0 +1,76 @@ +# 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 (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 + +- 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. 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/harness-baseline.md b/notes/harness-baseline.md new file mode 100644 index 00000000..d3672230 --- /dev/null +++ b/notes/harness-baseline.md @@ -0,0 +1,340 @@ +# 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; 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 +`.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. + +## 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. + +## 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%**. 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. + +## 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). + +## 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. + +## 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). + +## 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). + +## 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. + +## 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 +`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 +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 +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 +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 | +| --- | --- | --- | --- | --- | --- | +| 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) | — | — | + +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 + +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 new file mode 100644 index 00000000..2f7a31c4 --- /dev/null +++ b/notes/parity.md @@ -0,0 +1,175 @@ +# 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 | **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 | +| `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 — 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 | + +## 2. Parameters (input side) + +| feature | AOT status | impact | complexity | notes | +| --- | --- | --- | --- | --- | +| anonymous types / concrete POCOs | ✅ | — | — | | +| fields as members | ❓ | low | low | verify | +| `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 | **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 | **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 | +| 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 — 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) | +| `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 | **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 | + +## 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]`. + +## 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); + 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. + **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 + (`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*. diff --git a/notes/phase2-log.md b/notes/phase2-log.md new file mode 100644 index 00000000..1ed2f15a --- /dev/null +++ b/notes/phase2-log.md @@ -0,0 +1,135 @@ +# 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`. 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. +- **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. +- **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. +- **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. +- **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. +- **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. +- **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). +- **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. +- **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). 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)). diff --git a/notes/test-suite-audit.md b/notes/test-suite-audit.md new file mode 100644 index 00000000..e8a4115e --- /dev/null +++ b/notes/test-suite-audit.md @@ -0,0 +1,96 @@ +# 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. +- **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 + 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 + +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) + +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 + 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..e60e38d8 --- /dev/null +++ b/notes/tokens.md @@ -0,0 +1,132 @@ +# 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). + +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 + +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. + +## 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 +**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".