Dapper icon indicating copy to clipboard operation
Dapper copied to clipboard

Runtime error when using DateOnly

Open musictopia2 opened this issue 4 years ago • 26 comments
trafficstars

I have a database that has a date property. However, the C# class is DateOnly since I only need the date, not the time. However, when I try to use a sql command and the c# class is DateOnly, the error is something like this The member WhatDate1 of type System.DateOnly cannot be used as a parameter value at Dapper.SqlMapper.LookupDbType(Type type, String name, Boolean demand, ITypeHandler& handler) in /_/Dapper/SqlMapper.cs:line 426

WhatDate1 is the field name

musictopia2 avatar Nov 21 '21 14:11 musictopia2

Here is what I think can fix the problem. Under static SqlMapper() in SQLMapper.cs file, if you add DateOnly and DateOnly?, that will fix the problem. I would also need TimeOnly and TimeOnly? to be added for cases where only time is needed.

musictopia2 avatar Nov 21 '21 14:11 musictopia2

There's also a branch (and possibly PR) that does this, but I was waiting on .NET 6 GA (which has now passed, so: yay).

So: what RDBMS/provider are you seeing this with, so I can validate?

mgravell avatar Nov 21 '21 18:11 mgravell

I am using the sql server one.

musictopia2 avatar Nov 21 '21 19:11 musictopia2

There are 2 official SQL Server drivers - System.Data.SqlClient, and Microsoft.Data.SqlClient; which are you using, and exactly what version?

mgravell avatar Nov 21 '21 20:11 mgravell

Sorry, I should have added: I don't mean to sound picky - my aim is to add the relevant tests and deploy this tomorrow, so anything I can discover to help shorten that: is good

On Sun, 21 Nov 2021, 19:15 musictopia2, @.***> wrote:

I am using the sql server one.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/DapperLib/Dapper/issues/1728#issuecomment-974876555, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMEANBB43LK6BFT3KB3UNFAMZANCNFSM5IPHCV7Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

mgravell avatar Nov 21 '21 20:11 mgravell

The version I am using is this one. <PackageReference Include="Microsoft.Data.SqlClient" Version="1.1.1" />

musictopia2 avatar Nov 21 '21 22:11 musictopia2

Microsoft.Data.SqlClient version 1.1.1 Tried to add the xml but that was deleted.

musictopia2 avatar Nov 21 '21 22:11 musictopia2

looks like they have that one up to 4.0.0. I can use that one though if that would help as well.

musictopia2 avatar Nov 21 '21 22:11 musictopia2

So: I updated my test harness, and: it simply doesn't work for either SqlClient; both give an exception like:

  Message: 
System.ArgumentException : No mapping exists from object type System.DateOnly to a known managed provider native type.

  Stack Trace: 
MetaType.GetMetaTypeFromValue(Type dataType, Object value, Boolean inferLen, Boolean streamAllowed)
MetaType.GetMetaTypeFromType(Type dataType)
SqlParameter.GetMetaTypeOnly()
SqlParameter.Validate(Int32 index, Boolean isCommandProc)
SqlCommand.BuildParamList(TdsParser parser, SqlParameterCollection parameters, Boolean includeReturnValue)
SqlCommand.BuildExecuteSql(CommandBehavior behavior, String commandText, SqlParameterCollection parameters, _SqlRPC& rpc)
SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean isAsync, Int32 timeout, Task& task, Boolean asyncWrite, Boolean inRetry, SqlDataReader ds, Boolean describeParameterEncryptionRequest)
SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry, String method)
SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method)
SqlCommand.ExecuteReader(CommandBehavior behavior)
SqlCommand.ExecuteDbDataReader(CommandBehavior behavior)

Do you have an example of it actually working with raw ADO.NET?

mgravell avatar Nov 22 '21 09:11 mgravell

(to be clear: I mean "the underlying ADO.NET provider doesn't work with DateOnly/TimeOnly", not "Dapper doesn't ...")

mgravell avatar Nov 22 '21 09:11 mgravell

Cross-referencing: https://github.com/dotnet/SqlClient/issues/1009

mgravell avatar Nov 22 '21 09:11 mgravell

Do you think there is a way for dapper to do some type of version so if dateonly was used, then it can somehow go to the external as datetime behind the scenes. Because not only its possible they may not ever fix it, but i found the newest version of microsofts version of the sql provider only works if a person uses ssl on a webpage which would not work if somebody had an intranet that is only for people on the local network with no connection to the internet.

musictopia2 avatar Nov 22 '21 18:11 musictopia2

I see an issue that was posted here https://github.com/dotnet/efcore/issues/24507 where they showed a converter a person can use as a temporary workaround in entity framework core. Can something like this be done to dapper? Would be disappointing if a person is forced to use entity framework core. If that is the case, then dapper would not work so well since without the dateonly support, businesses are blocked from even moving forward.

musictopia2 avatar Nov 22 '21 18:11 musictopia2

I don't want to introduce lots of magic here. In particular, the "does it work or does it need a shim" question would be vendor-specific. I see no good sides to adding some hidden translation here - it seems.like that is something that can, and should, be done at the caller: so they know, with confidence, what they are sending. Not doing too much unpredictable voodoo is an unwritten design goal of Dapper. So no, I'd rather not do that.

mgravell avatar Nov 23 '21 07:11 mgravell

Any suggestion of a workaround while waiting for vendors to fix them. What if no vendors ever fix them? would be disappointing to have the new datatypes but the catch of not being able to use them in any database.

musictopia2 avatar Nov 23 '21 13:11 musictopia2

What if no vendors ever fix them?

FYI the new DateOnly and TimeOnly types are supported by Microsoft.Data.Sqlite, MySql and Npgsql (see this tracking issue). I agree with @mgravell that it's not Dapper's job to do hidden conversions etc. - it's the ADO.NET provider's job to support these types.

roji avatar Nov 26 '21 10:11 roji

@roji didn't realize it was in Npgsql already; I can use you for my smoke-test, then :) but: as discussed above, it'll be direct pass-thru, no magic

mgravell avatar Nov 26 '21 11:11 mgravell

Yep, exactly... Let me know if you run into any trouble!

roji avatar Nov 26 '21 12:11 roji

I've just encountered this issue on SQLite as well. Column in the database is TEXT, as SQLite does not have a date/time/datetime equivalent. Date is stored in ISO8601 format (YYYY-MM-dd), which DateOnly.Parse() accepts.

I just wanted to share my quick work-around here, until the ADO provider for SQLite is fixed.

Solution that worked for me was to change the POCO like this:

internal record MyPOCO{
    internal string DateString { get; init; }
    internal DateOnly Date { 
        get
        {
            return DateOnly.Parse(this.DateString);
        }
    }
}

Then just change the SELECT statement to "SELECT Date AS DateString FROM MyTable"

The rest of my code can still use the Date property.

Rainmaker52 avatar Dec 09 '21 10:12 Rainmaker52

So, seeing as people are still bumping this, I thought I'd dust off the PR again

Updates:

  • SQLite v6 now works for parameters, but values come back as string, because: obviously why wouldn't it
  • Npgsql v6.0.1 now works for parameters, but values come back as DateTime/TimeSpan and there are some precision differences

So for SQLite, this works:

        [Fact]
        public void TestDateOnlyTimeOnly()
        {

            var now = DateTime.UtcNow;
            var args = new { day = DateOnly.FromDateTime(now), time = TimeOnly.FromDateTime(now) };
            var result = connection.QuerySingle("select @day as day, @time as time", args);

            var day = DateOnly.Parse(Assert.IsType<string>(result.day));
            var time = TimeOnly.Parse(Assert.IsType<string>(result.time));
            
            Assert.Equal(args.day, day);
            Assert.Equal(args.time, time);
        }

and for Postgresql, this needs rounding due to losing millisecond precision

        [Fact]
        public void TestDateOnlyTimeOnly()
        {

            var now = DateTime.UtcNow;
            var args = new { day = DateOnly.FromDateTime(now), time = TimeOnly.FromDateTime(now) };
            var result = connection.QuerySingle("select @day::date as day, @time::time as time", args);

            var day = DateOnly.FromDateTime(Assert.IsType<DateTime>(result.day));
            var time = TimeOnly.FromTimeSpan(Assert.IsType<TimeSpan>(result.time));

            Assert.Equal(args.day, day);
            Assert.Equal(Round(args.time), Round(time));

            static TimeOnly Round(TimeOnly value)
                => new TimeOnly(value.Hour, value.Minute, value.Second);
        }

@roji any comments on ^^^? am I doing it wrong? I thought time had microsecond precision?

Also: I need to put some more thoughts into how we handle return types here, i.e. should Dapper add translations between expected types (string and DateTime to DateOnly, string and TimeSpan to TimeOnly)?

mgravell avatar Dec 09 '21 12:12 mgravell

@mgravell re return types, yeah - that's expected; I didn't change the default return type for PostgreSQL date and time to avoid breaking people (it would also mean the driver would have returned different types across different .NET TFMs). So these still return DateTime and TimeSpan, respectively. You can use GetFieldValue<DateOnly> to get what you want - hopefully that's something Dapper can do. I this this would also solve the SQLite-side issue (where there's no database-side types at all for these).

Re the precision issue, can you provide more detail, ideally with an ADO.NET repro? Here's some ADO.NET code that shows what I think is correct behavior:

await using var conn = new NpgsqlConnection("Host=localhost;Username=test;Password=test");
await conn.OpenAsync();

var expected = TimeOnly.MaxValue;
await using var command = new NpgsqlCommand("SELECT @p", conn)
{
    Parameters = { new("p", expected) }
};
await using var reader = await command.ExecuteReaderAsync();
await reader.ReadAsync();

var actual = reader.GetFieldValue<TimeOnly>(0);
Console.WriteLine($"Actual:   {actual:o}");
Console.WriteLine($"Expected: {expected:o}");

The results are:

Actual:   23:59:59.9999990
Expected: 23:59:59.9999999

The discrepancy is normal, since .NET has tick precision (100ns) whereas PostgreSQL has microsecond precision (1000ns). Are you seeing something different?

roji avatar Dec 09 '21 14:12 roji

As for more thoughts on how to handle this; I don't know if this would be considered "doing too much", but I'd kind of like the idea of member attributes. Where you'd have something like this

    [DapperSerialize(Convert.ToString)]
    [DapperDeserialize(DateOnly.Parse)]
    internal string DateString { get; init; }

Where a static method needs to be passed in with exactly one argument. It's fairly trivial to write your own static method if you need something more complex.

Rainmaker52 avatar Dec 09 '21 14:12 Rainmaker52

@roji ah, ta; I changed the rounding to millis and it still passes, so: great!

Re the GetFieldValue<>() - that's a bigger change; I'll need to think; maybe this is a good time to code a list of types that should use that approach. I also need to fix SQLite for this scenario, so... fun!

mgravell avatar Dec 09 '21 20:12 mgravell

Yeah, makes sense. Let me know if you need anything else.

roji avatar Dec 09 '21 20:12 roji

Any workarounds for this?

Anarios avatar Mar 12 '22 22:03 Anarios

Unfortunately, there is still none unfortunately.

musictopia2 avatar Mar 12 '22 22:03 musictopia2

There are 2 official SQL Server drivers - System.Data.SqlClient, and Microsoft.Data.SqlClient; which are you using, and exactly what version?

For both do not work

zanyar3 avatar Nov 30 '22 07:11 zanyar3

FYI I am using "Microsoft.Data.SqlClient" Version="5.1.0". I have introduced the type handlers from https://github.com/DapperLib/Dapper/issues/1715#issuecomment-1179407655 and they are working great - any chance we can get them in Dapper directly?

celluj34 avatar Mar 01 '23 17:03 celluj34

Probably not relevant but i've come across this https://learn.microsoft.com/en-us/ef/core/what-is-new/ef-core-8.0/whatsnew#dateonlytimeonly-supported-on-sql-server

It sound like a "recent release of a [Microsoft.Data.SqlClient]" added features that made it possible for someone to add EF support for these types, maybe the change to Microsoft.Data.SqlClient may also allow dapper to support it with some work?

"For SQL Server, the recent release of a Microsoft.Data.SqlClient package targeting .NET 6 has allowed ErikEJ to add support for these types at the ADO.NET level. This in turn paved the way for support in EF8 for DateOnly and TimeOnly as properties in entity types."

buzz100 avatar Apr 14 '23 20:04 buzz100