Skip to content

[Perf] Bypass SqlBulkCopy Graph Column Mapping if Not Copying Graph Tables #4535

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.

Sign up for GitHub

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

Open
benrr101 wants to merge 3 commits into
base: main
Choose a base branch
from
+118 −64
Open

[Perf] Bypass SqlBulkCopy Graph Column Mapping if Not Copying Graph Tables #4535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
df6dc04
:robot: Only perform graph column mapping if bulk copy includes graph…
benrr101 Aug 12, 2026
acfaf72
Fix for failing manual test
benrr101 Aug 13, 2026
d704624
Fix for the other manual test failure
benrr101 Aug 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 115 additions & 59 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ internal BulkCopySimpleResultSet()
_results = new List<Result>();
}

internal int Count => _results.Count;

internal Result this[int idx] => _results[idx];

// Callback function for the tdsparser
Expand Down Expand Up @@ -151,11 +153,11 @@ public SourceColumnMetadata(ValueMethod method, bool isSqlType, bool isDataFeed)
public readonly bool IsDataFeed;
}

// The initial query will return three tables.
// The initial query will return three tables, and may return a fourth for column aliases.
// Transaction count has only one value in one column and one row
// MetaData has n columns but no rows
// Collation has 4 columns and n rows
// Column aliases has 3 columns and n rows
// Column aliases has 2 columns and n rows

private const int MetaDataResultId = 1;

Expand Down Expand Up @@ -484,6 +486,50 @@ private string CreateInitialQuery()
string objectName = ADP.BuildMultiPartName(parts);
string escapedObjectName = SqlServerEscapeHelper.EscapeStringAsLiteral(objectName);
string catalogNameStringLiteral = CatalogName is null ? null : SqlServerEscapeHelper.EscapeStringAsLiteral(CatalogName);
bool resolveColumnAliases = ShouldResolveColumnAliases();

@priyankatiwari08 priyankatiwari08 Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: when the fragments collapse to string.Empty, the interpolation holes leave stray
blank lines in the generated batch. Harmless for the server, but it makes the
TryTraceEvent dump of the initial query noisier when diagnosing bulk copy issues.

@edwardneal edwardneal Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One point to bear in mind here is that ShouldResolveColumnAliases is only a heuristic. Only SQL Server truly knows whether a table is a Graph table. Any table (whether a normal table or a Graph table) can contain [$edge_id] / etc. columns. This is fine for now, but it doesn't (and can't) provide an absolute guarantee from the client.

string createColumnAliasesTableQuery = resolveColumnAliases
? """

CREATE TABLE #Column_Aliases
(
[Canonical_Column_Name] SYSNAME,
[Canonical_Column_Id] INT,
[Aliased_Column_Name] SYSNAME
)
"""
: string.Empty;
string populateColumnAliasesQuery = resolveColumnAliases
? $"""

EXEC sp_executesql N'
INSERT INTO #Column_Aliases ([Canonical_Column_Name], [Canonical_Column_Id], [Aliased_Column_Name])
SELECT [name], [column_id], ''$to_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 8
UNION ALL
SELECT [name], [column_id], ''$from_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 5
UNION ALL
SELECT [name], [column_id], ''$edge_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 2 AND [name] LIKE ''$edge[_]id[_]%''
UNION ALL
SELECT [name], [column_id], ''$node_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 2 AND [name] LIKE ''$node[_]id[_]%''',
N'@Object_ID INT', @Object_ID = @Object_ID
Comment on lines +504 to +513

@edwardneal edwardneal Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we can't simply remove this specific statement from the executed SQL command if the client-side heuristic fails? This means that the final result set always appears (and just doesn't contain a value). We keep the heuristic logic in one place and don't need to consider the downstream impacts - they already handle a zero-length result set.

"""
: string.Empty;
string removeShadowedColumnAliasesQuery = resolveColumnAliases
? $"""

DELETE FROM #Column_Aliases
WHERE [Aliased_Column_Name] IN (SELECT [name] FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID)
"""
: string.Empty;
string selectColumnAliasesQuery = resolveColumnAliases
? """

SELECT [Canonical_Column_Name], [Aliased_Column_Name]
FROM #Column_Aliases
ORDER BY [Canonical_Column_Id] ASC

DROP TABLE #Column_Aliases
"""
: string.Empty;
// Specify the column names explicitly. This is to ensure that we can map to hidden
// columns (e.g. columns in temporal tables.) If the target table doesn't exist,
// OBJECT_ID will return NULL and @Column_Names will remain non-null. The subsequent
Expand Down Expand Up @@ -543,13 +589,7 @@ private string CreateInitialQuery()
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');

CREATE TABLE #Column_Aliases
(
[Canonical_Column_Name] SYSNAME,
[Canonical_Column_Id] INT,
[Aliased_Column_Name] SYSNAME
)
{createColumnAliasesTableQuery}

IF CAST(SERVERPROPERTY('EngineEdition') AS INT) = 6
BEGIN
Expand All @@ -567,17 +607,7 @@ IF CAST(SERVERPROPERTY('EngineEdition') AS INT) = 6
IF EXISTS (SELECT TOP 1 * FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = OBJECT_ID('{catalogNameStringLiteral}.[sys].[all_columns]') AND [name] = 'graph_type')
BEGIN
SET @Column_Name_Query_FILTER = N'WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) NOT IN (1, 3, 4, 6, 7)';

EXEC sp_executesql N'
INSERT INTO #Column_Aliases ([Canonical_Column_Name], [Canonical_Column_Id], [Aliased_Column_Name])
SELECT [name], [column_id], ''$to_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 8
UNION ALL
SELECT [name], [column_id], ''$from_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 5
UNION ALL
SELECT [name], [column_id], ''$edge_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 2 AND [name] LIKE ''$edge[_]id[_]%''
UNION ALL
SELECT [name], [column_id], ''$node_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 2 AND [name] LIKE ''$node[_]id[_]%''',
N'@Object_ID INT', @Object_ID = @Object_ID
{populateColumnAliasesQuery}
END
ELSE
BEGIN
Expand All @@ -586,9 +616,7 @@ UNION ALL
SET @Column_Name_Query = @Column_Name_Query_SELECT + ' FROM {catalogNameStringLiteral}.[sys].[all_columns] ' + @Column_Name_Query_FILTER + ' ' + @Column_Name_Query_SORT + ';'

EXEC sp_executesql @Column_Name_Query, N'@Object_ID INT, @Column_Names NVARCHAR(MAX) OUTPUT', @Object_ID = @Object_ID, @Column_Names = @Column_Names OUTPUT;

DELETE FROM #Column_Aliases
WHERE [Aliased_Column_Name] IN (SELECT [name] FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID)
{removeShadowedColumnAliasesQuery}
END

SELECT @Column_Names = COALESCE(@Column_Names, '*');
Expand All @@ -598,12 +626,7 @@ WHERE [Aliased_Column_Name] IN (SELECT [name] FROM {CatalogName}.[sys].[all_colu
SET FMTONLY OFF;

EXEC {CatalogName}..{TableCollationsStoredProc} N'{SchemaName}.{TableName}';

SELECT [Canonical_Column_Name], [Aliased_Column_Name]
FROM #Column_Aliases
ORDER BY [Canonical_Column_Id] ASC

DROP TABLE #Column_Aliases
{selectColumnAliasesQuery}
""";
}

Expand All @@ -614,7 +637,8 @@ DROP TABLE #Column_Aliases
private Task<BulkCopySimpleResultSet> CreateAndExecuteInitialQueryAsync(out BulkCopySimpleResultSet result)
{
// Check if we have valid cached metadata for the current destination table
if (CachedMetadata != null)
if (CachedMetadata != null
&& (!ShouldResolveColumnAliases() || CachedMetadata.Count > ColumnAliasesResultId))

@priyankatiwari08 priyankatiwari08 Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ShouldResolveColumnAliases() is evaluated here and again in CreateInitialQuery,
walking the mapping collection each time. Beyond the (minor) duplicated work, the bigger
concern is that these two call sites must agree — if they ever observe different mapping
state, the cache-validity check and the actual query shape diverge, and
AnalyzeTargetAndCreateUpdateBulkCommand silently skips alias resolution.

We should consider computing it once per operation (e.g. in WriteRowSourceToServerCommon, right
after _localColumnMappings is finalized) and storing it in a field, so the query shape
and the cache check are guaranteed to be derived from the same evaluation.

{
SqlClientEventSource.Log.TryTraceEvent("SqlBulkCopy.CreateAndExecuteInitialQueryAsync | Info | Using cached metadata for table '{0}'", _destinationTableName);
result = CachedMetadata;
Expand Down Expand Up @@ -714,44 +738,47 @@ private string AnalyzeTargetAndCreateUpdateBulkCommand(BulkCopySimpleResultSet i

// Apply any necessary column aliases. If an aliased name exists in the
// local column mappings but the canonical name does not, update them.
Result columnAliasResults = internalResults[ColumnAliasesResultId];
for (int i = 0; i < columnAliasResults.Count; i++)
if (internalResults.Count > ColumnAliasesResultId)
{
Row aliasRow = columnAliasResults[i];
SqlString canonicalName = (SqlString)aliasRow[ColumnCanonicalNameColumnId];
SqlString aliasedName = (SqlString)aliasRow[ColumnAliasColumnId];

if (canonicalName.IsNull || aliasedName.IsNull)
Result columnAliasResults = internalResults[ColumnAliasesResultId];
for (int i = 0; i < columnAliasResults.Count; i++)
{
continue;
}
Row aliasRow = columnAliasResults[i];
SqlString canonicalName = (SqlString)aliasRow[ColumnCanonicalNameColumnId];
SqlString aliasedName = (SqlString)aliasRow[ColumnAliasColumnId];

string canonical = canonicalName.Value;
bool canonicalNameExists = unmatchedColumns.Contains(canonical)
// The destination columns might be escaped. If so, search for those instead
|| unmatchedColumns.Contains(SqlServerEscapeHelper.EscapeIdentifier(canonical));
if (canonicalName.IsNull || aliasedName.IsNull)
{
continue;
}

if (canonicalNameExists)
{
continue;
}
string canonical = canonicalName.Value;
bool canonicalNameExists = unmatchedColumns.Contains(canonical)
// The destination columns might be escaped. If so, search for those instead
|| unmatchedColumns.Contains(SqlServerEscapeHelper.EscapeIdentifier(canonical));

// The canonical name does not exist. Look for a local column mapping which matches
// the alias (or its escaped variant) and replace its name with its canonical name.
string alias = aliasedName.Value;
string escapedAlias = SqlServerEscapeHelper.EscapeIdentifier(alias);
if (canonicalNameExists)
{
continue;
}

for (int j = 0; j < _localColumnMappings.Count; j++)
{
if (unmatchedColumns.Comparer.Equals(_localColumnMappings[j].DestinationColumn, alias)
|| unmatchedColumns.Comparer.Equals(_localColumnMappings[j].DestinationColumn, escapedAlias))
// The canonical name does not exist. Look for a local column mapping which matches
// the alias (or its escaped variant) and replace its name with its canonical name.
string alias = aliasedName.Value;
string escapedAlias = SqlServerEscapeHelper.EscapeIdentifier(alias);

for (int j = 0; j < _localColumnMappings.Count; j++)
{
unmatchedColumns.Remove(_localColumnMappings[j].DestinationColumn);
if (unmatchedColumns.Comparer.Equals(_localColumnMappings[j].DestinationColumn, alias)
|| unmatchedColumns.Comparer.Equals(_localColumnMappings[j].DestinationColumn, escapedAlias))
{
unmatchedColumns.Remove(_localColumnMappings[j].DestinationColumn);

unmatchedColumns.Add(canonical);
_localColumnMappings[j].MappedDestinationColumn = canonical;
unmatchedColumns.Add(canonical);
_localColumnMappings[j].MappedDestinationColumn = canonical;

break;
break;
}
}
}
}
Expand Down Expand Up @@ -1632,6 +1659,33 @@ private void AppendColumnNameAndTypeName(StringBuilder query, string columnName,
query.Append(typeName);
}

private bool ShouldResolveColumnAliases()

@priyankatiwari08 priyankatiwari08 Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we get direct coverage for the bypass itself? The CopyAllFromReader stat changes
verify it only indirectly. Three cases worth pinning down:

  1. Copying to a real graph table by ordinal (no alias in ColumnMappings) — confirms the
    bypass doesn't regress Feature | Support SQL Graph column aliases in SqlBulkCopy #3677 when the alias names never appear in the mappings.
  2. Reusing one SqlBulkCopy across two WriteToServer calls, first with an alias mapping
    and then without (and vice versa) — covers the MappedDestinationColumn reset.
  3. SqlBulkCopyOptions.CacheMetadata combined with alias mappings — exercises the new
    CachedMetadata.Count > ColumnAliasesResultId guard in both directions.

{
if (_localColumnMappings is null)
{
return false;
}

for (int i = 0; i < _localColumnMappings.Count; i++)
{
if (IsGraphColumnAlias(_localColumnMappings[i].DestinationColumn))
{
return true;
}
}

return false;
}

private bool IsGraphColumnAlias(string name)
{
string unquotedName = UnquotedName(name);
return string.Equals(unquotedName, "$node_id", StringComparison.OrdinalIgnoreCase)
|| string.Equals(unquotedName, "$edge_id", StringComparison.OrdinalIgnoreCase)
|| string.Equals(unquotedName, "$from_id", StringComparison.OrdinalIgnoreCase)
|| string.Equals(unquotedName, "$to_id", StringComparison.OrdinalIgnoreCase);
}

private string UnquotedName(string name)
{
if (string.IsNullOrEmpty(name))
Expand Down Expand Up @@ -2330,6 +2384,8 @@ private void WriteRowSourceToServerCommon(int columnCount)
_localColumnMappings.ValidateCollection();
foreach (SqlBulkCopyColumnMapping bulkCopyColumn in _localColumnMappings)
{
bulkCopyColumn.MappedDestinationColumn = null;

@priyankatiwari08 priyankatiwari08 Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reset is inside a loop that breaks early, so mappings after the first one with
_internalSourceColumnOrdinal == -1 never get cleared.

_internalSourceColumnOrdinal is assigned during WriteRowSourceToServerCommon and is
never reset to -1 afterwards, and ColumnMappings.ReadOnly goes back to false at the
end of the operation — so a caller can mutate mappings and reuse the instance. If they
set SourceColumn on an early mapping (which resets that ordinal to -1), the loop breaks
before reaching a later graph-alias mapping, and it keeps the stale
MappedDestinationColumn resolved against the previous destination table.

Suggest hoisting it into its own unconditional pass before the ordinal scan:

foreach (SqlBulkCopyColumnMapping bulkCopyColumn in _localColumnMappings)
{
    bulkCopyColumn.MappedDestinationColumn = null;
}

foreach (SqlBulkCopyColumnMapping bulkCopyColumn in _localColumnMappings)
{
    if (bulkCopyColumn._internalSourceColumnOrdinal == -1)
    {
        unspecifiedColumnOrdinals = true;
        break;
    }
}


if (bulkCopyColumn._internalSourceColumnOrdinal == -1)
{
unspecifiedColumnOrdinals = true;
Expand Down
8 changes: 3 additions & 5 deletions src/Microsoft.Data.SqlClient/tests/ManualTests/BulkCopy/CopyAllFromReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,8 @@ public void Test()
using (DbDataReader reader = srcCmd.ExecuteReader())
{
IDictionary stats;
long expectedIduCount = DataTestUtility.IsAzureSynapse || DataTestUtility.IsAtLeastSQL2017() ? 2 : 1;
long expectedSelectCount = DataTestUtility.IsAzureSynapse ? 4 : 13;
long expectedSelectCount = DataTestUtility.IsAzureSynapse ? 4 : 12;
long expectedSelectRows = DataTestUtility.IsAzureSynapse ? 4 : 15;
long expectedTransactions = DataTestUtility.IsAzureSynapse || DataTestUtility.IsAtLeastSQL2017() ? 2 : 1;
using (SqlBulkCopy bulkcopy = new SqlBulkCopy(dstConn))
{
bulkcopy.DestinationTableName = dstTable;
Expand All @@ -69,12 +67,12 @@ public void Test()

DataTestUtility.AssertEqualsWithDescription((long)3, stats["BuffersReceived"], "Unexpected BuffersReceived value.");
DataTestUtility.AssertEqualsWithDescription((long)3, stats["BuffersSent"], "Unexpected BuffersSent value.");
DataTestUtility.AssertEqualsWithDescription(expectedIduCount, stats["IduCount"], "Unexpected IduCount value.");
DataTestUtility.AssertEqualsWithDescription((long)0, stats["IduCount"], "Unexpected IduCount value.");
DataTestUtility.AssertEqualsWithDescription(expectedSelectCount, stats["SelectCount"], "Unexpected SelectCount value.");
DataTestUtility.AssertEqualsWithDescription((long)3, stats["ServerRoundtrips"], "Unexpected ServerRoundtrips value.");
DataTestUtility.AssertEqualsWithDescription(expectedSelectRows, stats["SelectRows"], "Unexpected SelectRows value.");
DataTestUtility.AssertEqualsWithDescription((long)2, stats["SumResultSets"], "Unexpected SumResultSets value.");
DataTestUtility.AssertEqualsWithDescription(expectedTransactions, stats["Transactions"], "Unexpected Transactions value.");
DataTestUtility.AssertEqualsWithDescription((long)0, stats["Transactions"], "Unexpected Transactions value.");
}
}
}
Expand Down
Loading