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

Implement SqlHierarchyId type for .NET Core #322

Closed
Ansssss opened this issue Nov 20, 2019 · 26 comments · Fixed by #1848
Closed

Implement SqlHierarchyId type for .NET Core #322

Ansssss opened this issue Nov 20, 2019 · 26 comments · Fixed by #1848
Labels
💡 Enhancement Issues that are feature requests for the drivers we maintain. 🔗 External Issue is in an external component

Comments

@Ansssss
Copy link

Ansssss commented Nov 20, 2019

Microsoft SQL Server has a hierarchyid data type defined since SQL Server 2008. In the .NET Framework there is a nuget to support using the SqlHierarchyId struct: Microsoft.SqlServer.Types. Unfortunately that nuget is built for the .NET Framework and not .NET Core. If you add that nuget to a .NET Core project, the following compiler warning is produced:

Warning NU1701 Package 'Microsoft.SqlServer.Types 14.0.1016.290' was restored using '.NETFramework,Version=v4.6.1, .NETFramework,Version=v4.6.2, .NETFramework,Version=v4.7, .NETFramework,Version=v4.7.1, .NETFramework,Version=v4.7.2, .NETFramework,Version=v4.8' instead of the project target framework '.NETStandard,Version=v2.0'. This package may not be fully compatible with your project.

There should be an implementation of SqlHierarchyId for .NET Core.

I tried using the Microsoft.Data.SqlClient nuget, but that does not have the SqlHierarchyId defined.
My current workaround is to use the dotMarten.Microsoft.SqlServer.Types nuget.

