Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix SqlParameter with xml schema construction - #41008

Merged
cheenamalhotra merged 3 commits into
dotnet:masterfrom
Wraith2:sqlbug-mdsql-172
Sep 16, 2019
Merged

Fix SqlParameter with xml schema construction#41008
cheenamalhotra merged 3 commits into
dotnet:masterfrom
Wraith2:sqlbug-mdsql-172

Conversation

@Wraith2

@Wraith2 Wraith2 commented Sep 10, 2019

Copy link
Copy Markdown
Contributor

Fixes issue #41141 which is a bug in the corefx version not ported to Microsoft.Data.SqlClient yet and adds tests to verify the corrected and default behaviour of xml properties on SqParameter. The tests will run under CI.

This bug prevents the use of TableAdapters with autogenerated update commands

/cc reporter HSchwichtenberg, owners @David-Engel, @Gary-Zh, @cheenamalhotra and tracking @ErikEJ

Comment thread src/System.Data.SqlClient/src/System/Data/SqlClient/SqlParameter.cs Outdated
Comment thread src/System.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs Outdated
Comment thread src/System.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs Outdated
@Wraith2 Wraith2 changed the title Fix SqlParameter with xml schema construction WIP Fix SqlParameter with xml schema construction Sep 11, 2019
Comment thread src/System.Data.SqlClient/src/System/Data/SqlClient/SqlParameter.cs Outdated
@cheenamalhotra cheenamalhotra changed the title WIP Fix SqlParameter with xml schema construction Fix SqlParameter with xml schema construction Sep 12, 2019
@Wraith2

Wraith2 commented Sep 16, 2019

Copy link
Copy Markdown
Contributor Author

There's no chance of getting this into 3.0 but it can be ready for 3.1. Is there anything stopping this being merged now?

@cheenamalhotra

Copy link
Copy Markdown
Member

PR is ready to merge, I wanted to bring in @danmosemsft 's attention to figure out how and where we can backport the fix. I'm not sure of timelines for .NET Core but looking at impact on Asp.Net applications, 5.0 seemed a little further to me as well.

More information in dotnet/sqlclient#172

@danmoseley

Copy link
Copy Markdown

@cheenamalhotra as you know master is 5.0 and you'd want to merge there first. We haven't determined the exact bar for 3.0.x and 3.1 ports but it's likely the servicing bar we used for 2.2.

If you think this is a candidate, after merging into master please leave the issue open and set the milestone to 3.0.x. That way when we start the servicing system up again we'll find it.

@cheenamalhotra

Copy link
Copy Markdown
Member

@danmosemsft The issue is created in dotnet/SqlClient, should I move it here and mark milestone in that case? The issue has no impact in Microsoft.Data.SqlClient, only CoreFx is impacted.

@danmoseley

Copy link
Copy Markdown

@cheenamalhotra yes, if an issue only affects dotnet/corefx, let's please have it in dotnet/corefx as it doesn't make sense otherwise.

If an issue affects both, we probably need to figure out the best process

@cheenamalhotra
cheenamalhotra merged commit cd0576d into dotnet:master Sep 16, 2019
@Wraith2

Wraith2 commented Sep 18, 2019

Copy link
Copy Markdown
Contributor Author

@cheenamalhotra does this need to be re-opened and labelled 3.0.x so it can be considered for 3.1? otherwise we'll have to live with it broken until 5.0 which is at least a year away

cheenamalhotra added a commit to cheenamalhotra/corefx that referenced this pull request Sep 18, 2019
@cheenamalhotra

Copy link
Copy Markdown
Member

@Wraith2 done. It was WIP on my end.

@danmosemsft PR #41178 changes have been tested and hence created, requested review from @karinazhou who is currently reviewing it, let me know if anything more is needed.

@Wraith2

Wraith2 commented Sep 18, 2019

Copy link
Copy Markdown
Contributor Author

Excellent, just wanted to make sure it didn't get lost.

@cheenamalhotra

Copy link
Copy Markdown
Member

@danmosemsft currently this PR is based on release/3.1 branch, I wasn't sure which milestone/branch combinations work, if you think we need to modify milestone, please feel free to do so and let me know if I should consider porting it to 3.0 branch as well or any other branch for 3.0.x milestone.

The customer would like to see this change merged sooner for an earlier upcoming release version
(just not willing to wait till 5.0)

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants