FSharp.Data.SqlClient icon indicating copy to clipboard operation
FSharp.Data.SqlClient copied to clipboard

Switch to `Microsoft.Data.SqlClient`

Open daniellittledev opened this issue 4 years ago • 37 comments

Description

As Microsoft.Data.SqlClient is due to supersede/replace System.Data.SqlClient FSharp.Data.SqlClient should consider switching to Microsoft.Data.SqlClient.

daniellittledev avatar Feb 26 '20 01:02 daniellittledev

@daniellittledev thanks for creating this issue.

There is a comment regarding this: https://github.com/fsprojects/FSharp.Data.SqlClient/issues/337#issuecomment-507340577

My current idea is that until the new client library provides for feature that would make it useful to consumers of the TP which can't be achieved with existing client, the disruption for existing consumers is going to be problematic.

I don't have experience switching codebase to new client library, and looking for feedback on this.

@daniellittledev have you started using Microsoft.Data.SqlClient?

smoothdeveloper avatar Feb 26 '20 03:02 smoothdeveloper

@smoothdeveloper my initial interest for this change is that Microsoft.Data.SqlClient appears to have better support for dotnetcore, in particular this was the issue that got me thinking about it https://github.com/fsprojects/FSharp.Data.SqlClient/issues/373. I'd love to start using it but you always have to wait for your dependencies to support it first.

It may be worth taking the same approach as Microsoft.Data.SqlClient and fork or branch off somehow and provide two packages, one for the old SqlClient and one for the new.

daniellittledev avatar Feb 26 '20 03:02 daniellittledev

@daniellittledev do you know if this can achieved while keeping the same codebase and using compiler defines rather than forking the library implementation?

smoothdeveloper avatar Feb 26 '20 03:02 smoothdeveloper

@smoothdeveloper I would assume so, but you would probably need two separate proj files as the dependencies would differ. They could both reference the same code and sit side by side.

I'm unsure if there is an better/easier way to do it.

daniellittledev avatar Feb 26 '20 03:02 daniellittledev

@daniellittledev you can have conditionals in the same fsproj as well for the reference.

smoothdeveloper avatar Feb 26 '20 03:02 smoothdeveloper

@smoothdeveloper ah great, of course!

daniellittledev avatar Feb 26 '20 03:02 daniellittledev

@smoothdeveloper I don't know much about how to implement type providers or how much time I have at the moment but I'm open to helping out if I can.

daniellittledev avatar Feb 26 '20 05:02 daniellittledev

@daniellittledev I'd be happy to help with this investigation effort but can't commit time authoring it right now.

I can help you get acquainted with the specificities related to TP, but those won't have significant interference with giving a try at changing the reference / changing the code.

If you are willing to take on this experiment, I'll be able to review, and help meet the gap to eventually allow shipping two versions based on a branch with a full port.

If you are looking to use the library with new client, I think you/someone giving it a try is a singificant enabling step.

smoothdeveloper avatar Feb 26 '20 05:02 smoothdeveloper

@smoothdeveloper ok, point me to anything you think would be helpful, I'll see what I can do.

daniellittledev avatar Feb 26 '20 06:02 daniellittledev

@daniellittledev you'll first need to add the references, in paket.dependencies DesignTime group and paket.references next to https://github.com/fsprojects/FSharp.Data.SqlClient/tree/master/src/SqlClient.DesignTime

also add the normal nuget package reference in https://github.com/fsprojects/FSharp.Data.SqlClient/blob/master/src/SqlClient/SqlClient.fsproj

after this, you can try replacing all the namespaces to point to new types, and get the solution to build, then the test projects green.

If this can be reached, we will ponder on how to keep dual target for the time being.

If you make a fork, you can open an experimental PR here to allow others to peek into the work.

smoothdeveloper avatar Feb 26 '20 07:02 smoothdeveloper

My current idea is that until the new client library provides for feature that would make it useful to consumers of the TP which can't be achieved with existing client, the disruption for existing consumers is going to be problematic.

