-
Notifications
You must be signed in to change notification settings - Fork 317
[6.1] Fixing NullReferenceException issue with SqlDataAdapter #3846
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/6.1
Are you sure you want to change the base?
[6.1] Fixing NullReferenceException issue with SqlDataAdapter #3846
Conversation
fbdcc90 to
8291391
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Removed unused SqlDataAdapterBatchUpdateTests.cs file from project.
aed9e25
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Show resolved
Hide resolved
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Outdated
Show resolved
Hide resolved
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Outdated
Show resolved
Hide resolved
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Outdated
Show resolved
Hide resolved
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Show resolved
Hide resolved
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Show resolved
Hide resolved
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Show resolved
Hide resolved
…com/dotnet/SqlClient into dev/prtiwar/6.1-SqlDataAdapterIssue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Outdated
Show resolved
Hide resolved
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Outdated
Show resolved
Hide resolved
apoorvdeshmukh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to resolve the copilot comments
560d2c6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
...crosoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj
Outdated
Show resolved
Hide resolved
...crosoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj
Outdated
Show resolved
Hide resolved
...crosoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
...crosoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj
Show resolved
Hide resolved
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Outdated
Show resolved
Hide resolved
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Show resolved
Hide resolved
Removed EnsureBuyerSellerObjectsExist calls from tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
...Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
apoorvdeshmukh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be ported after #3749 is fixed in main
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
| <ItemGroup> | ||
| <Folder Include="microsoft.data.sqlclient\tests\manualtests\alwaysencrypted\testfixtures\setup\" /> | ||
| </ItemGroup> |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This empty Folder ItemGroup is unnecessary and should be removed. Modern SDK-style .csproj files automatically include folders with source files. This element adds no value and creates clutter in the project file. Additionally, the path uses incorrect casing (lowercase instead of proper casing) which doesn't match the actual folder structure.
| <ItemGroup> | |
| <Folder Include="microsoft.data.sqlclient\tests\manualtests\alwaysencrypted\testfixtures\setup\" /> | |
| </ItemGroup> |
| var dt = new DataTable(_tableName); | ||
| dt.Columns.AddRange(new[] | ||
| { | ||
| new DataColumn("BuyerSellerID", typeof(int)), |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'DataColumn' is created but not disposed.
| dt.Columns.AddRange(new[] | ||
| { | ||
| new DataColumn("BuyerSellerID", typeof(int)), | ||
| new DataColumn("SSN1", typeof(string)), |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'DataColumn' is created but not disposed.
| { | ||
| new DataColumn("BuyerSellerID", typeof(int)), | ||
| new DataColumn("SSN1", typeof(string)), | ||
| new DataColumn("SSN2", typeof(string)), |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'DataColumn' is created but not disposed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
| command.ExecuteNonQuery(); | ||
| } | ||
|
|
||
| // ✅ CHANGED: Use unique SP names |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment contains checkmark emojis and review-style markers that should be removed. Comments in production code should simply explain the code without review artifacts. Consider removing this comment since the code is self-explanatory.
| command.ExecuteNonQuery(); | ||
| } | ||
|
|
||
| // ✅ CHANGED: Use unique SP names |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment contains checkmark emojis and review-style markers that should be removed. Comments in production code should simply explain the code without review artifacts. Consider removing this comment since the code is self-explanatory.
| private readonly ColumnEncryptionKey _columnEncryptionKey1; | ||
| private readonly ColumnEncryptionKey _columnEncryptionKey2; | ||
|
|
||
| // ✅ ADD: Unique stored procedure names based on table name |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment contains checkmark emojis and review-style markers that should be removed. Comments in production code should simply explain the code without review artifacts. Consider changing to a simple explanatory comment or removing it entirely since the property name and implementation are self-explanatory.
| dt.Columns.AddRange(new[] | ||
| { | ||
| new DataColumn("BuyerSellerID", typeof(int)), | ||
| new DataColumn("SSN1", typeof(string)), | ||
| new DataColumn("SSN2", typeof(string)), | ||
| }); |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'DataColumn' is created but not disposed.
| dt.Columns.AddRange(new[] | |
| { | |
| new DataColumn("BuyerSellerID", typeof(int)), | |
| new DataColumn("SSN1", typeof(string)), | |
| new DataColumn("SSN2", typeof(string)), | |
| }); | |
| dt.Columns.Add("BuyerSellerID", typeof(int)); | |
| dt.Columns.Add("SSN1", typeof(string)); | |
| dt.Columns.Add("SSN2", typeof(string)); |
Ports #3749 to release/6.1 branch
Closes #3845