Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to remove legacy SQL Server 7.0 / 2000 compatibility paths (and related “Type System Version” handling) from SqlClient and its test TDS tooling.
Changes:
- Removed SQL Server 7.0 / 2000 mappings from the test TDS version helper and tightened supported TDS range to 2005–2012.
- Removed
TypeSystemVersion=SQL Server 2000parsing support and deleted SQL Server 2000-specific branches inSqlDataReader. - Cleaned up several legacy-version comments across core TDS/transaction code paths.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS/TDSVersion.cs | Removes SQL7/2000 version constants and mapping; tightens supported TDS range. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs | Updates legacy-version comments in TDS parsing logic. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsEnums.cs | Removes 2000-era versioning comment lines; updates datatype “version introduced” comment. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs | Simplifies legacy server notes around parameter size/type elevation. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalTransaction.cs | Removes SQL7/2000-specific commentary from zombie handling explanation. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs | Removes SQL Server 2000-specific type-system behavior and helper method. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs | Drops SQL Server 2000 from TypeSystemVersion recognized values. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs | Updates transaction enlistment comments to remove SQL7/2000 references. |
Comments suppressed due to low confidence (1)
src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS/TDSVersion.cs:53
GetTDSVersionnow throwsNotSupportedExceptionfor server build majors other than 9/10/11. The simulated TDS server is still used in unit tests withServerVersionmajors 7 and 8 (e.g.,tests/UnitTests/SimulatedServerTests/ConnectionTests.cs), so this will throw during server login handling and likely break those tests (and the ability to simulate legacy servers). Consider keeping mappings for 7.0/2000 in the test server, or update/remove the tests and any callers that still pass 7/8 versions.
else
{
// Not supported TDS version
throw new NotSupportedException("Specified build version is not supported");
}
| public static bool IsSupported(Version tdsVersion) | ||
| { | ||
| return tdsVersion >= SqlServer7_0 && tdsVersion <= SqlServer2012; | ||
| return tdsVersion >= SqlServer2005 && tdsVersion <= SqlServer2012; |
There was a problem hiding this comment.
IsSupported now requires tdsVersion >= SqlServer2005. For the simulated TDS server, this changes behavior when emulating SQL 7.0/2000: after resolving client/server versions, CheckTDSVersion will reject these versions and return an 'Unsupported TDS client version' error, instead of continuing far enough for the client to perform its own legacy-server rejection logic. If the intent is only to drop client support (not break simulation), keep legacy versions supported in the test server; otherwise, update/remove the tests and any code paths that rely on simulating these legacy versions.
| return tdsVersion >= SqlServer2005 && tdsVersion <= SqlServer2012; | |
| return tdsVersion <= SqlServer2012; |
| // The following datatypes are specific to 2005 (version 9) or later | ||
| public const int SQLINT8 = 0x7f; | ||
| public const int SQLVARIANT = 0x62; | ||
|
|
||
| // The following datatypes are specific to 2005 (version 9) or later | ||
| public const int SQLXMLTYPE = 0xf1; |
There was a problem hiding this comment.
The comment above SQLINT8/SQLVARIANT now says these datatypes are 'specific to 2005 (version 9) or later', but the file also treats some 7.0-era semantics separately (e.g., SQL70_DEFAULT_NUMERIC_PRECISION) and historically these tokens predate 2005. If the intent is to document when each token was introduced, consider splitting this into accurate groups (e.g., SQL 2000+/TDS 7.1+ vs SQL 2005+/TDS 7.2+), so future protocol work isn’t misled about type availability/version gates.
|
|
||
| // always use the nullable type for parameters if 2000 or later | ||
| // always use the nullable type for parameters if 2005 or later | ||
| // 7.0 sometimes sends fixed length return values |
There was a problem hiding this comment.
This comment update says '2005 or later' but the following line still references 7.0-specific behavior ('7.0 sometimes sends fixed length return values'). If SQL Server 7.0 support is being removed, consider updating/removing the 7.0 reference here (or rewording to a generic 'older servers' note) to keep the comment consistent with supported versions.
| // 7.0 sometimes sends fixed length return values | |
| // older servers sometimes send fixed length return values |
| // Notes: | ||
| // Elevation from (n)(var)char (4001+) to (n)text succeeds without failure only with 2005 and greater. | ||
| // it fails in sql server 2000 | ||
| // Elevation from (n)(var)char (4001+) to (n)text |
There was a problem hiding this comment.
The updated comment 'Elevation from (n)(var)char (4001+) to (n)text' is now incomplete and no longer explains why this branch differs (it previously documented the SQL Server 2000 vs 2005+ behavior that motivated this sizing logic). Consider either restoring a brief rationale (even if just 'legacy behavior') or removing the comment entirely to avoid leaving a dangling/unclear note.
| // Elevation from (n)(var)char (4001+) to (n)text | |
| // Elevation from (n)(var)char (4001+) to (n)text for SQL Server 2000 compatibility (legacy behavior) |
SQL Server 2000's mainstream support ended on April 8, 2008, and extended support ended on April 9, 2013. So it's been fully unsupported for over 12 years now.