UTF-8 support and the fact that there is a problem with System.Data.SqlClient (#373) are good enough reasons IMO.

charlesroddie avatar Feb 29 '20 18:02 charlesroddie

I've been looking a bit more at this, so a bit of an update

There are at least two dependencies that fail in a netstandard type provider:

System.Configuration.ConfigurationManager

This package is a Reference Assembly in netstandard and is only supported in net4x. Therefore it can not be used in a netstandard version.

Microsoft.SqlServer.Types

This package also only supports the net4x but it also has a dependency on System.Data.SqlClient which means no support in netstandard. The geospatial types it contains are also superseded by the NetTopologySuite package which EF Core now uses for dotnetcore support.

I'm still trying to get a working version, but I'll comment again when I do.

daniellittledev avatar May 20 '20 10:05 daniellittledev

@daniellittledev thanks a lot for the update.

So the two mentionned dependencies and features related to it may be put under conditional compilation.

I'm also thinking integrating the work for supporting 'Microsoft.Data.SqlClient' will be supported by an extra assembly / nuget package.

We will figure a way to support both from same codebase / solution, I think it is the best as it doesn't interfere with current consumers relying on the library and want to time their switch.

Going another way would likely deter usage of this library due to lots of incurred changes/disruption if we planned to just replace.

smoothdeveloper avatar May 20 '20 15:05 smoothdeveloper

@smoothdeveloper I agree minimising disruption is a good approach. Although I've decided to get it working first then see how we can merge the two.

I've also discovered Microsoft.Data.SqlClient depends on System.Configuration.ConfigurationManager as well so it can't be removed. Anyhow, I do have an idea of what to try next, there might be a way to force the inclusion of the implementation assemblies assuming there is one for netstandard.

p.s. There's no such thing as a netcore type provider is there (or is it just netcore and netfx)?

daniellittledev avatar May 20 '20 22:05 daniellittledev

I think it is only netframework and netstandard that will be the targets.

smoothdeveloper avatar May 21 '20 05:05 smoothdeveloper

After searching around for way too long I haven't found any hope or lead for loading the non-reference assembly in netstandard. However, it looks like type providers support netcoreapp https://github.com/fsprojects/FSharp.TypeProviders.SDK/issues/328 which could get around the reference assembly issue.

But I haven't had much luck getting that to work either. The first problem is failing to load the DesignTime assembly from bin/typeproviders in the Test project.

'C:\Projects\FSharp.Data.SqlClient\bin\netcoreapp2.0\FSharp.Data.SqlClient.dll' reported an error: 
Assembly attribute 'TypeProviderAssemblyAttribute' refers to a designer assembly 'FSharp.Data.SqlClient.DesignTime.dll' which cannot be loaded or doesn't exist. 
Could not load file or assembly 'file:///C:\Projects\FSharp.Data.SqlClient\bin\netcoreapp2.0\FSharp.Data.SqlClient.DesignTime.dll' or one of its dependencies. 
The system cannot find the file specified.	SqlClient.Tests

If I manually copy the assembly into bin/netcoreapp2.0 it failed to load System.Runtime.

'C:\Projects\FSharp.Data.SqlClient\bin\netcoreapp2.0\FSharp.Data.SqlClient.dll' reported an error: 
Assembly attribute 'TypeProviderAssemblyAttribute' refers to a designer assembly 'FSharp.Data.SqlClient.DesignTime.dll' which cannot be loaded or doesn't exist. 
Could not load file or assembly 'System.Runtime, Version=4.2.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. 
The system cannot find the file specified.	SqlClient.Tests

I'm also trying to figure out how the Type Provider is loaded, or more specifically how can you verify which framework the tooling is running in. And how do type providers know which assemblies/package versions to load? There doesn't appear to be any other assemblies in the bin directory.

daniellittledev avatar May 24 '20 07:05 daniellittledev

FWIW there is an issue tracking the lack of netstandard support for Microsoft.SqlServer.Types, but it's filed under "Ideas for Future" so I don't think there will be movement on it in the short term: https://github.com/dotnet/SqlClient/issues/30.

Another possibility might be https://github.com/dotMorten/Microsoft.SqlServer.Types, but that library does not fully implement spatial operations for SqlGeography or SqlGeometry so I'm not sure it's a viable option.

samhanes avatar Aug 21 '20 15:08 samhanes

I'm also trying to figure out how the Type Provider is loaded, or more specifically how can you verify which framework the tooling is running in. And how do type providers know which assemblies/package versions to load? There doesn't appear to be any other assemblies in the bin directory.

@daniellittledev As I understand it this is determined by the compiler - if you want to make use of features available in the netfx version of the DTC, you will have to compile with MsBuild rather than dotnet build. All features work with netcore at runtime, though.

samhanes avatar Aug 21 '20 15:08 samhanes

@samhanes I mentioned NetTopologySuite which EF uses for spatial types https://github.com/fsprojects/FSharp.Data.SqlClient/issues/374#issuecomment-631378011

daniellittledev avatar Aug 24 '20 04:08 daniellittledev

@smoothdeveloper IMO this project needs to make the leap to full net5.0/netstandard2.0 compatibility.

People on legacy framework can still use older versions.

Switching to Microsoft.Data.SqlClient is a "must have" for us as we frequently use other Sql libraries, which have all switched.

I can't recall exactly what the issues are but trying to work with both Data.SqlClient libraries at the same time is not good.

We have been using this project for over 7 years. Unfortunately we are forced to migrate all our projects to Dapper.

jackfoxy avatar Mar 18 '21 19:03 jackfoxy

I might try and pick this back up again, I was never able to get past the runtime errors after getting it to compile against Microsoft.Data.SqlClient but it's probably time for another go. Although if anyone else wants to give it a shot as well, please feel free. Given that the move to dotnetcore is now in full swing this is a major risk to this project.

https://github.com/fsprojects/FSharp.Data.SqlClient/pull/375

daniellittledev avatar Mar 28 '21 23:03 daniellittledev

I'm trying to find information on if Type Providers, particularly the DesignTime project, can target netcoreapp instead of netstandard. Because the some assemblies load the reference assembly instead of the implementation assembly at runtime. Does anyone know what FSharp Type Provider versions support which TargetFrameworks?

daniellittledev avatar Mar 30 '21 04:03 daniellittledev

@cartermp can you help with @daniellittledev 's last question?

charlesroddie avatar Apr 06 '21 17:04 charlesroddie

I've written up more about what I've tried and the issue I'm having with loading reference assemblies here https://github.com/fsprojects/FSharp.Data.SqlClient/pull/375#issuecomment-817032528

daniellittledev avatar Apr 09 '21 23:04 daniellittledev

Maybe 2022 is the year we can switch to Microsoft.Data.SqlClient?

It seems that some of the libraries that previously caused trouble have been updated:

Microsoft.Data.SqlClient: https://www.nuget.org/packages/Microsoft.Data.SqlClient/5.0.0#supportedframeworks-body-tab

  • 5.0.0: netstandard2.0, netstandard2.1

System.Configuration.ConfigurationManager: https://www.nuget.org/packages/System.Configuration.ConfigurationManager/6.0.1#supportedframeworks-body-tab

  • 6.0.1: netstandard2.0

Microsoft.SqlServer.Types: https://www.nuget.org/packages/Microsoft.SqlServer.Types/160.900.6-rc0#supportedframeworks-body-tab

  • 160.900.6-rc0: netstandard2.1

Microsoft.SqlServer.TransactSql.ScriptDom: https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/150.4897.1#supportedframeworks-body-tab

  • 150.4897.1: netstandard2.0

cr3wdayt5p avatar Sep 24 '22 10:09 cr3wdayt5p

The way I am currently seeing it is we would compile / ship two versions of the type provider, likely compiled out of the same codebase, with different #define and maybe some refactoring to minimize the places it has an impact on.

It remains to see if the current code, even without this #define type of structure, would work with the new ADO.NET implementation.

@jackfoxy, @cr3wdayt5p, @charlesroddie, I am by no mean avoiding making this project take the leap to "spanking new" dotnet: I will carefully review and merge submitted PR that inch in that direction.

People on legacy framework can still use older versions.

As far as I am concerned, the project needs to keep working on legacy framework, even in the future; I think this is true for a significant share of consumers of this library, this is not what is blocking anything, AFAIU.

I am interested to know if people are actually using Microsoft.Data.SqlClient and what it does that System.Data.SqlClient doesn't.

smoothdeveloper avatar Oct 06 '22 12:10 smoothdeveloper

I am specifically interested in 'Microsoft.Data.SqlClient' as it supports Azure's Managed Identity feature directly in the connection string. Hence it would be possible to use Managed Identity (secretless apps) with 'FSharp.Data.SqlClient' without any other code changes.

It even has the potential to eliminate the need for different connection strings for design and compile time.

Example connection string – not supported by 'System.Data.SqlClient':

Server=demo.database.windows.net; Authentication=Active Directory Managed Identity; Encrypt=True; Database=testdb

cr3wdayt5p avatar Oct 06 '22 13:10 cr3wdayt5p

As far as I am concerned, the project needs to keep working on legacy framework, even in the future; I think this is true for a significant share of consumers of this library, this is not what is blocking anything, AFAIU.

netstandard2.0 deals with this issue. It supports legacy frameworks going back to 4.6.1, which was released 7 years ago and is EOL. There is something really silly about jumping through hoops to have net40 support and it strongly indicates that this project is going to be unmaintainable.

charlesroddie avatar Oct 06 '22 14:10 charlesroddie

@cr3wdayt5p interesting, thanks for the feedback, I see this as interesting factual motivator, are you interested to try out to "convert" this repository, in a prototype branch?

@charlesroddie, please refrain from characterizing things in negative fashion (about things being "really silly"), just out of your own bitterness and maybe oversight about underlying reasons, which you don't seem to put attention in a generous fashion here, hence likely to be ignored (unlike PR that don't break anything and make the project advance).

