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

[API Proposal]: Separating the common UDT types using Microsoft.SqlServer.Server NuGet package #66531

Closed
DavoudEshtehari opened this issue Mar 12, 2022 · 10 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Data.SqlClient
Milestone

Comments

@DavoudEshtehari
Copy link

Background and motivation

Microsoft.SqlServer.Server NuGet package has been introduced to enable the cross framework support of UDT types using Microsoft.Data.SqlClient to avoid the types conflicts.

Available Types:

Microsoft.SqlServer.Server.IBinarySerializer
Microsoft.SqlServer.Server.InvalidUdtException
Microsoft.SqlServer.Server.SqlFacetAttribute
Microsoft.SqlServer.Server.SqlFunctionAttribute
Microsoft.SqlServer.Server.SqlMethodAttribute
Microsoft.SqlServer.Server.SqlUserDefinedAggregateAttribute
Microsoft.SqlServer.Server.SqlUserDefinedTypeAttribute
Microsoft.SqlServer.Server.DataAccessKind
Microsoft.SqlServer.Server.SystemDataAccessKind
Microsoft.SqlServer.Server.Format

To complete the separation project, MSS should be consumed using System.Data.SqlClient. Consequently, dotnet/runtime should be updated with the new SDS version.

MSS has forwarded the available types to .NET Framework using target framework .NET Framework 4.6 and above. And other compatible target frameworks with .NET Standard 2.0 should use the introduced types in this library.

Queries

  • In my understanding, the dotnet/corefx configuration doesn't allow consuming a package, except for test projects. To proceed with this approach I need some assistance to consume the Microsoft.SqlServer.Server NuGet with System.Data.SqlClient project.

  • Consequent queries will be added to this thread by moving forward.

API Proposal

  1. The System.Data.SqlClient should consume Microsoft.SqlServer.Server package:
<PackageReference Include="Microsoft.SqlServer.Server" Version="1.0.0" />
  1. Removing the related types from SDS.

  2. Forwarding the introduced types:

[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.IBinarySerialize))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.InvalidUdtException))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.Format))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.SqlFacetAttribute))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.SqlFunctionAttribute))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.SqlMethodAttribute))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.SqlUserDefinedAggregateAttribute))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.SqlUserDefinedTypeAttribute))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.DataAccessKind))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.SystemDataAccessKind))]

API Usage

From the user's perspective, nothing will change.

Alternative Designs

N/A

Risks

  • Any library using SDS -like dotnet/runtime- should be updated with the latest version.
  • The version conflict on the customer's applications, will be fixed by updating to the new SDS and dotnet/runtime.
@DavoudEshtehari DavoudEshtehari added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 12, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Data.SqlClient untriaged New issue has not been triaged by the area owner labels Mar 12, 2022
@ghost
Copy link

ghost commented Mar 12, 2022

Tagging subscribers to this area: @cheenamalhotra, @David-Engel
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Microsoft.SqlServer.Server NuGet package has been introduced to enable the cross framework support of UDT types using Microsoft.Data.SqlClient to avoid the types conflicts.

Available Types:

Microsoft.SqlServer.Server.IBinarySerializer
Microsoft.SqlServer.Server.InvalidUdtException
Microsoft.SqlServer.Server.SqlFacetAttribute
Microsoft.SqlServer.Server.SqlFunctionAttribute
Microsoft.SqlServer.Server.SqlMethodAttribute
Microsoft.SqlServer.Server.SqlUserDefinedAggregateAttribute
Microsoft.SqlServer.Server.SqlUserDefinedTypeAttribute
Microsoft.SqlServer.Server.DataAccessKind
Microsoft.SqlServer.Server.SystemDataAccessKind
Microsoft.SqlServer.Server.Format

To complete the separation project, MSS should be consumed using System.Data.SqlClient. Consequently, dotnet/runtime should be updated with the new SDS version.

MSS has forwarded the available types to .NET Framework using target framework .NET Framework 4.6 and above. And other compatible target frameworks with .NET Standard 2.0 should use the introduced types in this library.

Queries

  • In my understanding, the dotnet/corefx configuration doesn't allow consuming a package, except for test projects. To proceed with this approach I need some assistance to consume the Microsoft.SqlServer.Server NuGet with System.Data.SqlClient project.

  • Consequent queries will be added to this thread by moving forward.

API Proposal

  1. The System.Data.SqlClient should consume Microsoft.SqlServer.Server package:
<PackageReference Include="Microsoft.SqlServer.Server" Version="1.0.0" />
  1. Removing the related types from SDS.

  2. Forwarding the introduced types:

[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.IBinarySerialize))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.InvalidUdtException))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.Format))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.SqlFacetAttribute))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.SqlFunctionAttribute))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.SqlMethodAttribute))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.SqlUserDefinedAggregateAttribute))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.SqlUserDefinedTypeAttribute))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.DataAccessKind))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(Microsoft.SqlServer.Server.SystemDataAccessKind))]

API Usage

From the user's perspective, nothing will change.

Alternative Designs

N/A

Risks

  • Any library using SDS -like dotnet/runtime- should be updated with the latest version.
  • The version conflict on the customer's applications, will be fixed by updating to the new SDS and dotnet/runtime.
