SQL Server bulk insert doesn't allow inserting varchar column values longer than 1 character

[jdlundquist]

Title

SQL Server bulk insert generates table without specifying varchar column size

Functional impact

For SQL Server, a runtime exception is thrown and you cannot save changes on a DbContext when trying to insert more than one of the same type of entity if that entity has an underlying varchar column and you pass a string value longer than 1 character in length.

Minimal repro steps

My actual usage scenario involves various tables and entities, but the following is my hypothesis of steps to repro:

1. Create a database with a table having a column that is varchar(max) (or perhaps any size greater than 1), e.g. MyTable.ExampleColumn
2. Scaffold (or hand-build) a derived class from DbContext and map MyTable.ExampleColumn with .HasColumnType("varchar(max)")
3. Create multiple instances of the MyTable entity and put a string value in ExampleColumn longer than 1 character
4. Add the multiple entities via DbContext.Add
5. Call DbContext.SaveChanges() or DbContext.SaveChangesAsync()

Expected result

Adding multiple entities of the same type to a DbContext would insert successfully into the database without throwing an exception, even when they have string column values containing data longer than 1 character

Actual result

A runtime exception is thrown: SqlException: String or binary data would be truncated.
None of the changes to the DbContext are saved to the database.

Further technical details

I think the starting point for digging into the issue is: Microsoft.EntityFrameworkCore.Update.Internal.SqlServerUpdateSqlGenerator. The GetTypeNameForCopy method (used by AppendDeclareTable) should probably include a length for applicable data types (e.g. varchar, nvarchar, etc.) when generating the columns for the temp table.

Looking at the code statically without running the EF7 code directly, here's my suggestion for a couple high level things that could be done to make this work:

1. Make sure that the right mapping is retrieved for the property in GetTypeNameForCopy. (This may be a separate issue that could be skipped depending on how (2) is done below). Specifically, I think that if you have a varchar(max) column, the mapping currently returned may be the _varchar mapping rather than _varcharmax (from SqlServerTypeMapper) due to Microsoft.EntityFrameworkCore.Storage.FindMapping(IProperty) implementation that strips off everything after opening parentheses from a column type name. Perhaps this would be OK if there was a fluent API for building the model that allows specifying a SQL server special "Max length" column (e.g. varchar(max) vs. varchar(200)). If the fluent API requires specifying HasColumnType("varchar(max)"), then we may need to not strip off the "(max)" from the data type in this case. Perhaps SqlServerTypeMapper could overload FindMapping(IProperty) to customize the logic.
2. Rather than using the simple mapping.DefaultTypeName in GetTypeNameForCopy, get a length-qualified type. The issue RevEng: generated props .HasMaxLength(x).HasColumnType("varchar") create varchar(1) columns in DB #4312 may be related and its proposed fix in PR [RevEng] Output the HasColumnType() and HasMaxLength() APIs correctly #4348 could be useful, e.g. the new method SqlServerTypeMapper.MaxLengthQualifiedDataType or something like that. I know there is some custom logic for determining string length sizes for parameters (e.g. SqlServerMaxLengthMapping.ConfigureParameter discussed in Use column/property facets for parameter types in Update Pipeline #4134 ), so that could be useful to mirror to keep the temp table column types for bulk inserts consistent with the parameter values passed to them.

Right now, the generated SQL is something like this (scrubbed actual tables/columns that I got from SQL Server profiler and simplified):

exec sp_executesql N'SET NOCOUNT ON;
DECLARE @toInsert2 TABLE ([ExampleColumn] varchar, [_Position] [int]);
INSERT INTO @toInsert2
VALUES (@p0, 0),
(@p1, 1);
DECLARE @inserted2 TABLE ([ExampleColumn] varchar);
INSERT INTO [dbo].[MyTable] ([ExampleColumn])
OUTPUT INSERTED.ExampleColumn
INTO @inserted2
SELECT [ExampleColumn] FROM @toInsert2;
...
',N'@p0 varchar(8000),@p1 varchar(8000)',
@p0='First string',
@p1='Second string'

A couple notes:

1. The @toInsert2 column ExampleColumn needs a length for varchar (either 'max', which is the real column type, or perhaps 8000 to match the parameter) since varchar equates to varchar(1) and can't fit any string values larger than 1 character
2. The parameters @p0 and @p1 are varchar(8000) even though the DB column is actually varchar(max) and mapped as varchar(max) during 'OnModelCreating` because the runtime parameter values fit within the pre-determined string parameter sizes.

[jdlundquist]

Is there a workaround for this while it's getting fixed, e.g. turn off bulk inserts or some other DbContext configuration or usage?

[ErikEJ]

MaxBatchSize(1) ?

[jdlundquist]

Thanks, that is a good temporary workaround.

For others' benefit, example in Startup.ConfigureServices:

services
    .AddEntityFramework()
    .AddSqlServer()
    .AddDbContext<MyDbContext>(options =>
                {
                    options.UseSqlServer(connectionString)
                        .MaxBatchSize(1);
                });

Or configure the options in context class OnConfiguring

[divega]

@jdlundquist thanks for reporting this. It seems that indeed we are missing including a length when we generate the type names in SqlServerUpdateSqlGenerator.GetTypeNameForCopy() for most types in which it matters.

cc @rowanmiller @AndriySvyryd @ajcvickers This seems very important so I will mark it RC2 right away, but we can still discuss it in triage.

[rowanmiller]

This seems very important so I will mark it RC2 right away, but we can still discuss it in triage.

👍