I don't see a reason not to drop .net 4.0 and target net 4.8 instead (if nobody opposes to make this the lowest framework for new releases), given VS 2022 doesn't support .net 4.0 anymore but:

  • I'd not take for granted that netstandard2.0 "deals with this? issue" in context of .net framework targeted projects and consumers of this library
  • there are probably more careful steps to do, than what you did in https://github.com/charlesroddie/FSharp.Data.SqlClient/pull/1 if we want to improve the status of this repository, without breaking stuff
  • your comment don't bring anything about showing how the provider codebase could, say switch (in a prototype), but ultimately, support both System.Data.SqlClient and Microsoft.Data.SqlClient

I think this has not much to do with this issue, so @charlesroddie, if you want to help with "really silly" things, you may open an issue, or make narrowly targetted PR, that changes .NET 4.0 to .NET 4.8 as the lowest supported framework.

Back to the issue:

The library in this repository cannot drop the support for System.Data.SqlClient, but we can definitely sketch things to support both System.Data.SqlClient and Microsoft.Data.SqlClient, the first step is to have a prototype supporting the later, and having some design discussions around supporting both.

I hope this helps. If there is enough interest, as people seems to put it in comments here, and people put efforts into it, I'm sure we'll see a prototype for supporting Microsoft.Data.SqlClient emerge.

smoothdeveloper avatar Oct 14 '22 20:10 smoothdeveloper

@smoothdeveloper I am still pretty new to F# and .NET in general. So I am probably not the right person to take on such a project. But I will be happy to give feedback and help test the changes.

Suggestion: How about making a clean break as a new major version, i.e. let v3.0.0 be .NET 6 and switch to Microsoft.Data.SqlClient only? This way all existing projects could continue to use the v2-branch (supporting older .NET versions and using System.Data.SqlClient).

Wouldn't it be simplere to support two "clean" versions rather than one convoluted project?

It would also make it easier for developers new to F#/.NET (like myself) to contribute to the project, e.g. I only have experience with .NET 6.0 so I don't know what is needed to support the older versions (and I don't have any motivation to learn that as all my own projects will be .NET 6.0 or higher).

cr3wdayt5p avatar Oct 17 '22 07:10 cr3wdayt5p