Author: DavoudEshtehari
Assignees: -
Labels:

api-suggestion, area-System.Data.SqlClient, untriaged

Milestone: -

@teo-tsirpanis
Copy link
Contributor

teo-tsirpanis commented Mar 14, 2022

Isn't System.Data.SqlClient archived? It's not even in dotnet/runtime anymore; my understanding is that it exists in corefx only for .NET Core 3.1 servicing.

@Wraith2
Copy link
Contributor

Wraith2 commented Mar 14, 2022

In addition to @teo-tsirpanis 's points.

  • How would this ship? System.Data.SqlClient isn't shipped as part of net 5 or later so it could only affect those systems if it were published as an out of box package.
  • Even if it did ship it could cause a problem because S.D.SqlClient is generally assumed to be the in-box version but it would now have an external dependency that wasn't part f the runtime.
  • What benefit does this have?

@DavoudEshtehari
Copy link
Author

DavoudEshtehari commented Mar 14, 2022

@teo-tsirpanis
SDS is closed to expansion but it's still open for critical fixes. Microsoft.Data.SqlClient is the replacement of SDS.

@Wraith2
The MSS NuGet has been introduced after a long discussion among MS teams, that has been started a year ago. This is only part of this project that is approved to do by MS. Any active .NET Core/.NET versions should be updated.

It's just a minor dependency to unify any local copy of the UDT types to avoid the type conflict issue. It would cause some issues on porting time which could be fixed by updating the runtime.

It will unblock the team that is working on bringing spatial types to .NET Core/.NET (Microsoft.SqlServer.Types).

@teo-tsirpanis
Copy link
Contributor

Why not just bring spatial types only to MDS and not SDS?

@Wraith2
Copy link
Contributor

Wraith2 commented Mar 14, 2022

It will unblock the team that is working on bringing spatial types to .NET Core/.NET (Microsoft.SqlServer.Types).

That's news. It should probably be something that is mentioned in the issue on that topic. dotnet/SqlClient#30

The MSS NuGet has been introduced after a long discussion among MS teams, that has been started a year ago. This is only part of this project that is approved to do by MS. Any active .NET Core/.NET versions should be updated.

Well that's the part I don't understand. S.D.SqlClient isn't shipping anymore, it's archived. I'm not opposed to it i just don't understand how it'll work.

@David-Engel
Copy link
Contributor

Spatial types are based on UDTs, which are based on the classes defined in Microsoft.SqlServer.Server (MSS). MSS used to live in System.Data.dll in netfx and was subsequently brought into System.Data.SqlClient.dll in netcore. Microsoft.Data.SqlClient (MDS) needs to reference the MSS classes to support UDTs (in a backwards compatible manner) and thus spatial types (being brought forward to .NET/Core by another team in Microsoft). MDS can't take a dependency on SDS since MDS' goal is to replace SDS. MDS can't include the MSS packages in its own assembly without conflicting with SDS (for applications that still have a dependency (direct or indirect) on both MDS and SDS).

We discussed this at length internally and with the .NET team and the solution is to pull MSS out into its own package (done and released), have both SDS and MDS reference it, and type forward the classes in SDS to MSS.

We realize this means servicing SDS (which was last done 6 months ago). What we are finding is, it's not straightforward to figure out how to update SDS to be dependent on MSS, an external NuGet package.

Once a version of SDS is released that depends on MSS, users have a path forward, avoiding namespace conflicts when MDS also has a dependency on MSS.

David

@jkotas
Copy link
Member

jkotas commented Mar 15, 2022

We realize this means servicing SDS (which was last done 6 months ago).

We try hard to avoid moving types around and introducing new dependencies in servicing. It has a high chance of introducing breaks. We typically do changes like that in major releases only.

@David-Engel
Copy link
Contributor

We realize this means servicing SDS (which was last done 6 months ago).

We try hard to avoid moving types around and introducing new dependencies in servicing. It has a high chance of introducing breaks. We typically do changes like that in major releases only.

@jkotas I completely understand and agree in principal. However, given the current status of SDS (it doesn't even exist in the current runtime code), what are our options to avoid namespace conflicts between MDS and SDS once MDS depends on MSS?

In addition to this change, could we bump the major version on SDS? As a bonus, this would make support happy, as there is currently a lot of confusion since the SDS version, 4.8.3, is so similar to the .NET Framework version.

@jkotas
Copy link
Member

jkotas commented Mar 25, 2022

what are our options to avoid namespace conflicts between MDS and SDS once MDS depends on MSS?

The alternative that does not require major update of SDS can look like this:

  1. Reiterate that projects need to switch from SDS to MDS to get new features.
  2. If somebody absolutely has to reference both SDS and MDS in the same project, provide a recipe for how to reference SDS using C# alias to resolve type name conflicts.

I believe that this is in line with roadmap published at https://devblogs.microsoft.com/dotnet/introducing-the-new-microsoftdatasqlclient

@David-Engel David-Engel added this to the Future milestone Sep 6, 2022
@David-Engel David-Engel removed the untriaged New issue has not been triaged by the area owner label Sep 6, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Data.SqlClient
Projects
None yet
Development

No branches or pull requests

5 participants