Pack of API review changes (#28490)

* Rename GetReaderFieldValue -> FieldValueGetter
* Rename SelectingUpdateSqlGenerator -> UpdateAndSelectSqlGenerator
* Integrated "are more batches coming" into ModificationCommandBatch (instead of having a wrapping type)
* Renamed trigger name to modelName

src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs

@@ -1093,20 +1093,20 @@ public static bool CanSetComment(
     ///     See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
     /// </remarks>
     /// <param name="entityTypeBuilder">The entity type builder.</param>
-    /// <param name="name">The name of the trigger.</param>
+    /// <param name="modelName">The name of the trigger.</param>
     /// <param name="tableName">The name of the table on which this trigger is defined.</param>
     /// <param name="tableSchema">The schema of the table on which this trigger is defined.</param>
     /// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
     /// <returns>The same builder instance if the check constraint was configured, <see langword="null" /> otherwise.</returns>
     public static IConventionTriggerBuilder? HasTrigger(
         this IConventionEntityTypeBuilder entityTypeBuilder,
-        string name,
+        string modelName,
         string? tableName,
         string? tableSchema,
         bool fromDataAnnotation = false)
         => InternalTriggerBuilder.HasTrigger(
                 entityTypeBuilder.Metadata,
-                name,
+                modelName,
                 tableName,
                 tableSchema,
                 fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention)

src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs

@@ -1700,15 +1700,15 @@ public static IEnumerable<StoreObjectIdentifier> GetMappedStoreObjects(
         Check.DebugAssert(previousDetailedErrorsEnabled == detailedErrorsEnabled, "Differing values of DetailedErrorsEnabled");
 #endif
 
-        var getReadFieldValue = property.GetOrAddRuntimeAnnotationValue(
-            RelationalAnnotationNames.GetReaderFieldValue,
-            static x => CreateGetValueDelegate(x.property, x.detailedErrorsEnabled),
+        var fieldValueGetter = property.GetOrAddRuntimeAnnotationValue(
+            RelationalAnnotationNames.FieldValueGetter,
+            static x => CreateFieldValueGetter(x.property, x.detailedErrorsEnabled),
             (property, detailedErrorsEnabled));
 
-        return getReadFieldValue(relationalReader.DbDataReader, ordinal);
+        return fieldValueGetter(relationalReader.DbDataReader, ordinal);
     }
 
-    private static Func<DbDataReader, int, object?> CreateGetValueDelegate(IProperty property, bool detailedErrorsEnabled)
+    private static Func<DbDataReader, int, object?> CreateFieldValueGetter(IProperty property, bool detailedErrorsEnabled)
     {
         var readerParameter = Expression.Parameter(typeof(DbDataReader), "reader");
         var indexParameter = Expression.Parameter(typeof(int), "index");

src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder.cs

@@ -73,15 +73,15 @@ public virtual OwnedNavigationSplitTableBuilder ExcludeFromMigrations(bool exclu
     /// <summary>
     ///     Configures a database trigger on the table.
     /// </summary>
-    /// <param name="name">The name of the trigger.</param>
+    /// <param name="modelName">The name of the trigger.</param>
     /// <returns>A builder that can be used to configure the database trigger.</returns>
     /// <remarks>
     ///     See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
     /// </remarks>
-    public virtual TriggerBuilder HasTrigger(string name)
+    public virtual TriggerBuilder HasTrigger(string modelName)
         => new((Trigger)InternalTriggerBuilder.HasTrigger(
             (IConventionEntityType)MappingFragment.EntityType,
-            name,
+            modelName,
             Name,
             Schema,
             ConfigurationSource.Explicit)!);

src/EFCore.Relational/Metadata/Builders/OwnedNavigationTableBuilder.cs

@@ -69,15 +69,15 @@ public virtual OwnedNavigationTableBuilder ExcludeFromMigrations(bool excluded =
     /// <summary>
     ///     Configures a database trigger on the table.
     /// </summary>
-    /// <param name="name">The name of the trigger.</param>
+    /// <param name="modelName">The name of the trigger.</param>
     /// <returns>A builder that can be used to configure the database trigger.</returns>
     /// <remarks>
     ///     See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
     /// </remarks>
-    public virtual TriggerBuilder HasTrigger(string name)
+    public virtual TriggerBuilder HasTrigger(string modelName)
         => new((Trigger)InternalTriggerBuilder.HasTrigger(
             (IConventionEntityType)Metadata,
-            name,
+            modelName,
             Name,
             Schema,
             ConfigurationSource.Explicit)!);

src/EFCore.Relational/Metadata/Builders/SplitTableBuilder.cs

@@ -73,15 +73,15 @@ public virtual SplitTableBuilder ExcludeFromMigrations(bool excluded = true)
     /// <summary>
     ///     Configures a database trigger on the table.
     /// </summary>
-    /// <param name="name">The name of the trigger.</param>
+    /// <param name="modelName">The name of the trigger.</param>
     /// <returns>A builder that can be used to configure the database trigger.</returns>
     /// <remarks>
     ///     See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
     /// </remarks>
-    public virtual TriggerBuilder HasTrigger(string name)
+    public virtual TriggerBuilder HasTrigger(string modelName)
         => new((Trigger)InternalTriggerBuilder.HasTrigger(
             (IConventionEntityType)MappingFragment.EntityType,
-            name,
+            modelName,
             Name,
             Schema,
             ConfigurationSource.Explicit)!);

src/EFCore.Relational/Metadata/Builders/TableBuilder.cs

@@ -69,15 +69,15 @@ public virtual TableBuilder ExcludeFromMigrations(bool excluded = true)
     /// <summary>
     ///     Configures a database trigger on the table.
     /// </summary>
-    /// <param name="name">The name of the trigger.</param>
+    /// <param name="modelName">The name of the trigger.</param>
     /// <returns>A builder that can be used to configure the database trigger.</returns>
     /// <remarks>
     ///     See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
     /// </remarks>
-    public virtual TriggerBuilder HasTrigger(string name)
+    public virtual TriggerBuilder HasTrigger(string modelName)
         => new((Trigger)InternalTriggerBuilder.HasTrigger(
             (IConventionEntityType)Metadata,
-            name,
+            modelName,
             Name,
             Schema,
             ConfigurationSource.Explicit)!);

src/EFCore.Relational/Metadata/Internal/InternalTriggerBuilder.cs

@@ -57,13 +57,13 @@ public virtual bool CanSetName(string? name, ConfigurationSource configurationSo
     /// </summary>
     public static IConventionTrigger? HasTrigger(
         IConventionEntityType entityType,
-        string name,
+        string modelName,
         string? tableName,
         string? tableSchema,
         ConfigurationSource configurationSource)
     {
         List<IConventionTrigger>? triggersToBeDetached = null;
-        var trigger = entityType.FindTrigger(name);
+        var trigger = entityType.FindTrigger(modelName);
         if (trigger != null)
         {
             if ((tableName == null && tableSchema == null)
@@ -78,13 +78,13 @@ public virtual bool CanSetName(string? name, ConfigurationSource configurationSo
                 return null;
             }
 
-            entityType.RemoveTrigger(name);
+            entityType.RemoveTrigger(modelName);
         }
         else
         {
             foreach (var derivedType in entityType.GetDerivedTypes())
             {
-                var derivedTrigger = (IConventionTrigger?)Trigger.FindDeclaredTrigger(derivedType, name);
+                var derivedTrigger = (IConventionTrigger?)Trigger.FindDeclaredTrigger(derivedType, modelName);
                 if (derivedTrigger == null)
                 {
                     continue;
@@ -114,7 +114,7 @@ public virtual bool CanSetName(string? name, ConfigurationSource configurationSo
             }
         }
 
-        trigger = new Trigger((IMutableEntityType)entityType, name, tableName, tableSchema, configurationSource);
+        trigger = new Trigger((IMutableEntityType)entityType, modelName, tableName, tableSchema, configurationSource);
 
         if (detachedTriggers != null)
         {

src/EFCore.Relational/Metadata/Internal/Trigger.cs

@@ -29,13 +29,13 @@ public class Trigger : ConventionAnnotatable, IMutableTrigger, IConventionTrigge
     /// </summary>
     public Trigger(
         IMutableEntityType entityType,
-        string name,
+        string modelName,
         string? tableName,
         string? tableSchema,
         ConfigurationSource configurationSource)
     {
         EntityType = entityType;
-        ModelName = name;
+        ModelName = modelName;
         _tableName = tableName;
         _tableSchema = tableSchema;
         _configurationSource = configurationSource;
@@ -47,40 +47,40 @@ public Trigger(
             entityType.SetOrRemoveAnnotation(RelationalAnnotationNames.Triggers, triggers);
         }
 
-        if (triggers.ContainsKey(name))
+        if (triggers.ContainsKey(modelName))
         {
             throw new InvalidOperationException(
                 RelationalStrings.DuplicateTrigger(
-                    name, entityType.DisplayName(), entityType.DisplayName()));
+                    modelName, entityType.DisplayName(), entityType.DisplayName()));
         }
 
-        var baseTrigger = entityType.BaseType?.FindTrigger(name);
+        var baseTrigger = entityType.BaseType?.FindTrigger(modelName);
         if (baseTrigger != null)
         {
             throw new InvalidOperationException(
                 RelationalStrings.DuplicateTrigger(
-                    name, entityType.DisplayName(), baseTrigger.EntityType.DisplayName()));
+                    modelName, entityType.DisplayName(), baseTrigger.EntityType.DisplayName()));
         }
 
         foreach (var derivedType in entityType.GetDerivedTypes())
         {
-            var derivedTrigger = FindTrigger(derivedType, name);
+            var derivedTrigger = FindTrigger(derivedType, modelName);
             if (derivedTrigger != null)
             {
                 throw new InvalidOperationException(
                     RelationalStrings.DuplicateTrigger(
-                        name, entityType.DisplayName(), derivedTrigger.EntityType.DisplayName()));
+                        modelName, entityType.DisplayName(), derivedTrigger.EntityType.DisplayName()));
             }
         }
 
         if (entityType.GetTableName() is null)
         {
-            throw new InvalidOperationException(RelationalStrings.TriggerOnUnmappedEntityType(name, entityType.DisplayName()));
+            throw new InvalidOperationException(RelationalStrings.TriggerOnUnmappedEntityType(modelName, entityType.DisplayName()));
         }
 
         EnsureMutable();
 
-        triggers.Add(name, this);
+        triggers.Add(modelName, this);
 
         _builder = new InternalTriggerBuilder(this, ((IConventionModel)entityType.Model).Builder);
     }
@@ -113,20 +113,20 @@ public static IEnumerable<IReadOnlyTrigger> GetTriggers(IReadOnlyEntityType enti
     /// </summary>
     public static IReadOnlyTrigger? FindTrigger(
         IReadOnlyEntityType entityType,
-        string name)
-        => entityType.BaseType?.FindTrigger(name) ?? FindDeclaredTrigger(entityType, name);
+        string modelName)
+        => entityType.BaseType?.FindTrigger(modelName) ?? FindDeclaredTrigger(entityType, modelName);
 
     /// <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 static IReadOnlyTrigger? FindDeclaredTrigger(IReadOnlyEntityType entityType, string name)
+    public static IReadOnlyTrigger? FindDeclaredTrigger(IReadOnlyEntityType entityType, string modelName)
     {
         var triggers = (SortedDictionary<string, ITrigger>?)entityType[RelationalAnnotationNames.Triggers];
 
-        return triggers is not null && triggers.TryGetValue(name, out var trigger)
+        return triggers is not null && triggers.TryGetValue(modelName, out var trigger)
             ? trigger
             : null;
     }
@@ -137,17 +137,17 @@ public static IEnumerable<IReadOnlyTrigger> GetTriggers(IReadOnlyEntityType enti
     ///     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 static Trigger? RemoveTrigger(IMutableEntityType entityType, string name)
+    public static Trigger? RemoveTrigger(IMutableEntityType entityType, string modelName)
     {
         var triggers = (SortedDictionary<string, ITrigger>?)entityType[RelationalAnnotationNames.Triggers];
         if (triggers == null
-            || !triggers.TryGetValue(name, out var trigger))
+            || !triggers.TryGetValue(modelName, out var trigger))
         {
             return null;
         }
 
         var mutableTrigger = (Trigger)trigger;
-        triggers.Remove(name);
+        triggers.Remove(modelName);
         mutableTrigger.SetRemovedFromModel();
 
         return mutableTrigger;

src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs

@@ -260,7 +260,7 @@ public static class RelationalAnnotationNames
     public const string ModelDependencies = Prefix + "ModelDependencies";
 
     /// <summary>
-    ///     The name for the reader get value delegate annotations.
+    ///     The name for the reader field value getter delegate annotation.
     /// </summary>
-    public const string GetReaderFieldValue = Prefix + "GetReaderFieldValue";
+    public const string FieldValueGetter = Prefix + "FieldValueGetter";
 }

src/EFCore.Relational/Update/IBatchExecutor.cs

@@ -28,21 +28,15 @@ public interface IBatchExecutor
     /// <summary>
     ///     Executes the commands in the batches against the given database connection.
     /// </summary>
-    /// <param name="commandBatches">
-    ///     A list of value tuples, each of which contains a batch to execute, and whether more batches are available.
-    /// </param>
+    /// <param name="commandBatches">The batches to execute.</param>
     /// <param name="connection">The database connection to use.</param>
     /// <returns>The total number of rows affected.</returns>
-    int Execute(
-        IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> commandBatches,
-        IRelationalConnection connection);
+    int Execute(IEnumerable<ModificationCommandBatch> commandBatches, IRelationalConnection connection);
 
     /// <summary>
     ///     Executes the commands in the batches against the given database connection.
     /// </summary>
-    /// <param name="commandBatches">
-    ///     A list of value tuples, each of which contains a batch to execute, and whether more batches are available.
-    /// </param>
+    /// <param name="commandBatches">The batches to execute.</param>
     /// <param name="connection">The database connection to use.</param>
     /// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
     /// <returns>
@@ -51,7 +45,7 @@ int Execute(
     /// </returns>
     /// <exception cref="OperationCanceledException">If the <see cref="CancellationToken" /> is canceled.</exception>
     Task<int> ExecuteAsync(
-        IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> commandBatches,
+        IEnumerable<ModificationCommandBatch> commandBatches,
         IRelationalConnection connection,
         CancellationToken cancellationToken = default);
 }

src/EFCore.Relational/Update/ICommandBatchPreparer.cs

@@ -32,6 +32,6 @@ public interface ICommandBatchPreparer
     /// </summary>
     /// <param name="entries">The entries that represent the entities to be modified.</param>
     /// <param name="updateAdapter">The model data.</param>
-    /// <returns>A list of value tuples, each of which contains a batch to execute, and whether more batches are available.</returns>
-    IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> BatchCommands(IList<IUpdateEntry> entries, IUpdateAdapter updateAdapter);
+    /// <returns>The list of batches to execute.</returns>
+    IEnumerable<ModificationCommandBatch> BatchCommands(IList<IUpdateEntry> entries, IUpdateAdapter updateAdapter);
 }

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

@@ -48,9 +48,7 @@ public BatchExecutor(
     ///     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 virtual int Execute(
-        IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> commandBatches,
-        IRelationalConnection connection)
+    public virtual int Execute(IEnumerable<ModificationCommandBatch> commandBatches, IRelationalConnection connection)
     {
         using var batchEnumerator = commandBatches.GetEnumerator();
 
@@ -59,7 +57,7 @@ public virtual int Execute(
             return 0;
         }
 
-        var (batch, hasMoreBatches) = batchEnumerator.Current;
+        var batch = batchEnumerator.Current;
 
         var rowsAffected = 0;
         var transaction = connection.CurrentTransaction;
@@ -73,7 +71,7 @@ public virtual int Execute(
                 && transactionEnlistManager?.CurrentAmbientTransaction is null
                 && CurrentContext.Context.Database.AutoTransactionsEnabled
                 // Don't start a transaction if we have a single batch which doesn't require a transaction (single command), for perf.
-                && (hasMoreBatches || batch.RequiresTransaction))
+                && (batch.AreMoreBatchesExpected || batch.RequiresTransaction))
             {
                 transaction = connection.BeginTransaction();
                 beganTransaction = true;
@@ -92,7 +90,7 @@ public virtual int Execute(
 
             do
             {
-                batch = batchEnumerator.Current.Batch;
+                batch = batchEnumerator.Current;
                 batch.Execute(connection);
                 rowsAffected += batch.ModificationCommands.Count;
             }
@@ -156,7 +154,7 @@ public virtual int Execute(
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public virtual async Task<int> ExecuteAsync(
-        IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> commandBatches,
+        IEnumerable<ModificationCommandBatch> commandBatches,
         IRelationalConnection connection,
         CancellationToken cancellationToken = default)
     {
@@ -167,7 +165,7 @@ public virtual async Task<int> ExecuteAsync(
             return 0;
         }
 
-        var (batch, hasMoreBatches) = batchEnumerator.Current;
+        var batch = batchEnumerator.Current;
 
         var rowsAffected = 0;
         var transaction = connection.CurrentTransaction;
@@ -181,7 +179,7 @@ public virtual async Task<int> ExecuteAsync(
                 && transactionEnlistManager?.CurrentAmbientTransaction is null
                 && CurrentContext.Context.Database.AutoTransactionsEnabled
                 // Don't start a transaction if we have a single batch which doesn't require a transaction (single command), for perf.
-                && (hasMoreBatches || batch.RequiresTransaction))
+                && (batch.AreMoreBatchesExpected || batch.RequiresTransaction))
             {
                 transaction = await connection.BeginTransactionAsync(cancellationToken).ConfigureAwait(false);
                 beganTransaction = true;
@@ -200,7 +198,7 @@ public virtual async Task<int> ExecuteAsync(
 
             do
             {
-                batch = batchEnumerator.Current.Batch;
+                batch = batchEnumerator.Current;
                 await batch.ExecuteAsync(connection, cancellationToken).ConfigureAwait(false);
                 rowsAffected += batch.ModificationCommands.Count;
             }