Conversation
Smurf-IV
reviewed
Jul 28, 2025
| // Example using a module-level attribute | ||
| using Dapper; | ||
|
|
||
| [module: TypeHandler<YourDotNetType, YourCustomTypeHandler>] |
There was a problem hiding this comment.
If I have a query that required certain set of typeHandlers,
And other do not,
Then how to restore them, and then add if and when needed ?
Author
There was a problem hiding this comment.
Hello, this PR does not currently allow for switching or disabling type handlers on the fly.
This is definitely a feature that could be added, though.
|
Any update on this PR or #117? I am debugging custom Dapper.AOT I have registered them via and they are not used in source generation and I just get runtime cast errors when loading data from DB "Invalid cast from 'System.Int64' to 'MyCustomType'." What is the purpose of |
mgravell
added a commit
that referenced
this pull request
Aug 23, 2026
SqlMapper.AddTypeHandler registrations now work under interception, in both directions, by delegating to vanilla's own decision procedure at execution time rather than trying to see runtime state at compile time: - writes: a member type the generator does not recognize emits a dispatch through SqlMapper.LookupDbType (public, CS0618-suppressible - the same tier as PackListParameters): handler present -> handler.SetValue (with DBNull for null, never null itself - the TypeHandler<T> interface impl NREs on a raw null); otherwise any returned DbType is applied and the value binds raw as before (demand:false deliberately - modern providers natively handle types vanilla's map does not, DateOnly being the live case until the Dapper re-enable ships). Update-mode mirrors it, so command reuse stays legal (the parameter shape is stable); an expandable member checks the handler *first*, which is the order vanilla applies (a handled collection type must not list-expand - Issue253). - reads: the lib cannot reference Dapper (a consumer may use Dapper or Dapper.StrongName, and a hard reference would split the handler registry between the two), so generated code installs a TypeHandlerBridge from a module initializer, compiled against the consumer's own Dapper; the flexible read path consults it, and a whole-type handler overrides a generated row factory (RowFactory<T>.Resolve), matching vanilla's handler-before-member-binding order. A ModuleInitializerAttribute polyfill is emitted for down-level targets, probe-gated like the interceptor attribute; the whole feature is inert against a Dapper too old to have HasTypeHandler/LookupDbType. - char/char? stay excluded (their StringFixedLength map entry pads the round-trip); object/dynamic stay excluded (runtime-typed values); ParamMember gains TypeOfName, mirroring RowMember's, because typeof on an annotated reference type or dynamic does not compile. This also clears the bare-DataTable TVP shape and the Xml types for free - vanilla registers DataTableHandler and the XML handlers by default, and the dispatch reaches them like any other registration. Dapper test suite: 677 -> 705/793. Design and probed facts in notes/typehandlers-design.md; prior art PRs #117 and #162 (the announced- attribute tier) remain as the static-dispatch optimization, redone on the plain-data model.
mgravell
added a commit
that referenced
this pull request
Aug 25, 2026
…shapes Both external PRs propose the same fixture - module-scoped registration of a handler for a custom type, then three call sites exercising it - so it is a usable statement of intent. Translated verbatim (same call sites, type names and member names) into the registration and handler contract this PR ships, so the two can be compared line for line. All three generate through the handler: the read of a handled member on a mapped row type, the write of a handled member on an *anonymous* parameter type (shape witness intact - a case the existing fixture did not cover), and the output parameter read back via Parse. The handler is emitted once as a static readonly field shared by every factory in the file. That was #162's improvement over #117, which allocated at each parameter and each column read; static readonly also avoids the `??=` its review flagged as non-atomic. Two spellings differ, both deliberate: the registration is typeof-based rather than generic, because .NET Framework's GetCustomAttributes throws for the whole call when an assembly carries a generic attribute; and SetValueCore/Parse are abstract rather than defaulted, so a handler that handles nothing is a compiler error instead of a silent pass-through. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
mgravell
added a commit
that referenced
this pull request
Aug 25, 2026
…ping one Reading #162 for the prior-art fixture surfaced a gap it had already solved and this PR had not: two handlers registered for one type, with the second silently ignored. That is the same silent-wrongness the declarative form exists to remove, so it gets a diagnostic - worded like DAP021, which is the identical situation for parameters, naming both handlers and which one loses. First registration wins, deterministically: module scope before assembly scope, source order within each, and the generator no longer carries the losers. An exact repeat stays quiet, since the outcome is identical either way; a dropped duplicate is not also graded for usability, so a registration that is both duplicated and unusable produces one message rather than two. Also rewords DAP053 and DAP054 as single sentences, per the auto-review: the analyzer-design rule wants no trailing period on a single sentence and one on a multi-sentence message, and single-sentence is what DAP050-052 already do. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
Member
|
Apologies this sat so long — a year with no review is not a reasonable wait, and that's on me. I've built this in #208, and I want to be specific about what came from here, because it's more than encouragement. Sharing one handler instance across a generated file, rather than constructing at every parameter and column read, is your improvement over #117 and it's what shipped — as a Your fixture is in #208 more or less verbatim, as One difference worth flagging: the registration is Your DAP050 for duplicate handler registrations was a real gap here, and reading this PR is what surfaced it — #208 was quietly dropping the second registration. That's now DAP055, worded like DAP021 since it's the same situation for parameters. Credit where it's due. I intend to close this in favour of #208 — with genuine thanks, and sorry again for the wait. |
mgravell
added a commit
that referenced
this pull request
Aug 25, 2026
…dlers (#208) * Type handlers: attribute registration, a new runtime API, and a shim for vanilla handlers The shipped [TypeHandler<TValue, THandler>] was never read by anything, so there is no behavior to preserve - only metadata. Both it and its TypeHandler<T> constraint are now [Obsolete(error)], naming the replacement; the types stay, so binary compatibility holds. The replacement is non-generic on purpose. .NET Framework's GetCustomAttributes throws NotSupportedException for the whole call when an assembly or type carries a generic attribute, poisoning unrelated reflection - the same wall protobuf-net hit, with the same typeof-based resolution: [module: TypeHandler(typeof(LocalDate), typeof(LocalDateHandler))] and deliberately not [Conditional("DEBUG")], unlike most of our attributes, so a package can declare handlers for the types it owns. The runtime contract is IDbValueHandler<T> (with a DbValueHandler<T> base for convenience): SetValue / SetNullValue - so a handler over a struct is never handed a null it cannot express - Parse for output parameters, and a Tokenize/Parse(reader, ordinal, token) pair mirroring how generated row factories already work. It is an interface rather than a base class because that is what makes existing Dapper handlers portable across the snk boundary. The runtime library cannot reference Dapper (a consumer may use Dapper *or* Dapper.StrongName, and referencing either loads both and splits the registry) - but the generator can see the consumer's symbols, and generated code compiles against whichever Dapper they actually have. So a registration naming a SqlMapper.ITypeHandler emits new VanillaTypeHandler<Money>(new MoneyHandler()) where VanillaTypeHandler<T> : IDbValueHandler<T> is generated into the same file: our interface, their implementation, a shim in between. Generated code dispatches on all four paths - AddParameters, the in-place UpdateParameters used by command caching, PostProcess for output parameters, and the row-factory read - with one static per registration, emitted only for registrations the code actually reached. A handler owns the parameter's type and size, so CanPrepare is cleared for those sites. DAP053 reports SqlMapper.AddTypeHandler in a Dapper.AOT compilation with no attribute registration for that type, naming the attribute to paste. It only sees calls in the compilation being built, so it narrows the silent-wrongness surface rather than closing it. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY * Type handlers: wire Tokenize, refuse unusable registrations, renumber to DAP054 Three things that stood between the declarative registration and being usable. **Tokenize is wired.** A handler's per-column decision was being thrown away - generated code always passed token 0 - so anything the protocol exists for (which shape did the provider actually give us?) had to be re-decided per row. The handler's token now travels in the row-factory's existing `state` channel: Tokenize allocates one int array per query, a second pass fills it by walking the token span (so it works the same whichever shape the mapping loop took, and does not care whether that loop advanced columnOffset), and Read indexes it positionally. Members sharing a handler share one case-arm. Costs one array per query and nothing per row. TypeHandlerProtocolTests pins that contract from the outside: a hand-written factory in the shape the generator emits, asserting Tokenize runs once per column no matter how many rows are read, that its token reaches every Parse, and that a factory reading a slice asks about its own column rather than column 0. Written against the public API, so it fails if either side drifts. **DAP055** reports a registration naming something generated code cannot use - implements neither contract, handles the wrong value type, no public parameterless constructor, abstract, static, inaccessible. These were skipped silently, which is exactly the failure mode declarative registration exists to remove; the message names the reason and says the registration is ignored. It is a warning, not an error: the code still compiles and runs, it just binds without the handler. **DAP053 becomes DAP054** here, because #206 takes DAP053 for the [UseRuntimeTypeHandlers]/PublishAot warning; ids are allocated so the two PRs can land in either order. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY * Don't advertise a member-scoped [TypeHandler] until it is read The attribute accepted a one-argument, member-scoped form that nothing consults - which is precisely the silent no-op the shipped generic attribute was obsoleted for. Targets are now assembly/module only, matching what the generator actually reads. Both halves of this are non-breaking to add back: widening AttributeUsage and adding a constructor. So the narrower scope stays a plausible future addition, just not a shipped promise. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY * Renumber to DAP053/DAP054: the runtime-bridge PR is closed, so its id is free The migration diagnostic becomes DAP053 and the unusable-handler refusal DAP054, keeping the library block contiguous now that nothing is going to take DAP053 for [UseRuntimeTypeHandlers]. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY * Prove the prior-art fixture: #117 and #162's scenarios, in this PR's shapes Both external PRs propose the same fixture - module-scoped registration of a handler for a custom type, then three call sites exercising it - so it is a usable statement of intent. Translated verbatim (same call sites, type names and member names) into the registration and handler contract this PR ships, so the two can be compared line for line. All three generate through the handler: the read of a handled member on a mapped row type, the write of a handled member on an *anonymous* parameter type (shape witness intact - a case the existing fixture did not cover), and the output parameter read back via Parse. The handler is emitted once as a static readonly field shared by every factory in the file. That was #162's improvement over #117, which allocated at each parameter and each column read; static readonly also avoids the `??=` its review flagged as non-atomic. Two spellings differ, both deliberate: the registration is typeof-based rather than generic, because .NET Framework's GetCustomAttributes throws for the whole call when an assembly carries a generic attribute; and SetValueCore/Parse are abstract rather than defaulted, so a handler that handles nothing is a compiler error instead of a silent pass-through. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY * DAP055: report conflicting type-handler registrations instead of dropping one Reading #162 for the prior-art fixture surfaced a gap it had already solved and this PR had not: two handlers registered for one type, with the second silently ignored. That is the same silent-wrongness the declarative form exists to remove, so it gets a diagnostic - worded like DAP021, which is the identical situation for parameters, naming both handlers and which one loses. First registration wins, deterministically: module scope before assembly scope, source order within each, and the generator no longer carries the losers. An exact repeat stays quiet, since the outcome is identical either way; a dropped duplicate is not also graded for usability, so a registration that is both duplicated and unusable produces one message rather than two. Also rewords DAP053 and DAP054 as single sentences, per the auto-review: the analyzer-design rule wants no trailing period on a single sentence and one on a multi-sentence message, and single-sentence is what DAP050-052 already do. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
mgravell
added a commit
that referenced
this pull request
Sep 11, 2026
`state-of-play.md` is the "read first after a break" page and was the most wrong thing in the repo: it still listed #206/#207/#208 as in flight with a landing order to follow, and knew nothing of #209-#216. It now says what is true - nothing of ours in flight, the four open PRs are all external and all awaiting a decision. It also absorbs the account of **why #206 was closed**, which was sitting unmerged on the `typehandler-registration-note` branch and existed nowhere in main: gating stopped the bridge emitting for consumers who did not opt in, but the read-side check still sat in `RowFactory.GetValue<T>` - the type-flexible arm of every mapped member - so everyone paid for a feature almost nobody would enable. The emission-time fix that was considered and rejected is recorded with it, since that is the part most likely to be re-proposed. Two things promoted, because they gate real work and were buried: - the behavioural harness is local-only and does **not** exist on this machine (no `aot-harness` branch, no SQL Server, Windows-shaped repack recipe), so no phase-3 round can be closed and 677/793 cannot be re-measured until it is rebuilt; - net48 is unverified since #216 and #214, both of which changed interceptor goldens. Corrections: #117 and #162 are closed, not awaiting a decision; the `type-handler` branch harvest was filed as "before #208 settles", and #208 has settled; diagnostic ids as actually shipped (DAP053-056, next free DAP057). `parity.md`: the `CommandDefinition` row still said "27 overloads, every one skipped silently" - #214 moved the `Type`+`CommandDefinition` combinations out, so it is 21 of the 22 in that bucket. Cites the report rather than restating a number, which is what #213 was for. Also: `typehandlers-design.md` never existed in main - it was a file on the closed #206 branch - leaving dangling links in three notes. Pointed at the note that did land, `typehandler-registration.md`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR represents an attempt to resolve issue #159.
The prior approach of the source generator (#117) inadvertently led to the generation of code that instantiated a new
TypeHandlerobject for each parameter or column read operation (new global::CustomClassTypeHandler().Read(...)). This could lead to unnecessary object allocations and a minor performance overhead, especially in high-volume scenarios.This PR introduces a
TypeHandlerInstanceRegistryand modifies the code generation to ensure that:__Handler1,__Handler2).