SqlClient: fix BulkCopy double and single null widening handling#37603
Conversation
Wraith2
changed the title
|
@Wraith2 Will take this one as 1.0 milestone. Will start running the tests. |
@afsanehr, I'm confused, what does this mean? |
|
I interpretted that to mean it'll be in the Micosoft.Data.SqlClient first release. Being a bug fix it's also a good idea to have it in the System version as well in my opinion. |
|
Thanks. The milestones here are about .NET Core. This is definitely not going to be in .NET Core 1.0. If it's not going to be in System.Data.SqlClient, we shouldn't have a PR open for it here. @afsanehr, what should be done here? |
As i said, since it's a bug fix it makes sense to me to include it in System.Data.SqlClient since the work has been done and is very simple. It's a two line change and an additional test the only thing stopping it being merged is the lack of time to do so. |
|
Hi, sorry for the delay, currently we are encountering issues in our corefx testing pipelines, which is blocking the merge of this PR. Once we fix the pipeline and this PR passes all the tests we'll merge it immediately. This PR has been reviewed already. |
|
@Wraith2 This PR is being tracked and will be picked up for dotnet/SqlClient. Appreciate your patience for some more time while we come back to review and merge your contributions! 🙏 |
|
@cheenamalhotra I'll assign this PR to you while it's being worked out if you don't mind. |
cheenamalhotra
left a comment
cheenamalhotra
left a comment
There was a problem hiding this comment.
PR changes have been tested and should be ready to merge once comments are addressed.
… converting to decimal without throwing and add tests
…net/corefx#37603) * change BulkCopy double and single handling to handle Null values when converting to decimal without throwing and add tests * address feedback Commit migrated from dotnet/corefx@97ddad9
7 participants
fixes https://github.com/dotnet/corefx/issues/37465
When using bulk copy with an inexact numeric source which is null and a decimal target the conversion from numeric to decimal creates an appropriately typed source variable and then attempts to pass that into a
SqlDecimalconstructor using the sourceValueproperty. This action fails when the source field is null because theValueproperty throws on null.SqlDecimaldoes not expose a public constructor that allows setting null. It does have explicit conversion operators defined that do handle null values correctly so I have changed it to use these and noted why in the source. I have added a test which contains both real and float types. Manual and functional tests pass. This should be simple to pull up into Micosoft.Data.SqlServer./cc area owners @afsanehr, @Gary-Zh , @David-Engel and problem reporter @sergii-kryzh