Fix SQL Server bulk insert edge case

Fixes dotnet#29539

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

@@ -124,6 +124,28 @@ private void ApplyPendingBulkInsertCommands()
         }
     }
 
+    /// <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>
+    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);
+    }
+
     /// <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
@@ -132,31 +154,15 @@ private void ApplyPendingBulkInsertCommands()
     /// </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 (_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);
         }
     }

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;
     }
 }