Port | PR 4306#4402
Open
edwardneal wants to merge 3 commits into
Open
Conversation
Correct compile errors and test with Azure Synapse Analytics
edwardneal
commented
Jun 24, 2026
| Assert.Equal(BulkCopyRowCount, resultantRowCount); | ||
| } | ||
|
|
||
| [ConditionalFact(nameof(CanRunTests), nameof(IsAtLeastSQL2017))] |
Contributor
Author
There was a problem hiding this comment.
This prevents the test which uses SQL Graph tables from running on SQL 2016 (which doesn't support SQL Graph). It'll also be replicated on main in my next PR.
Contributor
There was a problem hiding this comment.
Pull request overview
Ports the SqlBulkCopy least-privilege fix (guarding sys.all_columns metadata access via HAS_PERMS_BY_NAME) onto the 7.0 branch, along with supporting ManualTests coverage and small shared test RAII primitives.
Changes:
- Updates
SqlBulkCopyinitial metadata query to conditionally querysys.all_columnsbased on permissions. - Adds ManualTests coverage for unprivileged SQL logins and adjusts an existing stats-based bulk copy test for the additional query.
- Introduces shared transient database-object helpers (
ServerLogin,DatabaseUser,Table) and a net462-onlyMemberNotNullAttributeshim for nullable analysis in ManualTests.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs | Adds HAS_PERMS_BY_NAME gating around sys.all_columns query in the initial metadata SQL batch. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs | Adds ManualTests validating SqlBulkCopy behavior under a login denied access to sys.all_columns. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/CopyAllFromReader.cs | Updates expected connection statistics to account for the additional metadata/permission query. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj | Includes new ManualTests source files (nullable attribute shim + new test). |
| src/Microsoft.Data.SqlClient/tests/ManualTests/Extensions/CodeAnalysis.netfx.cs | Adds a net462-only MemberNotNullAttribute shim for nullable flow analysis in tests. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs | Adds helper properties/methods for server role membership and SQL-auth capability detection. |
| src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs | Adds shared RAII base for transient database objects (create/drop + unique naming). |
| src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs | Adds RAII wrapper for database users, including cross-database command execution. |
| src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs | Adds RAII wrapper for SQL logins with generated compliant passwords. |
| src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs | Adds RAII wrapper for transient tables used by tests. |
Comment on lines
528
to
+530
| DECLARE @Column_Name_Query NVARCHAR(MAX); | ||
| DECLARE @Column_Names NVARCHAR(MAX) = NULL; | ||
| DECLARE @Has_Sys_All_Columns_Permissions INT = HAS_PERMS_BY_NAME('{catalogNameStringLiteral}.[sys].[all_columns]', 'OBJECT', 'SELECT'); |
| BEGIN | ||
| SET @Column_Name_Query_FILTER = N'WHERE [object_id] = @Object_ID'; | ||
| END | ||
| SET @Column_Name_Query = @Column_Name_Query_SELECT + ' FROM {catalogNameStringLiteral}.[sys].[all_columns] ' + @Column_Name_Query_FILTER + ' ' + @Column_Name_Query_SORT + ';' |
| // | ||
| // See: https://learn.microsoft.com/en-us/sql/t-sql/functions/serverproperty-transact-sql | ||
| // | ||
| // All of this is wrapped in an test against HAS_PERMS_BY_NAME. This test verifies that |
Comment on lines
+9
to
+12
| // These classes are provided to provide compile-time support for Microsoft.CodeAnalysis | ||
| // attributes. These attributes are native to netcore and available for netfx as a nuget | ||
| // package - but only for net472. As such, until net462 support is dropped, these placeholder | ||
| // classes will need to exist if we want to use them for static analysis. |
Comment on lines
+35
to
+40
| protected override void DropObject() | ||
| { | ||
| using SqlCommand dropCommand = new($"IF (OBJECT_ID('{Name}') IS NOT NULL) DROP TABLE {Name}", Connection); | ||
|
|
||
| dropCommand.ExecuteNonQuery(); | ||
| } |
Comment on lines
+94
to
+99
| protected override void DropObject() | ||
| { | ||
| using SqlCommand dropCommand = new($"IF SUSER_ID('{UnescapedName}') IS NOT NULL DROP LOGIN {Name}", Connection); | ||
|
|
||
| dropCommand.ExecuteNonQuery(); | ||
| } |
Comment on lines
+36
to
+41
| protected override void DropObject() | ||
| { | ||
| using SqlCommand dropCommand = new($"IF USER_ID('{UnescapedName}') IS NOT NULL DROP USER {Name}", Connection); | ||
|
|
||
| ExecuteCommandInDatabase(dropCommand); | ||
| } |
Comment on lines
+490
to
+500
| public static int GetAuthenticationMode() | ||
| { | ||
| using SqlConnection connection = new(TCPConnectionString); | ||
|
|
||
| connection.Open(); | ||
| using SqlCommand command = new("EXEC xp_instance_regread N'HKEY_LOCAL_MACHINE', N'Software\\Microsoft\\MSSQLServer\\MSSQLServer', N'LoginMode'", connection); | ||
| using SqlDataReader reader = command.ExecuteReader(); | ||
|
|
||
| reader.Read(); | ||
| return reader.GetInt32(1); | ||
| } |
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/7.0 #4402 +/- ##
===============================================
- Coverage 73.07% 65.63% -7.44%
===============================================
Files 280 275 -5
Lines 43087 65953 +22866
===============================================
+ Hits 31484 43291 +11807
- Misses 11603 22662 +11059
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
This PR ports #4306 to the 7.0 branch, and this contains the full description.
The automated test uses the RAII primitives, so I've also needed to include these.
Issues
Fixes #4370.
Ports #4306.
Testing
Automated tests ported from #4306, with one addition.