Related issues (which I don't think cover this same problem):

@cheenamalhotra cheenamalhotra added the 💡 Enhancement Issues that are feature requests for the drivers we maintain. label Nov 21, 2019
@cheenamalhotra
Copy link
Member

Thanks for filing a separate issue on this one.
We'll consider implementing it sometime in future.

@JohnyL
Copy link

JohnyL commented Jan 22, 2020

Are there any news on SqlHierarchyId?

@cheenamalhotra
Copy link
Member

@JohnyL

This isn't in our primary targets right now. We'll get to this when we have priority support addressed and have some room.

@brumlemann
Copy link

Any update on this? It is sorely needed

@yurart
Copy link

yurart commented Sep 2, 2020

Yeah, I also wait for this feature. Hope it will be implemented soon.

@alexey-kachalov
Copy link

Any plans to implement this?

@cheenamalhotra
Copy link
Member

cheenamalhotra commented Sep 4, 2020

Hi guys!

Have you tried using https://www.nuget.org/packages/Microsoft.SqlServer.DACFx that now supports Microsoft.Data.SqlClient (starting from v150.4769.1) and implements SqlHierarchyId for .NET Core, .NET Framework and .NET Standard?

If you check the "Package" defined in the .NET API reference docs,, this type in Microsoft.SqlServer.Types namespace is documented from this DACFx NuGet package.

It also seems to implement SqlGeography and SqlGeometry for all .NET platforms. (as requested in #30)

@ErikEJ
Copy link
Contributor

ErikEJ commented Sep 4, 2020

@cheenamalhotra I think it is not the cross platform package, but the old NetFX one.

@JohnyL
Copy link

JohnyL commented Sep 4, 2020

@cheenamalhotra I guess, if it'd be cross-platform (as ErikEJ said), you wouldn't bother mention it.

@cheenamalhotra
Copy link
Member

cheenamalhotra commented Sep 4, 2020

I think I missed out on internal APIs and only saw class reference.
So even though these classes are available in .NET Core, further APIs aren't. But the fact that this NuGet is actively updated, makes me think if there's a plan ahead for DACFx to support this fully in future.

@ErikEJ
Copy link
Contributor

ErikEJ commented Sep 4, 2020

Maybe the closed source DacFx team would be willing to share their plans in the open?

@ppasieka
Copy link

ppasieka commented Sep 8, 2020

There are few variants of the Microsoft.SqlServer.DACFx package:

  • Microsoft.SqlServer.DACFx -> very lightweight. A lot of things are missing. After decompiling, you can find a few "Not implemented" statements
  • Microsoft.SqlServer.DACFx.x64 -> that one actually looks good, but after running it against a database with existing data we are getting a lot of InvalidOperations exceptions on SqlGeography type. The same data are working fine with classical .net framework implementation of the SqlGeography

Anyway I'm getting this:

[NU1701] Package 'Microsoft.SqlServer.DacFx.x64 150.4826.1' was restored using '.NETFramework,Version=v4.6.1, .NETFramework,Version=v4.6.2, .NETFramework,Version=v4.7, .NETFramework,Version=v4.7.1, .NETFramework,Version=v4.7.2, .NETFramework,Version=v4.8' 
instead of the project target framework '.NETCoreApp,Version=v3.1'. 
This package may not be fully compatible with your project.

so as @ErikEJ mentioned the packages does not seem to be cross-platform

@vyrotek
Copy link

vyrotek commented Dec 11, 2020

I can't upvote this enough!

Not having .NET Core support for this and many other types from Microsoft.SqlServer.Types is a huge miss will most definitely impact .NET Core adoption.

The current state of things is disappointing and a really poor dev experience. Applications with these dependencies are limping along but only thanks to a handful of amazing OSS contributors.

@codeputer
Copy link

I'm confused as to why this isn't more of a priority under .Net 5? SQL Server is likely the most used data persistence service under .NET 5 - SQL Hierarchy Id is BUILD INTO FileTable, and FileStream functionality. To use those features means that we are limited to .net 4.8 ??? I would think that SQL types used in SQL server would be supported in SQL Server latest rev? I guess we could create a small micro service, where that service is 4.8 - but that would still require serialization to something that can be recognized in .NET 5.0. mmm... this just gets harder and harder doesn't it.

I could also advise the client that we drop support for SQL Server, as the language no longer supports the features.. but that seems like a huge hit for what I believe an effort by the team to support this type in .net core.

@David-Engel
Copy link
Contributor

@codeputer The problem is that SqlHierarchyId isn't really owned by SqlClient. It's owned by Microsoft.SqlServer.Types. We (the SqlClient team) are just kind of the proxy representing desire for support since the other library doesn't have a public repo and we do feed that info to the appropriate people. I also know that it is a priority in MS and that work has started to create a .NET Core/.NET 5 compatible Microsoft.SqlServer.Types package. I don't know any timelines, though.

@codeputer
Copy link

codeputer commented Mar 30, 2021 via email

@vyrotek
Copy link

vyrotek commented Mar 30, 2021

I also know that it is a priority in MS and that work has started to create a .NET Core/.NET 5 compatible Microsoft.SqlServer.Types package. I don't know any timelines, though.

That's really great to hear @David-Engel!

The lack of .NET Core support has impacted us in many places we didn't anticipate and we suspect it's holding back many other efforts. Most recently with Azure DevOps and not being able to build VS Database Projects on Linux pipeline instances. Some work was done in VS Code for DB Projects which appears to be cross-plat compatible but there appears to be hidden internals fetching the dlls required to make that work and no clear instructions on how to automate this.

@David-Engel David-Engel added the 🔗 External Issue is in an external component label Mar 30, 2021
@David-Engel
Copy link
Contributor

What am I to tell them, that SQL Client isn’t owned by SQL Server?

@codeputer I didn't say that nor did I mean to imply it. SqlClient is owned by the Azure Data organization, which encompasses many things, including SQL Server, Azure SQL offerings, and a lot of SQL-related tools and libraries.

I was responding to your comment in the context of this repo, which only hosts the SqlClient library. There are many repos for "SQL"-related things and this repo is not the forum for all SQL Server feedback. (That's aka.ms/sqlfeedback, btw.) This issue only remains open because we (the SqlClient team) feel like it impacts us and our users so we wanted to track it closely as a way to let our users know when it is available. I was just trying to clarify that SqlHierarchyId support for .NET 5 / .NET Core is out of our team's control, while also sharing the information we know - that it's being actively worked on.

As for what to tell your customers, I can sympathize. This is a public forum. The current state of things are what they are. We are trying to improve things and share what we can. They will have to make the best decisions for their business given the information at hand.

If they are looking for options, they can wait, stick with .NET Framework for a little while longer, or they can investigate dotMorten.Microsoft.SqlServer.Types, which is a .NET Standard, 3rd party implementation that includes SqlHierarchyId. (Note: I know nothing about that implementation but if it is API-equivalent and good-enough-for-them-now in terms of the specific functionality it supports, it could probably be swapped out for a Microsoft version later.)

@ErikEJ
Copy link
Contributor

ErikEJ commented Mar 31, 2021

@codeputer
Copy link

codeputer commented Mar 31, 2021 via email

@JohnyL
Copy link

JohnyL commented Mar 31, 2021

@codeputer We're all frustrated. I think that such fundamental thing must be realized in the early days of .NET Core. The priorities have changed, and this was not on the top of the drawer.

@Wraith2
Copy link
Contributor

Wraith2 commented Apr 21, 2021

If you just want hierarchyid it's specified in https://docs.microsoft.com/en-us/openspecs/sql_server_protocols/MS-SSCLRT/77460aa9-8c2f-4449-a65e-1d649ebd77fa and it's only a page so it shouldn't be that hard to implement a basic form. I'd attempt it but I've never seen or used the type and I wouldn't be able to write conformance tests even if I managed to deconstruct it correctly.

@ststeiger
Copy link

ststeiger commented Jul 27, 2021

2021 approaching 3/4 and still ... just speechless.
Well, sql-server geography&geometry support has always been shaky to phoney.
Try to make a polygon that spans across the equator, for example.
That whole thing as a .NET extension type.
If you need geometry&geography, you're better of with PostgreSQL and PostGis anyway.
OpenStreetMap has noticed that a long time ago.
Sad for FileStream, but the same could be said about it.
You can use PostgreSQL's 3rd party bfile implementation to replace FileStream.

@Wraith2
Copy link
Contributor

Wraith2 commented Jul 27, 2021

I don't use hierarchy id's so I don't have a need to add them but I could because the code is right here. If you do need them what's your reason for not adding them?

@klemmchr
Copy link

If you do need them what's your reason for not adding them?

Because it is expected that the official SqlClient has the same feature set support as Sql Server. I could accept that the support for Hierarchy Ids would be shady in some Ruby package or some other third party language. However, we're talking about the ecosystem that Microsoft promotes. How can it be that such features are missing, then are being added by some humble open source dev that are being picked up by the ef core team afterwards? This reads like a joke.

@Wraith2
Copy link
Contributor

Wraith2 commented Aug 16, 2021

Because it is expected that the official SqlClient has the same feature set support as Sql Server.

It doesn't. It seems best not to expect that.
As I found recently you can't even get the collation of a column through this driver without writing the SQL query to do it and I'd have thought was possible. I checked, it's not possible, I can add that functionality.

How can it be that such features are missing

The effort required to port the feature from the native implementation to a managed implementation that can be used cross platform doesn't seem to have been judged to be worth the time investment it requires. So even though the feature is important to you it doesn't seem to be important to the overall ecosystem. Having read through the spec I can see why no-one would want to implement it.

This reads like a joke.

Not really. It reads like an under-resourced ream not having time or developers to spare to to implement a relatively niche feature. At 53 upvotes at the time of writing it's pretty well liked for an issue in this quite low traffic repo so it might see some work eventually, we'll see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 Enhancement Issues that are feature requests for the drivers we maintain. 🔗 External Issue is in an external component
Projects
None yet
Development

Successfully merging a pull request may close this issue.