JordanMarr / SqlHydra

SqlHydra is a suite of NuGet packages for working with databases in F# including code generation tools and query expressions.
MIT License
212 stars 21 forks source link

MSSQL (Microsoft.Data.SqlClient): can't retrieve DateOnly #25

Closed ntwilson closed 1 year ago

ntwilson commented 1 year ago

I'm using .net 6, SQL Hydra v. 1.0.0, and Microsoft.Data.SqlClient v. 4.1.0 to connect to an MSSQL database that includes some DATE columns. SQL Hydra tries to use System.DateOnly for this, and it can write dates to the database just fine, but as soon as it tries to read a date from the database, it crashes in the SqlHydra-generated Reader code with

System.InvalidCastException: Unable to cast object of type 'System.DateTime' to type 'System.DateOnly'.
   at Microsoft.Data.SqlClient.SqlDataReader.GetFieldValueFromSqlBufferInternal[T](SqlBuffer data, _SqlMetaData metaData, Boolean isAsync)
   at Microsoft.Data.SqlClient.SqlDataReader.GetFieldValueInternal[T](Int32 i, Boolean isAsync)
   at Microsoft.Data.SqlClient.SqlDataReader.GetFieldValue[T](Int32 i)
   at MyProject.MyDatabase.dbo.get_MyColumn@537-2.Invoke(Int32 arg00) in C:\Git\MyRepo\MyProject\MyDatabase.fs:line 537
   at MyProject.MyDatabase.RequiredColumn`2.Read(FSharpOption`1 alias) in C:\Git\MyRepo\MyProject\MyDatabase.fs:line 11
   at MyProject.MyDatabase.dbo.MyTableReader.Read() in C:\Git\MyRepo\MyProject\MyDatabase.fs:line 542

It seems like GetFieldValue<DateOnly> doesn't work? Can we just use GetDateTime >> DateOnly.FromDateTime?

ntwilson commented 1 year ago

I'm sorry I stated that backwards. It can write a DateOnly to the database, but it can't read it from the database. I'm going to edit the original text.

JordanMarr commented 1 year ago

That is surprising as there are some .NET 6 tests that query records with DateOnly properties. (Or at least I thought there were...)

ntwilson commented 1 year ago

Here's my .toml contents if that helps

[general]
connection = "Server=MyServer;Database=MyDatabase;Trusted_Connection=True;TrustServerCertificate=True"
output = "MyDatabase.fs"
namespace = "MyProject.MyDatabase"
cli_mutable = false
[readers]
reader_type = "Microsoft.Data.SqlClient.SqlDataReader"
[filters]
include = []
exclude = []
ntwilson commented 1 year ago

FWIW, if I'm looking in the right places, I don't think there are integration tests for the DateOnly stuff. In AdventureWorksNet6.fs, DateOnly is only used in the Employee and EmployeeDepartmentHistory tables. However, if I search the whole repo for "Employee" in the F# code, it only shows up in the AdventureWorksNetX.fs files and TypeAnnotationTests.fs. It doesn't show up in QueryIntegrationTests.fs or SelectXyzTests.fs (which is where I think you're actually evaluating against a real database).

I could definitely benefit from somebody with more experience with this repository checking my facts on this.

JordanMarr commented 1 year ago

Yeah I did the same check and saw that there are no tests against that record. Adding a test now. An immediate workaround would be to change that project to net5 so that DateTime is used instead of DateOnly. But hopefully it won't take long to fix this.

JordanMarr commented 1 year ago

The test definitely failed! 😄

I think your GetDateTime >> DateOnly.FromDateTime is the way to go.

JordanMarr commented 1 year ago

That fix definitely works:

image

So the better interim fix would be for you to manually add that fix to your generated file (rather than reverting to net5) until I release the patch.

Also change it at the bottom of the generated file as well (for doing select on individual columns):

image

ntwilson commented 1 year ago

Thanks for the really fast turnaround on this, and for the awesome library for working with databases! Really a big help!

JordanMarr commented 1 year ago

It looks like DateOnly updates/inserts are not yet supported in Microsoft.Data.SqlClient. So it looks like SqlHydra.Query will need to handle conversion when creating parameters.

JordanMarr commented 1 year ago

SqlHydra.SqlServer v1.0.1 now generates a GetDateOnly extension method for .NET 6 and later. SqlHydra.Query v1.0.1 now converts DateOnly properties to DateTime when creating insert/update parameters.

Let me know if you have any issues.