[release/7.0] Fix SQL Server bulk insert edge case (#29669)

src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommandBatch.cs

@@ -27,6 +27,8 @@ public class SqlServerModificationCommandBatch : AffectedCountModificationComman
 
     private static readonly bool QuirkEnabled29502
         = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29502", out var enabled) && enabled;
+    private static readonly bool QuirkEnabled29539
+        = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29539", out var enabled) && enabled;
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -143,31 +145,64 @@ private void ApplyPendingBulkInsertCommands()
     ///     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.
     /// </summary>
-    protected override void AddCommand(IReadOnlyModificationCommand modificationCommand)
+    public override bool TryAddCommand(IReadOnlyModificationCommand modificationCommand)
     {
-        if (modificationCommand.EntityState == EntityState.Added && modificationCommand.StoreStoredProcedure is null)
+        if (!QuirkEnabled29539)
         {
+            // 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
-                && !CanBeInsertedInSameStatement(_pendingBulkInsertCommands[0], modificationCommand))
+                && (modificationCommand.EntityState != EntityState.Added
+                    || modificationCommand.StoreStoredProcedure is not null
+                    || !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();
             }
+        }
+
+        return base.TryAddCommand(modificationCommand);
+    }
+
+    /// <summary>
+    ///     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.
+    /// </summary>
+    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 (QuirkEnabled29539)
+            {
+                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)
+            if (QuirkEnabled29539)
             {
-                // 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();
+                // 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);

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<string, object> ParameterValues
             => base.ParameterValues;
+
+        public new RawSqlCommand StoreCommand
+            => base.StoreCommand;
+
+        public new IList<ResultSetMapping> ResultSetMappings
+            => base.ResultSetMappings;
     }
 }