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 20 forks source link

DateOnly/ TimeOnly Support #58

Closed sydsutton closed 1 year ago

sydsutton commented 1 year ago

Microsoft.Data.SqlClient supports DateOnly and TimeOnly as of 5.1.0. See release notes: https://github.com/dotnet/SqlClient/blob/main/release-notes/5.1/5.1.0.md .

Would it be possible to support these as well? Maybe as simple as removing the "convertIfDateOnlyTimeOnly " function?

As always, thanks!

JordanMarr commented 1 year ago

Thanks for the suggestion.

SqlHydra has supported DateOnly and TimeOnly types since they first came out. Are they not working for you?

As far as actually using the native DateOnly/TimeOnly support in Microsoft.Data.SqlClient, I looked into doing that for the release of SqlHydra.Cli a few days ago. Issues I had:

But you are right that I can now simplify by at least removing the NET6_0_OR_GREATER check in convertIfDateOnlyTimeOnly since SqlHydra no longer supports .NET 5.

sydsutton commented 1 year ago

They're working, to a degree, but when using DateOnly's in my SqlHydra query, the resulting SQL code passes a DATETIME as a param instead of a DATE which slows the queries down significantly. Would it be advisable to require SqlClient >= 5.1.0 if using .NET >= 6?

sydsutton commented 1 year ago

Here's an example of generated SQL from a query using DateOnly's. ([Date] is of type DateOnly in the table). You can see the datetime param that's passed in. exec sp_executesql N'SELECT MAX([a].[Date]) FROM [dbo].[SomeTable] AS [a] WHERE ([a].[Date] >= @p0)',N'@p0 datetime',@p0='2023-06-05 00:00:00'

JordanMarr commented 1 year ago

Ah ok. In that case, I should be able to modify SqlHydra.Cli to detect NET7_0_OR_GREATER and add a ProviderDbType attribute to the generated DateOnly/TimeOnly record fields which will force the params to use the new parameter types.

JordanMarr commented 1 year ago

Just tried this: image

But I get an error: "No mapping exists from object type System.DateOnly to a known managed provider native type."

That's using the latest Microsoft.Data.SqlClient Version="5.1.1".

sydsutton commented 1 year ago

I created a test Console App with Micrsoft.Data.SqlClient 5.1.1 installed and did the same thing. Here's the repo: https://github.com/sydsutton/TestApp

In the console, you can see that it printed the correct types. It might be worth making sure that you have the correct version of SqlClient? image

JordanMarr commented 1 year ago

Ah ok, my test project targets both .NET 6 and .NET 7. I probably ran it using .NET 6 which uses the older version.

JordanMarr commented 1 year ago

SqlHydra.Cli v2.0.2 is now released. This version should create DateOnly as DbType.Date and TimeOnly as DbType.Time. However, it is still converting DateOnly back into a DateTime and TimeOnly into a TimeSpan because I'm trying to avoid breaking the other DB providers at this point. Please test and see if setting the parameter type alone is enough to fix your query performance. If not, then maybe we can look into getting rid of the conversion.

sydsutton commented 1 year ago

Thank you for working on it. A datetime param is still being passed in. Should I be using .NET 7?

JordanMarr commented 1 year ago

Did you regen your types?

sydsutton commented 1 year ago

No, no I did not. 'SELECT MAX([a].[Date]) FROM [dbo].[SomeTable] AS [a] WHERE ([a].[Date] >= @p0)',N'@p0 date',@p0='2023-06-09' I think it worked! Thanks so much for your help and responsiveness.

JordanMarr commented 1 year ago

Awesome! 🎉