Dapper
Dapper copied to clipboard
DapperLib
Custom type handlers for enums are ignored
I have an enum that is serialized to the DB using a single letter code. I tried adding a custom type handler for the enum to handle the conversion but dapper seems to ignore the custom type handler.
Given the example code I expected to see the values; ID = 10, Type = B ID = 1, Type = Foo
What I actually got was: ID = 1, Type = 2 DataException, Error parsing column 1 (Type=F - String)
void Main()
{
SqlMapper.AddTypeHandler(new ItemTypeHandler());
using (var connection = new SqlConnection("Server=(local); Database=tempdb; Integrated Security=SSPI")) {
connection.Open();
connection.Query ("SELECT @ID AS ID, @Type AS Type", new Item {ID = 10, Type = ItemType.Bar}).Dump();
connection.Query<Item> ("SELECT 1 AS ID, 'F' AS Type").Dump();
}
}
class ItemTypeHandler : SqlMapper.TypeHandler<ItemType>
{
public override ItemType Parse(object value)
{
var c = ((string) value) [0];
switch (c) {
case 'F': return ItemType.Foo;
case 'B': return ItemType.Bar;
default: throw new ArgumentOutOfRangeException();
}
}
public override void SetValue(IDbDataParameter p, ItemType value)
{
p.DbType = DbType.AnsiStringFixedLength;
p.Size = 1;
if (value == ItemType.Foo)
p.Value = "F";
else if (value == ItemType.Bar)
p.Value = "B";
else
throw new ArgumentOutOfRangeException();
}
}
class Item
{
public long ID { get; set; }
public ItemType Type { get; set; }
}
enum ItemType
{
Foo = 1,
Bar = 2,
}
I've just hit exactly the same problem. It seems like SqlMapper handles enums differently than every other type and tries to map them with Enum.Parse even if a custom type handler is specified. I'm just about to delve into the source and see if I can see an easy fix - I'll report back if I find anything useful.
Urgh. I knew Dapper was optimised for performance but I didn't realise it literally emitted IL op codes to do value conversions. No chance I'm ever going to get to make enough sense of the source to offer up my own solution to this one so I'm hoping the devs will accept this as an enhancement.
I don't thing you need to touch the IL sections. It's mostly a case of changing the order of the type checks to look for a custom handler first. At the moment it checks if it's an enum before it checks for custom type handlers and thus never discovers the custom type handler.
My only concern is that I'm not sure what impact that will have on the rest of the system since similar type checks are in other places.
Backwards compatibility shouldn't be a concern since it has never worked in the past I doubt anyone will have custom handlers for primitive types such as enums.
Enums have quite a few nuances and special cases. I'm sympathetic to what you want to do here, but it isn't trivial - I'll need to have a look at it with a clear head.
On 10 April 2015 at 20:11, Chris Chilvers [email protected] wrote:
I don't thing you need to touch the IL sections. It's mostly a case of changing the order of the type checks to look for a custom handler first. At the moment it checks if it's an enum before it checks for custom type handlers and thus never discovers the custom type handler.
My only concern is that I'm not sure what impact that will have on the rest of the system since similar type checks are in other places.
Backwards compatibility shouldn't be a concern since it has never worked in the past I doubt anyone will have custom handlers for primitive types such as enums.
— Reply to this email directly or view it on GitHub https://github.com/StackExchange/dapper-dot-net/issues/259#issuecomment-91655115 .
Regards,
Marc
I would like to be able the same for the generic System.Enum type to change the serialization behaviour of all Enums (I'm using attributes to specify the letter used to serialize an Enum)
First check for a
SqlMapper.TypeHandler<ItemType>
then for a
SqlMapper.TypeHandler<Enum>
(the default Dapper behaviour could be specified that way too) Thanks for your interest on this feature @mgravell
@mgravell I worked on this a bit yesterday, and came up with this:
https://github.com/otac0n/dapper-dot-net/commit/817fa9593397b2fc5d2482159f21315e9f43a662
It works for everything I've tested, but I haven't written any unit tests for it.
I was testing it with this Type Handler:
public class StringEnumTypeHandler<T> : SqlMapper.TypeHandler<T> where T : struct, IConvertible
{
static StringEnumTypeHandler()
{
if (!typeof(T).IsEnum)
{
throw new ArgumentException("T must be an enumeration type");
}
}
public override T Parse(object value)
{
return (T)Enum.Parse(typeof(T), Convert.ToString(value));
}
public override void SetValue(IDbDataParameter parameter, T value)
{
parameter.Value = value.ToString();
parameter.DbType = DbType.AnsiString;
}
}
This doesn't support @vdaron's use case, however.
@vdaron, thanks for the reference. Didn't make an effort to check that this issue was already logged, my bad.
My use case is exactly the same as what @chilversc tried to achieve - single letter code in DB to represent an enum value. But it does not support @vdaron's "fall back" behavior request though.
@mgravell, my commit in issue #286 included relevant test code and "all green" tests.
We submitted a pull request to fix this two years ago, but it was never accepted.
A one-line fix is all it takes:
In public static DbType LookupDbType, at the top of the function, change
handler = null
to
typeHandlers.TryGetValue(type, out handler);
Still it's not fixed, and no feedback why the pull request was rejected :-( We'd love to be using the NuGet version of Dapper, but we need this fix because it's stopping TypeHandlers being used for any built-in type (and enums).
Ultimately, my reservation here is that it was not intended or designed that type-handlers should subvert the expected behavior of primitives; I haven't thought though the implication of every edge case of that; and I haven't checked what the impact of that is on the IL emitting code - does it still do "the right thing" (whatever the right thing is, which isn't necessarily obvious), in all cases?
I think the elegance and convenience of a 1-line change is all well and good, but it isn't necessarily that simple.
Have I been lax and tardy on bottoming out what that impact is? yes. that is valid criticism, and I take it squarely on the chin. But when faced with complex and nuanced unknowns, I tend to default towards the conservative.
On 28 September 2016 at 20:08, wmate [email protected] wrote:
+1 This. Is. The. Feature.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/StackExchange/dapper-dot-net/issues/259#issuecomment-250268121, or mute the thread https://github.com/notifications/unsubscribe-auth/AABDsPNsoeweGYdz7U35qT_-Bm1txKMWks5qursUgaJpZM4D0geM .
Regards,
Marc
@kbilsted my main concern would be the external 3rd-party code that could get an unexpected change in behaviour, i.e. a type handler that was previously doing nothing and had been forgotten about could start running.
As such this change would have to be a major release under semantic versioning.
There were further comments on the pull request #458 (which probably also needs the breaking change label).
At the moment, the simplest way is to wrap the enum in a simple struct with implicit conversions, then you can add a type handler on the wrapping struct.
@chilversc I don't think bumping the major version is an argument for not implementing the support.
More importantly, I did not realize there was a simple solution that did not require code changes in dapper. Would you kindly document the suggested approach in the readme.md file ?
It would be nice to be able to opt-into support for the behavior requested by this issue instead of having to wait for v2.0.
I'm impressed this is still an Issue, I had it last year and just ran into it once again...
Let's get my fork with fixes of this bug. I don't know why owner of Dapper not yet reviewed and merged it.
Any news on this? It's a kind of disappointment that enums can't be mapped even with my own type handler.
Hello Dapper community; I encountered this same problem as well, dealing with SAGE accounting and LoadMaster transportation databases. I wish we could have the linked PR merged; writing types with implicit string conversions is tedious, and I've had to defend this bizarre enum alternative in code reviews on multiple dev teams over the last few years. I have a fork of Dapper I use in personal projects due to things like this Enum TypeHandler support that are hard to customize using the Dapper API. Then I can merge any PR I like into it. But I can't typically use my private fork in production because it doesn't have the official backing of the Dapper community.
What can I do to satisfy the apprehensions of the Dapper leadership, so we can merge the solution?
Could hide it behind a feature flag, or add a new interface such as SqlEnumConverter so that unless you explicitly use the new features it won't affect existing code. Only donwside is a slightly more confusing API as you have to explain the flag / type and have people trying to use the existing interface by mistake.