dotnet / dotnet/SqlClient

SqlBulkCopy loses SqlMoney type identity when writing sql_variant columns

Open
#4,040 0 comments 0 reactions 0 assignees View on GitHub
Area\Sql Bulk Copy Repro Available :heavy_check_mark:
Dominant language
C#
Stars
989
Forks
340
Avg merge
4d 18h
Merged PRs (30d)
69

Description

## Description

When SqlMoney values are bulk-copied into a `sql_variant` column, they arrive as `numeric` instead of `money`. This is because SqlBulkCopy's write path strips SqlMoney to its underlying CLR `decimal` and then fails to recover the original TDS type.

## Root Cause

SqlBulkCopy invokes `ValidateBulkCopyVariant()`, which calls `MetaType.GetComValueFromSqlVariant()` to convert all `INullable` SqlTypes to their CLR equivalents. For most types the CLR value maps back to the same TDS type (e.g. `SqlInt32` → `int` → `SQLINT4`), but `SqlMoney` → `decimal` → `SQLNUMERICN` instead of `SQLMONEY`.

The normal parameter write path (`TdsParser.WriteSqlVariantValue`) has a heuristic that works around this: when it encounters a `decimal` value with `length == 8`, it assumes the value was originally `SqlMoney` and writes `SQLMONEY` on the wire. However, the bulk copy write path (`TdsParser.WriteSqlVariantDataRowValue`) does not accept a length parameter and has no such recovery logic. Its `case TdsEnums.SQLMONEY` branch is effectively unreachable because the value has already been converted to a plain `decimal`, which maps to `SQLNUMERICN`.

### Relevant Code Locations

- **`SqlBulkCopy.cs` ~line 1515** — `ValidateBulkCopyVariant()` calls `GetComValueFromSqlVariant()`, which converts `SqlMoney` to `decimal`
- **`TdsParser.cs` ~line 7694-7714** — `WriteSqlVariantValue` contains the `length == 8` heuristic that recovers `SqlMoney` from `decimal`
- **`TdsParser.cs` ~line 7869-8060** — `WriteSqlVariantDataRowValue` lacks this heuristic; `case TdsEnums.SQLMONEY` (~line 7985) is unreachable for SqlMoney values

## Expected Behavior

A `SqlMoney` value bulk-copied into a `sql_variant` column should produce a value with `SQL_VARIANT_PROPERTY(col, 'BaseType') = 'money'`.

## Actual Behavior

The value arrives as `numeric` (i.e. `SQL_VARIANT_PROPERTY(col, 'BaseType') = 'numeric'`).

## Reproduction

```csharp
using var connection = new SqlConnection(connectionString);
connection.Open();

// Setup
using var cmd = new SqlCommand(
"CREATE TABLE #Test (Id INT, Val SQL_VARIANT)", connection);
cmd.ExecuteNonQuery();

// Bulk copy a SqlMoney value
var dt = new DataTable();
dt.Columns.Add("Id", typeof(int));
dt.Columns.Add("Val", typeof(object));
dt.Rows.Add(1, new SqlMoney(1.23m));

using var bulkCopy = new SqlBulkCopy(connection);
bulkCopy.DestinationTableName = "#Test";
bulkCopy.WriteToServer(dt);

// Verify
cmd.CommandText =
"SELECT SQL_VARIANT_PROPERTY(Val, 'BaseType') FROM #Test WHERE Id = 1";
var baseType = (string)cmd.ExecuteScalar();
// baseType is "numeric" — expected "money"
```

## Impact

This is a data fidelity issue: the value is numerically correct, but the SQL Server base type metadata is lost. Consumers that branch on `SQL_VARIANT_PROPERTY(..., 'BaseType')` or expect `money` semantics will see incorrect behavior.

## Possible Fix

`WriteSqlVariantDataRowValue` could apply the same `length == 8` heuristic used by `WriteSqlVariantValue`, or `ValidateBulkCopyVariant` could preserve the original `MetaType` for `SqlMoney` values rather than stripping them to `decimal`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.