From 09bf40106d495748382c25e93e758be0055465a5 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Wed, 23 Nov 2022 21:11:12 +0100 Subject: [PATCH] Fix SQL Server bulk insert edge case Fixes #29539 --- .../SqlServerModificationCommandBatch.cs | 42 ++++++++------ .../SqlServerModificationCommandBatchTest.cs | 58 +++++++++++++++++++ 2 files changed, 82 insertions(+), 18 deletions(-) diff --git a/src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommandBatch.cs b/src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommandBatch.cs index 03cbccdc58a..a61a7a7b27f 100644 --- a/src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommandBatch.cs +++ b/src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommandBatch.cs @@ -124,6 +124,28 @@ private void ApplyPendingBulkInsertCommands() } } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public override bool TryAddCommand(IReadOnlyModificationCommand modificationCommand) + { + // If there are any pending bulk insert commands and the new command is incompatible with them (not an insert, insert into a + // separate table..), apply the pending commands. + if (_pendingBulkInsertCommands.Count > 0 + && (modificationCommand.EntityState != EntityState.Added + || modificationCommand.StoreStoredProcedure is not null + || !CanBeInsertedInSameStatement(_pendingBulkInsertCommands[0], modificationCommand))) + { + ApplyPendingBulkInsertCommands(); + _pendingBulkInsertCommands.Clear(); + } + + return base.TryAddCommand(modificationCommand); + } + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -132,31 +154,15 @@ private void ApplyPendingBulkInsertCommands() /// protected override void AddCommand(IReadOnlyModificationCommand modificationCommand) { + // TryAddCommand above already applied any pending commands if the new command is incompatible with them. + // So if the new command is an insert, just append it to pending, otherwise do the regular add logic. if (modificationCommand.EntityState == EntityState.Added && modificationCommand.StoreStoredProcedure is null) { - if (_pendingBulkInsertCommands.Count > 0 - && !CanBeInsertedInSameStatement(_pendingBulkInsertCommands[0], modificationCommand)) - { - // The new Add command cannot be added to the pending bulk insert commands (e.g. different table). - // Write out the pending commands before starting a new pending chain. - ApplyPendingBulkInsertCommands(); - _pendingBulkInsertCommands.Clear(); - } - _pendingBulkInsertCommands.Add(modificationCommand); AddParameters(modificationCommand); } else { - // If we have any pending bulk insert commands, write them out before the next non-Add command - if (_pendingBulkInsertCommands.Count > 0) - { - // Note that we don't care about the transactionality of the bulk insert SQL, since there's the additional non-Add - // command coming right afterwards, and so a transaction is required in any case. - ApplyPendingBulkInsertCommands(); - _pendingBulkInsertCommands.Clear(); - } - base.AddCommand(modificationCommand); } } diff --git a/test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchTest.cs b/test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchTest.cs index 2739516fa82..0c62bab1051 100644 --- a/test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchTest.cs +++ b/test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchTest.cs @@ -73,6 +73,58 @@ ColumnModificationParameters CreateModificationParameters(string columnName) }; } + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] + public void AddCommand_when_max_parameters_are_reached_with_pending_commands(bool lastCommandPending) + { + var typeMapper = CreateTypeMappingSource(); + var intMapping = typeMapper.FindMapping(typeof(int)); + var paramIndex = 0; + + var batch = CreateBatch(); + + for (var i = 0; i < 20; i++) + { + var pendingCommand = CreateModificationCommand("T1", null, false); + pendingCommand.EntityState = EntityState.Added; + for (var j = 0; j < 100; j++) + { + pendingCommand.AddColumnModification(CreateModificationParameters("col" + j)); + } + + Assert.True(batch.TryAddCommand(pendingCommand)); + } + + // We now have 20 pending commands with a total of 2000 parameters. + // Add another command - either compatible with the pending ones or not - and which also gets us past the 2098 parameter limit. + var command = CreateModificationCommand(lastCommandPending ? "T1" : "T2", null, false); + command.EntityState = EntityState.Added; + for (var i = 0; i < 100; i++) + { + command.AddColumnModification(CreateModificationParameters("col" + i)); + } + + Assert.False(batch.TryAddCommand(command)); + + batch.Complete(moreBatchesExpected: false); + + Assert.Equal(2000, batch.ParameterValues.Count); + Assert.Contains("INSERT", batch.StoreCommand.RelationalCommand.CommandText); + Assert.Equal(20, batch.ResultSetMappings.Count); + + ColumnModificationParameters CreateModificationParameters(string columnName) + => new() + { + ColumnName = columnName, + ColumnType = "integer", + TypeMapping = intMapping, + IsWrite = true, + OriginalValue = 8, + GenerateParameterName = () => "p" + paramIndex++ + }; + } + private class FakeDbContext : DbContext { } @@ -121,5 +173,11 @@ public TestSqlServerModificationCommandBatch(ModificationCommandBatchFactoryDepe public new Dictionary ParameterValues => base.ParameterValues; + + public new RawSqlCommand StoreCommand + => base.StoreCommand; + + public new IList ResultSetMappings + => base.ResultSetMappings; } }