-
Notifications
You must be signed in to change notification settings - Fork 289
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
Prevent connections to SQL 7.0 & 2000 #2839
base: main
Are you sure you want to change the base?
Conversation
Also add a test to verify failed/successful connections to different simulated server versions
While those systems aren't supported is there a reason to remove the ability to use it? I've encountered a live sql 2000 server recently and while i wasn't programming against it others may be, for example sql management studio will lose the ability to connect. |
That's a completely fair point, and part of the reason I raised the PR is to talk that through. As we start to merge the more sensitive classes relating to connections and transactions, there are code paths which we handle differently for downlevel clients. The options for this are:
The initial feedback was:
I've seen enough SQL 2008/R2/2012 servers recently to err on the side of caution when thinking about enforcing a minimum version of SQL 2014 across the board, but a baseline of SQL 2005 seems fair enough. I'm only proposing the removal of SQL 2000 support from the .NET Framework version, which still has the supported alternative of System.Data.SqlClient; the .NET Core library has never supported it. As far as SSMS support goes: I expect that people who still need to connect to downlevel versions of SQL Server would need an older version of SSMS. To put some context to the downstream changes this'll enable, I've got a second PR after this which covers the actual code changes. The diff of this is here. |
We aren't going to test EOL SQL Server versions in CI. Security compliance rules that out. I'm all for cleaning up old code. We should not connect to anything prior to TDS 7.4. CC: @saurabh500 and @cheenamalhotra as an FYI. |
Removing SQL 7.0/2000 support from TdsParser would remove quite a few differences between netcore and netfx and make merging the classes much easier. I hope this gets in soon :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sort of on the fence about this one. I can totally sympathize with @Wraith2 desire to leave support for old versions intact (eg, I work on a side project that targets a totally out-of-date version of nodejs so I can have the widest support I can). Yet, I also acknowledge that with corporate-sponsored open-source projects, we have to follow the compliance rules, and that generally means dropping support for EOL products. If there's no easy/approved way to test against EOL products, the code is going to wither, we'll get complaints about it not working, and won't be able to address the issues. So ultimately, I think we'll have to drop support for EOL SQL Server versions. As long as we don't pull old versions of MDS from Nuget, there'll still be an option for users of old versions of SQL Server.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
This prevents connections from the .NET Framework build of Microsoft.Data.SqlClient to SQL Server 7.0 and SQL Server 2000 (RTM & SP1.) Doing so aligns the .NET Framework build with the .NET Core build on all OSes: the library will no longer be able to connect to any version of SQL Server prior to 2005.
I've also included a test to verify failed/successful connections to different simulated server versions.
There's a bit of SQL Server 2000-specific code in the .NET Framework project, and this hasn't been removed yet. Since this could be considered a breaking change, I'm aiming simply to implement the functional block for the v6 release, ready for the then-redundant paths to be removed later.