Skip to content

SqlBatchCommand support CommandBehavior#4125

Open
campersau wants to merge 15 commits into
dotnet:mainfrom
campersau:GetColumnSchemaKeyInfo
Open

SqlBatchCommand support CommandBehavior#4125
campersau wants to merge 15 commits into
dotnet:mainfrom
campersau:GetColumnSchemaKeyInfo

Conversation

@campersau
Copy link
Copy Markdown

@campersau campersau commented Apr 1, 2026

Description

#1825 added the Batch APIs and also added a CommandBehavior property to SqlBatchCommand which was completly unused.
Note: SqlBatchCommand.CommandBehavior is a custom property in SqlClient which isn't in the DbBatchCommand

Also the CommandBehavior for ExecuteReader was ignored.

This PR enables the use of the SqlBatchCommand.CommandBehavior and also supports passing CommandBehavior to ExecuteReader.

Issues

Fixes #4083

Testing

Added tests for passing CommandBehavior.SchemaOnly | CommandBehavior.KeyInfo to SqlBatchCommand.CommandBehavior.
Added tests for passing CommandBehavior.CloseConnection to ExecuteReader.

@campersau campersau requested a review from a team as a code owner April 1, 2026 12:12
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Apr 1, 2026
Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBatch.cs Outdated
@mdaigle mdaigle moved this from To triage to In review in SqlClient Board Apr 1, 2026
@mdaigle mdaigle added this to the 7.1.0-preview1 milestone Apr 1, 2026
Comment thread src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Batch/BatchTests.cs Outdated
@campersau campersau force-pushed the GetColumnSchemaKeyInfo branch from 0629d0c to 19a768e Compare April 21, 2026 17:51
@campersau campersau force-pushed the GetColumnSchemaKeyInfo branch from 19a768e to fe1ee31 Compare April 21, 2026 17:53
cheenamalhotra
cheenamalhotra previously approved these changes May 7, 2026
Copy link
Copy Markdown
Member

@cheenamalhotra cheenamalhotra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cheenamalhotra
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026

Codecov Report

❌ Patch coverage is 90.62500% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.74%. Comparing base (c724554) to head (2ff5fe5).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
...SqlClient/src/Microsoft/Data/SqlClient/SqlBatch.cs 88.46% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4125      +/-   ##
==========================================
- Coverage   66.57%   64.74%   -1.83%     
==========================================
  Files         284      279       -5     
  Lines       43301    66225   +22924     
==========================================
+ Hits        28827    42879   +14052     
- Misses      14474    23346    +8872     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 64.74% <90.62%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@mdaigle mdaigle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I have some concerns about existing behavior that we have copied over without explanation and I would like to remove any unnecessary refactors.

Comment thread src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Batch/BatchTests.cs Outdated
Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs Outdated
Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBatch.cs Outdated
Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs Outdated
@github-project-automation github-project-automation Bot moved this from In review to In progress in SqlClient Board May 12, 2026
@mdaigle
Copy link
Copy Markdown
Contributor

mdaigle commented May 15, 2026

Thank you for making those changes. I'll do another review pass on Monday.

@mdaigle
Copy link
Copy Markdown
Contributor

mdaigle commented May 15, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Copy Markdown
Contributor

@mdaigle mdaigle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to think through the best way to expose things on SqlBatchCommand. It already accepts a CommandBehavior, and this PR now pipes that through. But certain behaviors are not meaningful. SingleResult, SingleRow, SequentialAccess, and CloseConnection all only apply at the top level (SqlBatch). I'm thinking we should add some validation to SqlBatchCommand that limits the CommandBehavior values we can pass in, or add some explanation to the public docs that most options will be no-ops.

Comment thread doc/snippets/Microsoft.Data.SqlClient/SqlBatch.xml Outdated
Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs Outdated
Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs Outdated
@campersau
Copy link
Copy Markdown
Author

Throwing exceptions is a breaking change, but not sure if somebody used it. So I am fine either way (validation or doc or both).

Copy link
Copy Markdown
Contributor

@mdaigle mdaigle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those changes look good. Let's note the restriction in a doc comment.

@campersau
Copy link
Copy Markdown
Author

I updated the doc comments.

@mdaigle
Copy link
Copy Markdown
Contributor

mdaigle commented May 20, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

@mdaigle
Copy link
Copy Markdown
Contributor

mdaigle commented May 26, 2026

@cheenamalhotra this is ready for another review

@mdaigle mdaigle requested a review from cheenamalhotra May 26, 2026 20:35
Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBatch.cs Outdated
@github-project-automation github-project-automation Bot moved this from In progress to Waiting for customer in SqlClient Board May 28, 2026
@cheenamalhotra cheenamalhotra added the Author attention needed PRs that require author to respond or make updates to PR. label May 28, 2026
@cheenamalhotra
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

@cheenamalhotra cheenamalhotra removed the Author attention needed PRs that require author to respond or make updates to PR. label May 29, 2026
@cheenamalhotra cheenamalhotra added the Public API 🆕 Issues/PRs that introduce new APIs to the driver. label May 29, 2026
@cheenamalhotra
Copy link
Copy Markdown
Member

Mentioning @Wraith2 if he's interesting in reviewing too as the primary contributor of SqlBatch support in the driver :)

@mdaigle mdaigle moved this from Waiting for customer to In review in SqlClient Board Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Public API 🆕 Issues/PRs that introduce new APIs to the driver.

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

SqlBatchCommand.CommandBehavior is unused

4 participants