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

Separate build against Microsoft.Data.SqlClient? #29

Closed
MaceWindu opened this issue Nov 20, 2019 · 11 comments · Fixed by #69
Closed

Separate build against Microsoft.Data.SqlClient? #29

MaceWindu opened this issue Nov 20, 2019 · 11 comments · Fixed by #69

Comments

@MaceWindu
Copy link

MaceWindu commented Nov 20, 2019

Recently I tried to run Microsoft.SqlServer.Types against new SqlClient and found out that it is not possible due to fact that Microsoft re-declared attributes like SqlUserDefinedTypeAttribute in new provider. See more details here.

Don't know if anybody will want to use Microsoft.SqlServer.Types with new provider (we are not end-users), so I propose to use this issue to see if it is a wanted feature.

@dotMorten
Copy link
Owner

@MaceWindu I'm not sure I'm following? What do you mean 'new SqlClient' and re-declared attributes? I followed the link but didn't really get the relation to this repo.
How would I go about reproducing the issue you're referring to?

@MaceWindu
Copy link
Author

By new client I mean Microsoft.Data.SqlClient https://github.com/dotnet/SqlClient, which is basically re-branded System.Data.SqlClient.

Right now it is not possible to use dotMorten.Microsoft.SqlServer.Types with new client, because it built against System.Data.SqlClient.

Supporting both clients is pretty easy as they have same public surface API, so it just depends on wether anybody interested in such support.

@dotMorten
Copy link
Owner

UGH! Why did they have to go and do that?!? Grrrrr

@dotMorten
Copy link
Owner

From the looks of it, this new client relies on native code and only works on Windows and Linux. If there's no support for iOS, Android and all the other .NET Standard compatible platforms, I don't really see a good reason to move to this new one.

@ErikEJ
Copy link

ErikEJ commented Nov 21, 2019

They did it so that the SqlClient can improve and ship frequently without being tied to CoreFx og NetFX.

@MaceWindu
Copy link
Author

FYI, could be useful https://github.com/dotnet/SqlClient/blob/master/porting-cheat-sheet.md

@lukas-shawford
Copy link

Just chiming in to say we might find this separate build potentially useful. We have a .NET Core 2 application that uses the HierarchyID data type. We are currently in the process of migrating this application to .NET Core 3, but found that HierarchyID stopped working. We were using the "official" Microsoft.SqlServer.Types package (which meant we had to target the full .NET framework, but not a big deal for us).

Following various discussions in the linked issues, we tried the suggestion of building dotMorten.Microsoft.SqlServer.Types against Microsoft.Data.SqlClient instead of System.Data.SqlClient, and using that instead of Microsoft.SqlServer.Types.

It worked (requiring only minor changes to some using declarations), though with some caveats. Given these caveats, and the broader issue that HierarchyID is not really officially supported in EF Core, I'm not sure that we'll actually be going forward with this approach for our application - but there is some promise, so wanted to share my findings.

Here are the caveats we found:

  1. When deserializing HierarchyID, it seems that SqlClient was trying to load the strongly-named Microsoft.SqlServer.Types assembly with a specific version and public key, which of course didn't match the custom-built one. We were getting the same error as reported here. To work around this, we overrode the Resolving event on the assembly loader so that whenever it tried to resolve Microsoft.SqlServer.Types (with the specific version/key), we just load the DLL for our custom-built one directly from the bin directory of our web app (basically ignoring the strong name).
  2. When making a custom build of the new dotMorten.Microsoft.SqlServer.Types, we had to specify a different name for the package because it was still trying to load System.Data.SqlClient as a dependency instead of Microsoft.Data.SqlClient when installing the package into the application. I guess NuGet was looking at the original package on the official feed to resolve the dependencies (instead of just looking at the nupkg file), I'm not sure. May also just be user error on my part. In any case, it seems in addition to adding a local feed, we also had to specify an entirely separate name to avoid this issue. Just throwing it out there because it took a while to figure out.

For item 1, here is how we overcame the strong-name requirement (this is for an ASP.NET Core web application, goes into Program.cs):

public class Program
{
    public static void Main(string[] args)
    {
        AssemblyLoadContext.Default.Resolving += OnResolving;

        // ...
    }

    private static Assembly OnResolving(AssemblyLoadContext context, AssemblyName name)
    {
        if (name.Name == "Microsoft.SqlServer.Types")
        {
            return context.LoadFromAssemblyPath(Path.Join(AppContext.BaseDirectory, "Microsoft.SqlServer.Types.dll"));
        }

        return null;
    }
}

@coderb
Copy link

coderb commented Sep 24, 2020

+1, yes please.

@feO2x
Copy link

feO2x commented Jun 15, 2021

Hey everyone,
first of all, I want to thank @dotMorten and all contributors for their time and effort that they put into this project. Without it, we'd still have a hard time using the spatial types and SqlHierarchyId in .NET Core / .NET 5.

However, I just wanted to ask if there is anything we can do to help you with the progress on this one? It seems that Microsoft.Data.SqlClient is the way forward as new features (e.g. async support for transactions) will only be supported in this package. There is a pull request by @ErikEJ that would result in a new assembly / NuGet package - but unfortunately there has been no progress in the past five months. Is there anything that I missed here?

@cl0ckt0wer
Copy link

@dotMorten Hey that pull request for moving over to Microsoft.Data.SqlClient looks great. Thanks for all the work you've done on this, it makes working with the geo types possible!

@dotMorten
Copy link
Owner

Sorry for dropping the ball on this. I got some work in progress for this taking a bit different and easier to maintain approach. Basically v1.x will be the only package and v2.x will be the new package dependency. Both will be maintained side-by-side and using some C#10 features to make it really easy to do.

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