Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Dapper call GetFieldValue<T> instead of GetValue #1644

Closed
mibiio opened this issue Apr 6, 2021 · 4 comments
Closed

Make Dapper call GetFieldValue<T> instead of GetValue #1644

mibiio opened this issue Apr 6, 2021 · 4 comments

Comments

@mibiio
Copy link

mibiio commented Apr 6, 2021

Hello,

I am using the NodaTime Plugin of Npgsql and it implements "NodaTime.Instant" as the Default Type for timestamptz.
In addition it also provides the possibility to get a "NodaTime.ZonedDateTime" by calling GetFieldValue<T>.

https://www.npgsql.org/doc/types/basic.html
https://www.npgsql.org/doc/types/nodatime.html

Currently when we defined a class like that:
public class TestData
{
public ZonedDateTime TheTime { get; set; }
}

and try to load some data into it by calling:

c.Query<TestData>(@"select now() as the_time;").FirstOrDefault()

An exception is raised:

Unable to cast object of type 'NodaTime.Instant' to type 'NodaTime.ZonedDateTime'.

When we use Npgsql directly and call GetFieldValue<T> the ZonedDateTime mapping works perfectly.
Is there any way to get this to work with Dapper beside modifying the Npgsql.NodaTime Plugin?

Thanks,
michael

@mgravell
Copy link
Member

mgravell commented Apr 6, 2021

The thing is: as a library, it isn't as easy as just changing to GetFieldValue<T>; a lot of the time, the value that comes back isn't what you think it is, and the library needs to do some munging / casting / converting; if we used GetField<T> throughout, then we start getting exceptions (mostly InvalidCastException) instead of just finding that the result isn't what we expected - unless we use GetFieldValue<object>(), which is pointless. So then we'd need to special case things instead, and need to start detecting both nodataime and the nodataime npgsql plugin. And from there, it starts getting more and more complex.

I'm not saying it can't be done; but: it really isn't as simple as just changing the API call.

@roji
Copy link

roji commented Apr 6, 2021

@mgravell just noting that GetFieldValue would eliminate the boxing of value types, which is another bonus.

@mgravell
Copy link
Member

mgravell commented Apr 6, 2021

@roji yep, I understand that; it is a mixed bag - but the range of different providers means that we need to be far more selective about when to prefer it. I'd actively prefer to use it. Maybe this is something for the V2 list

@mgravell
Copy link
Member

mgravell commented Jun 9, 2023

This should now be possible via #1910 - you'll need to call AddTypeMap, optionally specifying the DbType, but specifying true for the new parameter that enables this mode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants