Merged
Conversation
…lbak, CancelIgnoreFailure
…cachedMetaData, _reconnectionCompletionSource, _intrnalEndExecuteInitiated, CachingQueryMetadataPostponed, IsProviderRetriable, ProcParamsColIndex, PreSql2008ProcParamsNames, Sql2008ProcParamsNames, MetaData
…d renamed synchronous isSynchronous)
…ctionClosed (rewritten as ?.), NotifyDependency (rewritten as ?.)
…ion with Span<string>)
…and GetParameterCount
…g), GetResetOptionsString (made static, renamed to GetOptionsResetString), GetCommandText
… remove boolean "found" flag and just return directly)
…Notification, and netfx-only SqlNotificationContext
benrr101
added
the
Common Project 🚮
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates SqlCommand platform-specific code by removing separate SqlCommand.netfx.cs and SqlCommand.netcore.cs files and moving shared implementation into the main SqlCommand.cs file. The changes also create a new SqlCommand.Batch.cs partial file for batch-related functionality and update method signatures for consistency.
Key Changes:
- Eliminated platform-specific SqlCommand partial files (netfx and netcore)
- Consolidated shared implementation into main SqlCommand.cs
- Created SqlCommand.Batch.cs for batch RPC operations
- Renamed parameter
synchronoustoisSynchronousin WriteEndExecuteEvent calls
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| SqlCommand.cs | Added consolidated implementation including AsyncState, batch fields, and helper methods previously split across platform files |
| SqlCommand.Batch.cs | New partial containing batch RPC mode methods |
| SqlCommand.Xml.cs | Updated parameter name from synchronous to isSynchronous |
| SqlCommand.Scalar.cs | Updated parameter name from synchronous to isSynchronous |
| SqlCommand.Reader.cs | Updated parameter name and added explicit parameter for GetRPCObject and BuildParamList calls |
| SqlCommand.NonQuery.cs | Updated parameter name from synchronous to isSynchronous |
| SqlCommand.Encryption.cs | Added explicit forSpDescribeParameterEncryption parameter to GetRPCObject call |
| SqlCommand.netfx.cs | File deleted - content moved to shared files |
| SqlCommand.netcore.cs | File deleted - content moved to shared files |
| Microsoft.Data.SqlClient.csproj (both) | Removed references to deleted platform files, added reference to SqlCommand.Batch.cs |
| SqlUtil.cs | Added TODO comment for PendingBeginXXXExists method |
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs
Outdated
Show resolved
Hide resolved
* fAddSeparation => fAddSeparator * revert order or scale/precision * value => val * FIX BUG numChars GetActualScale => GetActualSize
…e made - will annotate.
benrr101
commented
Nov 3, 2025
Contributor
Author
benrr101
left a comment
benrr101
left a comment
There was a problem hiding this comment.
Adding annotations for bug fixes
paulmedynski
approved these changes
Nov 4, 2025
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs
Outdated
Show resolved
Hide resolved
priyankatiwari08
approved these changes
Nov 4, 2025
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3738 +/- ##
==========================================
- Coverage 76.78% 76.60% -0.19%
==========================================
Files 273 272 -1
Lines 44914 44189 -725
==========================================
- Hits 34487 33849 -638
+ Misses 10427 10340 -87
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mdaigle
pushed a commit
that referenced
this pull request
Nov 6, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
In the final installment of merging the SqlCommand class, I've mopped up the remaining methods and whatnot into the main SqlCommand partial and a new partial for batch RPC methods (not convinced this is the right idea, but otherwise the main partial becomes way too big, imo). This PR completes the SqlCommand merge (and leaves one class left to merge). As with the other PRs as part of this series, each commit merges a small collection of members. It is recommended to step through the commits one at a time to review.
This PR merges the following members:
Issues
More work towards #1261
Testing
CI should validate 😬