Skip to content

Commit 048c5d7

Browse files
committed
First round of code review
Correct casing and escaping of query. Factor new tests into their own classes, allow them to run against Azure SQL.
1 parent f7f3484 commit 048c5d7

File tree

4 files changed

+31
-34
lines changed

4 files changed

+31
-34
lines changed

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -471,13 +471,13 @@ private string CreateInitialQuery()
471471
SELECT @@TRANCOUNT;
472472
473473
DECLARE @Column_Names NVARCHAR(MAX) = NULL;
474-
IF EXISTS (SELECT TOP 1 * FROM sys.all_columns where object_id = object_id('sys.all_columns') AND [name] = 'graph_type')
474+
IF EXISTS (SELECT TOP 1 * FROM sys.all_columns WHERE [object_id] = OBJECT_ID('sys.all_columns') AND [name] = 'graph_type')
475475
BEGIN
476-
SELECT @Column_Names = COALESCE(@Column_Names + ', ', '') + QUOTENAME(name) FROM {CatalogName}.sys.all_columns WHERE OBJECT_ID = OBJECT_ID('{escapedObjectName}') AND COALESCE(graph_type, 0) NOT IN (1, 3, 4, 6, 7) ORDER BY column_id ASC;
476+
SELECT @Column_Names = COALESCE(@Column_Names + ', ', '') + QUOTENAME([name]) FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = OBJECT_ID('{escapedObjectName}') AND COALESCE([graph_type], 0) NOT IN (1, 3, 4, 6, 7) ORDER BY [column_id] ASC;
477477
END
478478
ELSE
479479
BEGIN
480-
SELECT @Column_Names = COALESCE(@Column_Names + ', ', '') + QUOTENAME(name) FROM {CatalogName}.sys.all_columns WHERE OBJECT_ID = OBJECT_ID('{escapedObjectName}') ORDER BY column_id ASC;
480+
SELECT @Column_Names = COALESCE(@Column_Names + ', ', '') + QUOTENAME([name]) FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = OBJECT_ID('{escapedObjectName}') ORDER BY [column_id] ASC;
481481
END
482482
483483
SELECT @Column_Names = COALESCE(@Column_Names, '*');

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/HiddenTargetColumn.cs

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,25 @@
66
using System.Data.Common;
77
using Xunit;
88

9-
namespace Microsoft.Data.SqlClient.ManualTesting.Tests
9+
namespace Microsoft.Data.SqlClient.ManualTesting.Tests.SqlBulkCopyTests
1010
{
1111
public class HiddenTargetColumn
1212
{
13-
public static void Test(string srcConstr, string dstConstr, string dstTable)
13+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
14+
public void WriteToServer_CopyToHiddenTargetColumn_ThrowsSqlException()
1415
{
15-
using (SqlConnection dstConn = new SqlConnection(dstConstr))
16+
string connectionString = DataTestUtility.TCPConnectionString;
17+
string destinationTable = DataTestUtility.GetUniqueNameForSqlServer("HiddenTargetColumn");
18+
string destinationHistoryTable = DataTestUtility.GetUniqueNameForSqlServer("HiddenTargetColumn_History");
19+
20+
using (SqlConnection dstConn = new SqlConnection(connectionString))
1621
using (SqlCommand dstCmd = dstConn.CreateCommand())
1722
{
1823
dstConn.Open();
1924

2025
try
2126
{
22-
Helpers.TryExecute(dstCmd, $"""
23-
create table dbo.{dstTable}
27+
DataTestUtility.CreateTable(dstConn, destinationTable, $"""
2428
(
2529
Column1 int primary key not null,
2630
Column2 nvarchar(10) not null,
@@ -29,18 +33,18 @@ Column2 nvarchar(10) not null,
2933
ValidTo datetime2 generated always as row end hidden not null,
3034
period for system_time (ValidFrom, ValidTo)
3135
)
32-
with (system_versioning = on(history_table = dbo.{dstTable}_History));
36+
with (system_versioning = on(history_table = dbo.{destinationHistoryTable}));
3337
""");
3438

35-
using (SqlConnection srcConn = new SqlConnection(srcConstr))
39+
using (SqlConnection srcConn = new SqlConnection(connectionString))
3640
using (SqlCommand srcCmd = new SqlCommand("select top 5 EmployeeID, FirstName, LastName, HireDate, sysdatetime() as CurrentDate from employees", srcConn))
3741
{
3842
srcConn.Open();
3943

4044
using (DbDataReader reader = srcCmd.ExecuteReader())
4145
using (SqlBulkCopy bulkcopy = new SqlBulkCopy(dstConn))
4246
{
43-
bulkcopy.DestinationTableName = dstTable;
47+
bulkcopy.DestinationTableName = destinationTable;
4448
SqlBulkCopyColumnMappingCollection ColumnMappings = bulkcopy.ColumnMappings;
4549

4650
ColumnMappings.Add("EmployeeID", "Column1");
@@ -52,18 +56,18 @@ Column2 nvarchar(10) not null,
5256
SqlException sqlEx = Assert.Throws<SqlException>(() => bulkcopy.WriteToServer(reader));
5357

5458
Assert.Equal(13536, sqlEx.Number);
55-
Assert.StartsWith($"Cannot insert an explicit value into a GENERATED ALWAYS column in table '{dstConn.Database}.dbo.{dstTable}'.", sqlEx.Message);
59+
Assert.StartsWith($"Cannot insert an explicit value into a GENERATED ALWAYS column in table '{dstConn.Database}.dbo.{destinationTable.Replace("[", "").Replace("]", "")}'.", sqlEx.Message);
5660
}
5761
}
5862
}
5963
finally
6064
{
61-
Helpers.TryExecute(dstCmd, $"""
62-
alter table {dstTable} set (system_versioning = off);
63-
alter table {dstTable} drop period for system_time;
64-
drop table {dstTable}
65-
drop table {dstTable}_History
65+
DataTestUtility.RunNonQuery(connectionString, $"""
66+
alter table {destinationTable} set (system_versioning = off);
67+
alter table {destinationTable} drop period for system_time;
6668
""");
69+
DataTestUtility.DropTable(dstConn, destinationTable);
70+
DataTestUtility.DropTable(dstConn, destinationHistoryTable);
6771
}
6872
}
6973
}

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/SqlBulkCopyTest.cs

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -109,17 +109,6 @@ public void MissingTargetColumnsTest()
109109
{
110110
MissingTargetColumns.Test(_connStr, _connStr, AddGuid("SqlBulkCopyTest_MissingTargetColumns"));
111111
}
112-
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))]
113-
public void HiddenTargetColumnTest()
114-
{
115-
HiddenTargetColumn.Test(_connStr, _connStr, AddGuid("SqlBulkCopyTest_HiddenTargetColumn"));
116-
}
117-
118-
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))]
119-
public void SqlGraphTablesTest()
120-
{
121-
SqlGraphTables.Test(_connStr, AddGuid("SqlBulkCopyTest_SqlGraphTables_Node"));
122-
}
123112

124113
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))]
125114
public void Bug85007Test()

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/SqlGraphTables.cs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,17 @@
77
using System.Data.Common;
88
using Xunit;
99

10-
namespace Microsoft.Data.SqlClient.ManualTesting.Tests
10+
namespace Microsoft.Data.SqlClient.ManualTesting.Tests.SqlBulkCopyTests
1111
{
1212
public class SqlGraphTables
1313
{
14-
public static void Test(string dstConnectionString, string dstNodeTable)
14+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
15+
public void WriteToServer_CopyToSqlGraphNodeTable_Succeeds()
1516
{
16-
using SqlConnection dstConn = new SqlConnection(dstConnectionString);
17+
string connectionString = DataTestUtility.TCPConnectionString;
18+
string destinationTable = DataTestUtility.GetUniqueNameForSqlServer("SqlGraphNodeTable");
19+
20+
using SqlConnection dstConn = new SqlConnection(connectionString);
1721
using DataTable nodes = new DataTable()
1822
{
1923
Columns = { new DataColumn("Name", typeof(string)) }
@@ -28,17 +32,17 @@ public static void Test(string dstConnectionString, string dstNodeTable)
2832

2933
try
3034
{
31-
DataTestUtility.CreateTable(dstConn, dstNodeTable, "(Id INT PRIMARY KEY IDENTITY(1,1), [Name] VARCHAR(100)) AS NODE");
35+
DataTestUtility.CreateTable(dstConn, destinationTable, "(Id INT PRIMARY KEY IDENTITY(1,1), [Name] VARCHAR(100)) AS NODE");
3236

3337
using SqlBulkCopy nodeCopy = new SqlBulkCopy(dstConn);
3438

35-
nodeCopy.DestinationTableName = dstNodeTable;
39+
nodeCopy.DestinationTableName = destinationTable;
3640
nodeCopy.ColumnMappings.Add("Name", "Name");
3741
nodeCopy.WriteToServer(nodes);
3842
}
3943
finally
4044
{
41-
DataTestUtility.DropTable(dstConn, dstNodeTable);
45+
DataTestUtility.DropTable(dstConn, destinationTable);
4246
}
4347
}
4448
}

0 commit comments

Comments
 (0)