Refactor ReaderModificationCommandBatch (#27584)

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

Closes #27583

src/EFCore.Relational/Storage/IRelationalCommandBuilder.cs

@@ -30,6 +30,13 @@ public interface IRelationalCommandBuilder
     /// <returns>The same builder instance so that multiple calls can be chained.</returns>
     IRelationalCommandBuilder AddParameter(IRelationalParameter parameter);
 
+    /// <summary>
+    ///     Removes the parameter with the given index from this command.
+    /// </summary>
+    /// <param name="index">The index of the parameter to be removed.</param>
+    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    IRelationalCommandBuilder RemoveParameterAt(int index);
+
     /// <summary>
     ///     The source for <see cref="RelationalTypeMapping" />s to use.
     /// </summary>

src/EFCore.Relational/Storage/RelationalCommandBuilder.cs

@@ -3,19 +3,7 @@
 
 namespace Microsoft.EntityFrameworkCore.Storage;
 
-/// <summary>
-///     <para>
-///         Builds a command to be executed against a relational database.
-///     </para>
-///     <para>
-///         This type is typically used by database providers (and other extensions). It is generally
-///         not used in application code.
-///     </para>
-/// </summary>
-/// <remarks>
-///     See <see href="https://aka.ms/efcore-docs-providers">Implementation of database providers and extensions</see>
-///     for more information and examples.
-/// </remarks>
+/// <inheritdoc />
 public class RelationalCommandBuilder : IRelationalCommandBuilder
 {
     private readonly List<IRelationalParameter> _parameters = new();
@@ -42,17 +30,12 @@ public RelationalCommandBuilder(
     /// </summary>
     protected virtual RelationalCommandBuilderDependencies Dependencies { get; }
 
-    /// <summary>
-    ///     The source for <see cref="RelationalTypeMapping" />s to use.
-    /// </summary>
+    /// <inheritdoc />
     [Obsolete("Code trying to add parameter should add type mapped parameter using TypeMappingSource directly.")]
     public virtual IRelationalTypeMappingSource TypeMappingSource
         => Dependencies.TypeMappingSource;
 
-    /// <summary>
-    ///     Creates the command.
-    /// </summary>
-    /// <returns>The newly created command.</returns>
+    /// <inheritdoc />
     public virtual IRelationalCommand Build()
         => new RelationalCommand(Dependencies, _commandTextBuilder.ToString(), Parameters);
 
@@ -62,72 +45,59 @@ public virtual IRelationalCommand Build()
     public override string ToString()
         => _commandTextBuilder.ToString();
 
-    /// <summary>
-    ///     The collection of parameters.
-    /// </summary>
+    /// <inheritdoc />
     public virtual IReadOnlyList<IRelationalParameter> Parameters
         => _parameters;
 
-    /// <summary>
-    ///     Adds the given parameter to this command.
-    /// </summary>
-    /// <param name="parameter">The parameter.</param>
-    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    /// <inheritdoc />
     public virtual IRelationalCommandBuilder AddParameter(IRelationalParameter parameter)
     {
         _parameters.Add(parameter);
 
         return this;
     }
 
-    /// <summary>
-    ///     Appends an object to the command text.
-    /// </summary>
-    /// <param name="value">The object to be written.</param>
-    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    /// <inheritdoc />
+    public virtual IRelationalCommandBuilder RemoveParameterAt(int index)
+    {
+        _parameters.RemoveAt(index);
+
+        return this;
+    }
+
+    /// <inheritdoc />
     public virtual IRelationalCommandBuilder Append(string value)
     {
         _commandTextBuilder.Append(value);
 
         return this;
     }
 
-    /// <summary>
-    ///     Appends a blank line to the command text.
-    /// </summary>
-    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    /// <inheritdoc />
     public virtual IRelationalCommandBuilder AppendLine()
     {
         _commandTextBuilder.AppendLine();
 
         return this;
     }
 
-    /// <summary>
-    ///     Increments the indent of subsequent lines.
-    /// </summary>
-    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    /// <inheritdoc />
     public virtual IRelationalCommandBuilder IncrementIndent()
     {
         _commandTextBuilder.IncrementIndent();
 
         return this;
     }
 
-    /// <summary>
-    ///     Decrements the indent of subsequent lines.
-    /// </summary>
-    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    /// <inheritdoc />
     public virtual IRelationalCommandBuilder DecrementIndent()
     {
         _commandTextBuilder.DecrementIndent();
 
         return this;
     }
 
-    /// <summary>
-    ///     Gets the length of the command text.
-    /// </summary>
+    /// <inheritdoc />
     public virtual int CommandTextLength
         => _commandTextBuilder.Length;
 }

src/EFCore.Relational/Update/ColumnModification.cs

@@ -55,110 +55,72 @@ public ColumnModification(in ColumnModificationParameters columnModificationPara
         UseParameter = _generateParameterName != null;
     }
 
-    /// <summary>
-    ///     The <see cref="IUpdateEntry" /> that represents the entity that is being modified.
-    /// </summary>
+    /// <inheritdoc />
     public virtual IUpdateEntry? Entry { get; }
 
-    /// <summary>
-    ///     The property that maps to the column.
-    /// </summary>
+    /// <inheritdoc />
     public virtual IProperty? Property { get; }
 
-    /// <summary>
-    ///     The relational type mapping for the column.
-    /// </summary>
+    /// <inheritdoc />
     public virtual RelationalTypeMapping? TypeMapping { get; }
 
-    /// <summary>
-    ///     A value indicating whether the column could contain a null value.
-    /// </summary>
+    /// <inheritdoc />
     public virtual bool? IsNullable { get; }
 
-    /// <summary>
-    ///     Indicates whether a value must be read from the database for the column.
-    /// </summary>
+    /// <inheritdoc />
     public virtual bool IsRead { get; }
 
-    /// <summary>
-    ///     Indicates whether a value must be written to the database for the column.
-    /// </summary>
+    /// <inheritdoc />
     public virtual bool IsWrite { get; }
 
-    /// <summary>
-    ///     Indicates whether the column is used in the <c>WHERE</c> clause when updating.
-    /// </summary>
+    /// <inheritdoc />
     public virtual bool IsCondition { get; }
 
-    /// <summary>
-    ///     Indicates whether the column is part of a primary or alternate key.
-    /// </summary>
+    /// <inheritdoc />
     public virtual bool IsKey { get; }
 
-    /// <summary>
-    ///     Indicates whether the original value of the property must be passed as a parameter to the SQL.
-    /// </summary>
+    /// <inheritdoc />
     public virtual bool UseOriginalValueParameter
         => UseParameter && UseOriginalValue;
 
-    /// <summary>
-    ///     Indicates whether the current value of the property must be passed as a parameter to the SQL.
-    /// </summary>
+    /// <inheritdoc />
     public virtual bool UseCurrentValueParameter
         => UseParameter && UseCurrentValue;
 
-    /// <summary>
-    ///     Indicates whether the original value of the property should be used.
-    /// </summary>
+    /// <inheritdoc />
     public virtual bool UseOriginalValue
         => IsCondition;
 
-    /// <summary>
-    ///     Indicates whether the current value of the property should be used.
-    /// </summary>
+    /// <inheritdoc />
     public virtual bool UseCurrentValue
         => IsWrite;
 
-    /// <summary>
-    ///     Indicates whether the value of the property must be passed as a parameter to the SQL as opposed to being inlined.
-    /// </summary>
+    /// <inheritdoc />
     public virtual bool UseParameter { get; }
 
-    /// <summary>
-    ///     The parameter name to use for the current value parameter (<see cref="UseCurrentValueParameter" />), if needed.
-    /// </summary>
+    /// <inheritdoc />
     public virtual string? ParameterName
         => _parameterName ??= UseCurrentValueParameter ? _generateParameterName!() : null;
 
-    /// <summary>
-    ///     The parameter name to use for the original value parameter (<see cref="UseOriginalValueParameter" />), if needed.
-    /// </summary>
+    /// <inheritdoc />
     public virtual string? OriginalParameterName
         => _originalParameterName ??= UseOriginalValueParameter ? _generateParameterName!() : null;
 
-    /// <summary>
-    ///     The name of the column.
-    /// </summary>
+    /// <inheritdoc />
     public virtual string ColumnName { get; }
 
-    /// <summary>
-    ///     The database type of the column.
-    /// </summary>
+    /// <inheritdoc />
     public virtual string? ColumnType { get; }
 
-    /// <summary>
-    ///     The original value of the property mapped to this column.
-    /// </summary>
+    /// <inheritdoc />
     public virtual object? OriginalValue
         => Entry == null
             ? _originalValue
             : Entry.SharedIdentityEntry == null
                 ? Entry.GetOriginalValue(Property!)
                 : Entry.SharedIdentityEntry.GetOriginalValue(Property!);
 
-    /// <summary>
-    ///     Gets or sets the current value of the property mapped to this column.
-    /// </summary>
+    /// <inheritdoc />
     public virtual object? Value
     {
         get => Entry == null
@@ -186,10 +148,7 @@ public virtual object? Value
         }
     }
 
-    /// <summary>
-    ///     Adds a modification affecting the same database value.
-    /// </summary>
-    /// <param name="modification">The modification for the shared column.</param>
+    /// <inheritdoc />
     public virtual void AddSharedColumnModification(IColumnModification modification)
     {
         Check.DebugAssert(Entry is not null, "Entry is not null");
@@ -258,4 +217,8 @@ public virtual void AddSharedColumnModification(IColumnModification modification
 
         _sharedColumnModifications.Add(modification);
     }
+
+    /// <inheritdoc />
+    public virtual void ResetParameterNames()
+        => _parameterName = _originalParameterName = null;
 }

src/EFCore.Relational/Update/IColumnModification.cs

@@ -122,4 +122,9 @@ public interface IColumnModification
     /// </summary>
     /// <param name="modification">The modification for the shared column.</param>
     public void AddSharedColumnModification(IColumnModification modification);
+
+    /// <summary>
+    ///     Resets parameter names, so they can be regenerated if the command needs to be re-added to a new batch.
+    /// </summary>
+    public void ResetParameterNames();
 }

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

@@ -24,14 +24,14 @@ public abstract class ModificationCommandBatch
     public abstract IReadOnlyList<IReadOnlyModificationCommand> ModificationCommands { get; }
 
     /// <summary>
-    ///     Adds the given insert/update/delete <see cref="ModificationCommands" /> to the batch.
+    ///     Attempts to adds the given insert/update/delete <paramref name="modificationCommand" /> to the batch.
     /// </summary>
     /// <param name="modificationCommand">The command to add.</param>
     /// <returns>
     ///     <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.