Refactor ReaderModificationCommandBatch

* Allow flexible/alternative parameter strategies
* Insert commands and update SQL immediately

Closes #27583

src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs

@@ -74,7 +74,7 @@ public CommandBatchPreparer(CommandBatchPreparerDependencies dependencies)
  continue;
  }
 
- if (!batch.AddCommand(modificationCommand))
+ if (!batch.TryAddCommand(modificationCommand))
  {
  if (batch.ModificationCommands.Count == 1
  || batch.ModificationCommands.Count >= _minBatchSize)
@@ -144,7 +144,7 @@ private ModificationCommandBatch StartNewBatch(
  {
  parameterNameGenerator.Reset();
  var batch = Dependencies.ModificationCommandBatchFactory.Create();
- batch.AddCommand(modificationCommand);
+ batch.TryAddCommand(modificationCommand);
  return batch;
  }
 

src/EFCore.Relational/Update/ModificationCommandBatch.cs

@@ -31,7 +31,7 @@ public abstract class ModificationCommandBatch
  /// <see langword="true" /> if the command was successfully added; <see langword="false" /> if there was no
  /// room in the current batch to add the command and it must instead be added to a new batch.
  /// </returns>
- public abstract bool AddCommand(IReadOnlyModificationCommand modificationCommand);
+ public abstract bool TryAddCommand(IReadOnlyModificationCommand modificationCommand);
 
  /// <summary>
  /// Indicates that no more commands will be added to this batch, and prepares it for execution.

src/EFCore.Relational/Update/SingularModificationCommandBatch.cs

@@ -41,6 +41,6 @@ protected override bool CanAddCommand(IReadOnlyModificationCommand modificationC
  /// <returns>
  /// <see langword="true" />
  /// </returns>
- protected override bool IsCommandTextValid()
+ protected override bool IsBatchValid()
  => true;
 }

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

@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections;
-using System.Text;
 using Microsoft.Data.SqlClient;
 using Microsoft.EntityFrameworkCore.SqlServer.Internal;
 
@@ -22,8 +21,7 @@ public class SqlServerModificationCommandBatch : AffectedCountModificationComman
  private const int MaxRowCount = 1000;
  private int _parameterCount = 1; // Implicit parameter for the command text
  private readonly int _maxBatchSize;
- private readonly List<IReadOnlyModificationCommand> _bulkInsertCommands = new();
- private int _commandsLeftToLengthCheck = 50;
+ private readonly List<IReadOnlyModificationCommand> _pendingBulkInsertCommands = new();
 
  /// <summary>
  /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -84,24 +82,8 @@ protected override bool CanAddCommand(IReadOnlyModificationCommand modificationC
  /// 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 bool IsCommandTextValid()
- {
- if (--_commandsLeftToLengthCheck < 0)
- {
- UpdateCachedCommandText();
- var commandTextLength = CachedCommandText.Length;
- if (commandTextLength >= MaxScriptLength)
- {
- return false;
- }
-
- var averageCommandLength = commandTextLength / ModificationCommands.Count;
- var expectedAdditionalCommandCapacity = (MaxScriptLength - commandTextLength) / averageCommandLength;
- _commandsLeftToLengthCheck = Math.Max(1, expectedAdditionalCommandCapacity / 4);
- }
-
- return true;
- }
+ protected override bool IsBatchValid()
+ => SqlBuilder.Length < MaxScriptLength;
 
  /// <summary>
  /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -139,24 +121,26 @@ private static int CountParameters(IReadOnlyModificationCommand modificationComm
  /// 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 ResetCommandText()
+ protected override void Reset()
  {
- base.ResetCommandText();
+ base.Reset();
 
- _bulkInsertCommands.Clear();
+ _pendingBulkInsertCommands.Clear();
  }
 
- private void AppendBulkInsertCommandText(int lastIndex)
+ private void ApplyPendingBulkInsertCommands()
  {
- if (_bulkInsertCommands.Count == 0)
+ if (_pendingBulkInsertCommands.Count == 0)
  {
  return;
  }
 
+ var commandPosition = CommandResultSet.Count;
+
  var wasCachedCommandTextEmpty = IsCachedCommandTextEmpty;
 
  var resultSetMapping = UpdateSqlGenerator.AppendBulkInsertOperation(
- CachedCommandText, _bulkInsertCommands, lastIndex - _bulkInsertCommands.Count, out var resultsContainPositionMapping,
+ SqlBuilder, _pendingBulkInsertCommands, commandPosition, out var resultsContainPositionMapping,
  out var requiresTransaction);
 
  SetRequiresTransaction(!wasCachedCommandTextEmpty || requiresTransaction);
@@ -165,27 +149,29 @@ private void AppendBulkInsertCommandText(int lastIndex)
  {
  if (ResultsPositionalMappingEnabled is null)
  {
- ResultsPositionalMappingEnabled = new BitArray(CommandResultSet.Count);
+ ResultsPositionalMappingEnabled = new BitArray(CommandResultSet.Count + _pendingBulkInsertCommands.Count);
  }
  else
  {
- ResultsPositionalMappingEnabled.Length = CommandResultSet.Count;
+ ResultsPositionalMappingEnabled.Length = CommandResultSet.Count + _pendingBulkInsertCommands.Count;
  }
 
- for (var i = lastIndex - _bulkInsertCommands.Count; i < lastIndex; i++)
+ for (var i = commandPosition; i < commandPosition + _pendingBulkInsertCommands.Count; i++)
  {
  ResultsPositionalMappingEnabled![i] = true;
  }
  }
 
- for (var i = lastIndex - _bulkInsertCommands.Count; i < lastIndex; i++)
+ foreach (var pendingCommand in _pendingBulkInsertCommands)
  {
- CommandResultSet[i] = resultSetMapping;
+ AddParameters(pendingCommand);
+
+ CommandResultSet.Add(resultSetMapping);
  }
 
  if (resultSetMapping != ResultSetMapping.NoResultSet)
  {
- CommandResultSet[lastIndex - 1] = ResultSetMapping.LastInResultSet;
+ CommandResultSet[^1] = ResultSetMapping.LastInResultSet;
  }
  }
 
@@ -195,50 +181,33 @@ private void AppendBulkInsertCommandText(int lastIndex)
  /// 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 UpdateCachedCommandText()
+ protected override void AddCommand(IReadOnlyModificationCommand modificationCommand)
  {
- base.UpdateCachedCommandText();
-
- AppendBulkInsertCommandText(ModificationCommands.Count);
- }
-
- /// <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 UpdateCachedCommandText(int commandPosition)
- {
- var newModificationCommand = ModificationCommands[commandPosition];
-
- if (newModificationCommand.EntityState == EntityState.Added)
+ if (modificationCommand.EntityState == EntityState.Added)
  {
- if (_bulkInsertCommands.Count > 0
- && !CanBeInsertedInSameStatement(_bulkInsertCommands[0], newModificationCommand))
+ 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.
- AppendBulkInsertCommandText(commandPosition);
- _bulkInsertCommands.Clear();
+ ApplyPendingBulkInsertCommands();
+ _pendingBulkInsertCommands.Clear();
  }
 
- _bulkInsertCommands.Add(newModificationCommand);
-
- LastCachedCommandIndex = commandPosition;
+ _pendingBulkInsertCommands.Add(modificationCommand);
  }
  else
  {
  // If we have any pending bulk insert commands, write them out before the next non-Add command
- if (_bulkInsertCommands.Count > 0)
+ 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.
- AppendBulkInsertCommandText(commandPosition);
- _bulkInsertCommands.Clear();
+ ApplyPendingBulkInsertCommands();
+ _pendingBulkInsertCommands.Clear();
  }
 
- base.UpdateCachedCommandText(commandPosition);
+ base.AddCommand(modificationCommand);
  }
  }
 
@@ -252,6 +221,19 @@ private static bool CanBeInsertedInSameStatement(
  && firstCommand.ColumnModifications.Where(o => o.IsRead).Select(o => o.ColumnName).SequenceEqual(
  secondCommand.ColumnModifications.Where(o => o.IsRead).Select(o => o.ColumnName));
 
+ /// <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 void Complete()
+ {
+ ApplyPendingBulkInsertCommands();
+
+ base.Complete();
+ }
+
  /// <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