Fix bugs in SQL Server bulk insert management (#28713)

Fixes #27840
Fixes #28712

All.sln.DotSettings

@@ -320,6 +320,7 @@ The .NET Foundation licenses this file to you under the MIT license.
  <s:Boolean x:Key="/Default/UserDictionary/Words/=sqlite/@EntryIndexedValue">True</s:Boolean>
  <s:Boolean x:Key="/Default/UserDictionary/Words/=subqueries/@EntryIndexedValue">True</s:Boolean>
  <s:Boolean x:Key="/Default/UserDictionary/Words/=subquery/@EntryIndexedValue">True</s:Boolean>
+ <s:Boolean x:Key="/Default/UserDictionary/Words/=transactionality/@EntryIndexedValue">True</s:Boolean>
  <s:Boolean x:Key="/Default/UserDictionary/Words/=unconfigured/@EntryIndexedValue">True</s:Boolean>
  <s:Boolean x:Key="/Default/UserDictionary/Words/=unignore/@EntryIndexedValue">True</s:Boolean>
  <s:Boolean x:Key="/Default/UserDictionary/Words/=fixup/@EntryIndexedValue">True</s:Boolean>

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

@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Collections;
 using Microsoft.Data.SqlClient;
 using Microsoft.EntityFrameworkCore.SqlServer.Internal;
 
@@ -15,9 +14,15 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Update.Internal;
 /// </summary>
 public class SqlServerModificationCommandBatch : AffectedCountModificationCommandBatch
 {
+ // https://docs.microsoft.com/sql/sql-server/maximum-capacity-specifications-for-sql-server
  private const int DefaultNetworkPacketSizeBytes = 4096;
  private const int MaxScriptLength = 65536 * DefaultNetworkPacketSizeBytes / 2;
- private const int MaxParameterCount = 2100;
+
+ /// <summary>
+ /// The SQL Server limit on parameters, including two extra parameters to sp_executesql (@stmt and @params).
+ /// </summary>
+ private const int MaxParameterCount = 2100 - 2;
+
  private readonly List<IReadOnlyModificationCommand> _pendingBulkInsertCommands = new();
 
  /// <summary>
@@ -53,7 +58,6 @@ protected override void RollbackLastCommand()
  if (_pendingBulkInsertCommands.Count > 0)
  {
  _pendingBulkInsertCommands.RemoveAt(_pendingBulkInsertCommands.Count - 1);
- return;
  }
 
  base.RollbackLastCommand();
@@ -66,9 +70,30 @@ protected override void RollbackLastCommand()
  /// doing so can result in application failures when updating to a new Entity Framework Core release.
  /// </summary>
  protected override bool IsValid()
- => SqlBuilder.Length < MaxScriptLength
- // A single implicit parameter for the command text itself
- && ParameterValues.Count + 1 < MaxParameterCount;
+ {
+ if (ParameterValues.Count > MaxParameterCount)
+ {
+ return false;
+ }
+
+ var sqlLength = SqlBuilder.Length;
+
+ if (_pendingBulkInsertCommands.Count > 0)
+ {
+ // Conservative heuristic for the length of the pending bulk insert commands.
+ // See EXEC sp_server_info.
+ var numColumns = _pendingBulkInsertCommands[0].ColumnModifications.Count;
+
+ sqlLength +=
+ numColumns * 128 // column name lengths
+ + 128 // schema name length
+ + 128 // table name length
+ + _pendingBulkInsertCommands.Count * numColumns * 6 // column parameter placeholders
+ + 300; // some extra fixed overhead
+ }
+
+ return sqlLength < MaxScriptLength;
+ }
 
  private void ApplyPendingBulkInsertCommands()
  {
@@ -86,10 +111,8 @@ private void ApplyPendingBulkInsertCommands()
 
  SetRequiresTransaction(!wasCachedCommandTextEmpty || requiresTransaction);
 
- foreach (var pendingCommand in _pendingBulkInsertCommands)
+ for (var i = 0; i < _pendingBulkInsertCommands.Count; i++)
  {
- AddParameters(pendingCommand);
-
  CommandResultSet.Add(resultSetMapping);
  }
 
@@ -119,6 +142,7 @@ protected override void AddCommand(IReadOnlyModificationCommand modificationComm
  }
 
  _pendingBulkInsertCommands.Add(modificationCommand);
+ AddParameters(modificationCommand);
  }
  else
  {

test/EFCore.SqlServer.FunctionalTests/Update/StoredProcedureUpdateSqlServerTest.cs

@@ -12,7 +12,7 @@ public StoredProcedureUpdateSqlServerTest(StoredProcedureUpdateSqlServerFixture
  : base(fixture)
  {
  Fixture.TestSqlLoggerFactory.Clear();
- Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
+ // Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
  }
 
  public override async Task Insert_with_output_parameter(bool async)

test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchTest.cs

@@ -11,20 +11,33 @@ namespace Microsoft.EntityFrameworkCore.Update;
 
 public class SqlServerModificationCommandBatchTest
 {
- [ConditionalFact]
- public void AddCommand_returns_false_when_max_batch_size_is_reached()
+ [ConditionalTheory]
+ [InlineData(EntityState.Added)]
+ [InlineData(EntityState.Deleted)]
+ [InlineData(EntityState.Modified)]
+ public void AddCommand_returns_false_when_max_batch_size_is_reached(EntityState entityState)
  {
  var batch = CreateBatch(maxBatchSize: 1);
 
  var firstCommand = CreateModificationCommand("T1", null, false);
+ firstCommand.EntityState = entityState;
+ var secondCommand = CreateModificationCommand("T1", null, false);
+ secondCommand.EntityState = entityState;
+
  Assert.True(batch.TryAddCommand(firstCommand));
- Assert.False(batch.TryAddCommand(CreateModificationCommand("T1", null, false)));
+ Assert.False(batch.TryAddCommand(secondCommand));
 
  Assert.Same(firstCommand, Assert.Single(batch.ModificationCommands));
  }
 
- [ConditionalFact]
- public void AddCommand_returns_false_when_max_parameters_are_reached()
+ [ConditionalTheory]
+ [InlineData(EntityState.Added, true)]
+ [InlineData(EntityState.Added, false)]
+ [InlineData(EntityState.Deleted, true)]
+ [InlineData(EntityState.Deleted, false)]
+ [InlineData(EntityState.Modified, true)]
+ [InlineData(EntityState.Modified, false)]
+ public void AddCommand_returns_false_when_max_parameters_are_reached(EntityState entityState, bool withSameTable)
  {
  var typeMapper = CreateTypeMappingSource();
  var intMapping = typeMapper.FindMapping(typeof(int));
@@ -33,13 +46,15 @@ public void AddCommand_returns_false_when_max_parameters_are_reached()
  var batch = CreateBatch();
 
  var command = CreateModificationCommand("T1", null, false);
+ command.EntityState = entityState;
  for (var i = 0; i < 2098; i++)
  {
  command.AddColumnModification(CreateModificationParameters("col" + i));
  }
  Assert.True(batch.TryAddCommand(command));
 
  var secondCommand = CreateModificationCommand("T2", null, false);
+ secondCommand.EntityState = entityState;
  secondCommand.AddColumnModification(CreateModificationParameters("col"));
  Assert.False(batch.TryAddCommand(secondCommand));
  Assert.Same(command, Assert.Single(batch.ModificationCommands));