Allow opting out of RETURNING/OUTPUT clauses in SaveChanges

Fixes dotnet#29916

Author: roji

src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs

@@ -1458,8 +1458,7 @@ public static bool IsTableExcludedFromMigrations(this IReadOnlyEntityType entity
         }
 
         var ownership = entityType.FindOwnership();
-        if (ownership != null
-            && ownership.IsUnique)
+        if (ownership is { IsUnique: true })
         {
             return ownership.PrincipalEntityType.IsTableExcludedFromMigrations();
         }

src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs

@@ -271,4 +271,60 @@ public static void SetHistoryTableSchema(this IMutableEntityType entityType, str
     /// <returns>The configuration source for the temporal history table schema setting.</returns>
     public static ConfigurationSource? GetHistoryTableSchemaConfigurationSource(this IConventionEntityType entityType)
         => entityType.FindAnnotation(SqlServerAnnotationNames.TemporalHistoryTableSchema)?.GetConfigurationSource();
+
+    /// <summary>
+    ///     Returns a value indicating whether to use the SQL OUTPUT clause when saving changes to the table. The OUTPUT clause is
+    ///     incompatible with certain SQL Server features, such as tables with triggers.
+    /// </summary>
+    /// <param name="entityType">The entity type.</param>
+    /// <returns><see langword="true" /> if the SQL OUTPUT clause is used to save changes to the table.</returns>
+    public static bool GetIsSqlOutputClauseUsed(this IReadOnlyEntityType entityType)
+    {
+        if (entityType.FindAnnotation(SqlServerAnnotationNames.UseSqlOutputClause) is { Value: bool useSqlOutputClause } )
+        {
+            return useSqlOutputClause;
+        }
+
+        if (entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy
+            && entityType.BaseType is not null)
+        {
+            return entityType.GetRootType().GetIsSqlOutputClauseUsed();
+        }
+
+        return true;
+    }
+
+    /// <summary>
+    ///     Sets a value indicating whether to use the SQL OUTPUT clause when saving changes to the table. The OUTPUT clause is incompatible
+    ///     with certain SQL Server features, such as tables with triggers.
+    /// </summary>
+    /// <param name="entityType">The entity type.</param>
+    /// <param name="useSqlOutputClause">The value to set.</param>
+    public static void UseSqlOutputClause(this IMutableEntityType entityType, bool? useSqlOutputClause)
+        => entityType.SetOrRemoveAnnotation(SqlServerAnnotationNames.UseSqlOutputClause, useSqlOutputClause);
+
+    /// <summary>
+    ///     Sets a value indicating whether to use the SQL OUTPUT clause when saving changes to the table. The OUTPUT clause is incompatible
+    ///     with certain SQL Server features, such as tables with triggers.
+    /// </summary>
+    /// <param name="entityType">The entity type.</param>
+    /// <param name="useSqlOutputClause">The value to set.</param>
+    /// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
+    /// <returns>The configured value.</returns>
+    public static bool? UseSqlOutputClause(
+        this IConventionEntityType entityType,
+        bool? useSqlOutputClause,
+        bool fromDataAnnotation = false)
+        => (bool?)entityType.SetOrRemoveAnnotation(
+            SqlServerAnnotationNames.UseSqlOutputClause,
+            useSqlOutputClause,
+            fromDataAnnotation)?.Value;
+
+    /// <summary>
+    ///     Gets the configuration source for whether to use the SQL OUTPUT clause when saving changes to the table.
+    /// </summary>
+    /// <param name="entityType">The entity type.</param>
+    /// <returns>The configuration source for the memory-optimized setting.</returns>
+    public static ConfigurationSource? GetUseSqlOutputClauseConfigurationSource(this IConventionEntityType entityType)
+        => entityType.FindAnnotation(SqlServerAnnotationNames.UseSqlOutputClause)?.GetConfigurationSource();
 }

src/EFCore.SqlServer/Extensions/SqlServerTableBuilderExtensions.cs

@@ -10,6 +10,8 @@ namespace Microsoft.EntityFrameworkCore;
 /// </summary>
 public static class SqlServerTableBuilderExtensions
 {
+    #region IsTemporal
+
     /// <summary>
     ///     Configures the table as temporal.
     /// </summary>
@@ -183,6 +185,10 @@ public static OwnedNavigationTableBuilder<TOwnerEntity, TDependentEntity> IsTemp
         return tableBuilder;
     }
 
+    #endregion IsTemporal
+
+    #region IsMemoryOptimized
+
     /// <summary>
     ///     Configures the table that the entity maps to when targeting SQL Server as memory-optimized.
     /// </summary>
@@ -264,4 +270,96 @@ public static OwnedNavigationTableBuilder<TOwnerEntity, TDependentEntity> IsMemo
 
         return tableBuilder;
     }
+
+    #endregion IsMemoryOptimized
+
+    #region UseSqlOutputClause
+
+    /// <summary>
+    ///     Configures whether to use the SQL OUTPUT clause when saving changes to the table. The OUTPUT clause is incompatible with
+    ///     certain SQL Server features, such as tables with triggers.
+    /// </summary>
+    /// <remarks>
+    ///     See <see href="https://aka.ms/efcore-docs-sqlserver-save-changes-and-output-clause">Using the SQL OUTPUT clause with SQL Server</see>
+    ///     for more information and examples.
+    /// </remarks>
+    /// <param name="tableBuilder">The builder for the table being configured.</param>
+    /// <param name="useSqlOutputClause">A value indicating whether to use the OUTPUT clause when saving changes to the table.</param>
+    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    public static TableBuilder UseSqlOutputClause(
+        this TableBuilder tableBuilder,
+        bool useSqlOutputClause = true)
+    {
+        tableBuilder.Metadata.UseSqlOutputClause(useSqlOutputClause);
+
+        return tableBuilder;
+    }
+
+    /// <summary>
+    ///     Configures whether to use the SQL OUTPUT clause when saving changes to the table. The OUTPUT clause is incompatible with
+    ///     certain SQL Server features, such as tables with triggers.
+    /// </summary>
+    /// <remarks>
+    ///     See <see href="https://aka.ms/efcore-docs-sqlserver-save-changes-and-output-clause">Using the SQL OUTPUT clause with SQL Server</see>
+    ///     for more information and examples.
+    /// </remarks>
+    /// <typeparam name="TEntity">The entity type being configured.</typeparam>
+    /// <param name="tableBuilder">The builder for the table being configured.</param>
+    /// <param name="useSqlOutputClause">A value indicating whether to use the OUTPUT clause when saving changes to the table.</param>
+    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    public static TableBuilder<TEntity> UseSqlOutputClause<TEntity>(
+        this TableBuilder<TEntity> tableBuilder,
+        bool useSqlOutputClause = true)
+        where TEntity : class
+    {
+        tableBuilder.Metadata.UseSqlOutputClause(useSqlOutputClause);
+
+        return tableBuilder;
+    }
+
+    /// <summary>
+    ///     Configures whether to use the SQL OUTPUT clause when saving changes to the table. The OUTPUT clause is incompatible with
+    ///     certain SQL Server features, such as tables with triggers.
+    /// </summary>
+    /// <remarks>
+    ///     See <see href="https://aka.ms/efcore-docs-sqlserver-save-changes-and-output-clause">Using the SQL OUTPUT clause with SQL Server</see>
+    ///     for more information and examples.
+    /// </remarks>
+    /// <param name="tableBuilder">The builder for the table being configured.</param>
+    /// <param name="useSqlOutputClause">A value indicating whether to use the OUTPUT clause when saving changes to the table.</param>
+    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    public static OwnedNavigationTableBuilder UseSqlOutputClause(
+        this OwnedNavigationTableBuilder tableBuilder,
+        bool useSqlOutputClause = true)
+    {
+        tableBuilder.Metadata.UseSqlOutputClause(useSqlOutputClause);
+
+        return tableBuilder;
+    }
+
+    /// <summary>
+    ///     Configures whether to use the SQL OUTPUT clause when saving changes to the table. The OUTPUT clause is incompatible with
+    ///     certain SQL Server features, such as tables with triggers.
+    /// </summary>
+    /// <remarks>
+    ///     See <see href="https://aka.ms/efcore-docs-sqlserver-save-changes-and-output-clause">Using the SQL OUTPUT clause with SQL Server</see>
+    ///     for more information and examples.
+    /// </remarks>
+    /// <typeparam name="TOwnerEntity">The entity type owning the relationship.</typeparam>
+    /// <typeparam name="TDependentEntity">The dependent entity type of the relationship.</typeparam>
+    /// <param name="tableBuilder">The builder for the table being configured.</param>
+    /// <param name="useSqlOutputClause">A value indicating whether to use the OUTPUT clause when saving changes to the table.</param>
+    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    public static OwnedNavigationTableBuilder<TOwnerEntity, TDependentEntity> UseSqlOutputClause<TOwnerEntity, TDependentEntity>(
+        this OwnedNavigationTableBuilder<TOwnerEntity, TDependentEntity> tableBuilder,
+        bool useSqlOutputClause = true)
+        where TOwnerEntity : class
+        where TDependentEntity : class
+    {
+        tableBuilder.Metadata.UseSqlOutputClause(useSqlOutputClause);
+
+        return tableBuilder;
+    }
+
+    #endregion UseSqlOutputClause
 }

src/EFCore.SqlServer/Metadata/Conventions/SqlServerConventionSetBuilder.cs

@@ -54,6 +54,7 @@ public override ConventionSet CreateConventionSet()
         conventionSet.Add(new SqlServerIndexConvention(Dependencies, RelationalDependencies, _sqlGenerationHelper));
         conventionSet.Add(new SqlServerMemoryOptimizedTablesConvention(Dependencies, RelationalDependencies));
         conventionSet.Add(new SqlServerDbFunctionConvention(Dependencies, RelationalDependencies));
+        conventionSet.Add(new SqlServerOutputClauseConvention(Dependencies, RelationalDependencies));
 
         conventionSet.Replace<CascadeDeleteConvention>(
             new SqlServerOnDeleteConvention(Dependencies, RelationalDependencies));

src/EFCore.SqlServer/Metadata/Conventions/SqlServerIndexConvention.cs

@@ -161,8 +161,7 @@ private void SetIndexFilter(IConventionIndexBuilder indexBuilder, bool columnNam
         var index = indexBuilder.Metadata;
         if (index.IsUnique
             && index.IsClustered() != true
-            && GetNullableColumns(index) is List<string> nullableColumns
-            && nullableColumns.Count > 0)
+            && GetNullableColumns(index) is { Count: > 0 } nullableColumns)
         {
             if (columnNameChanged
                 || index.GetFilter() == null)

src/EFCore.SqlServer/Metadata/Conventions/SqlServerOutputClauseConvention.cs

@@ -0,0 +1,123 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// ReSharper disable once CheckNamespace
+namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
+
+/// <summary>
+///     A convention that configures tables with triggers to not use the OUTPUT clause when saving changes.
+/// </summary>
+/// <remarks>
+///     See <see href="https://aka.ms/efcore-docs-conventions">Model building conventions</see>, and
+///     <see href="https://aka.ms/efcore-docs-sqlserver">Accessing SQL Server and SQL Azure databases with EF Core</see>
+///     for more information and examples.
+/// </remarks>
+public class SqlServerOutputClauseConvention
+    : ITriggerAddedConvention, ITriggerRemovedConvention, IEntityTypeBaseTypeChangedConvention
+{
+    /// <summary>
+    ///     Creates a new instance of <see cref="SqlServerDbFunctionConvention" />.
+    /// </summary>
+    /// <param name="dependencies">Parameter object containing dependencies for this convention.</param>
+    /// <param name="relationalDependencies"> Parameter object containing relational dependencies for this convention.</param>
+    public SqlServerOutputClauseConvention(
+        ProviderConventionSetBuilderDependencies dependencies,
+        RelationalConventionSetBuilderDependencies relationalDependencies)
+    {
+        Dependencies = dependencies;
+        RelationalDependencies = relationalDependencies;
+    }
+
+    /// <summary>
+    ///     Dependencies for this service.
+    /// </summary>
+    protected virtual ProviderConventionSetBuilderDependencies Dependencies { get; }
+
+    /// <summary>
+    ///     Relational provider-specific dependencies for this service.
+    /// </summary>
+    protected virtual RelationalConventionSetBuilderDependencies RelationalDependencies { get; }
+
+    /// <inheritdoc />
+    public virtual void ProcessTriggerAdded(IConventionTriggerBuilder triggerBuilder, IConventionContext<IConventionTriggerBuilder> context)
+    {
+        var entityType = triggerBuilder.Metadata.EntityType;
+
+        if (entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy)
+        {
+            entityType = entityType.GetRootType();
+        }
+
+        entityType.UseSqlOutputClause(false);
+    }
+
+    /// <inheritdoc />
+    public virtual void ProcessTriggerRemoved(
+        IConventionEntityTypeBuilder entityTypeBuilder,
+        IConventionTrigger trigger,
+        IConventionContext<IConventionTrigger> context)
+    {
+        var entityType = entityTypeBuilder.Metadata;
+
+        if (entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy
+            && entityType.GetRootType() is { } rootEntityType
+            && !HasAnyTriggers(rootEntityType))
+        {
+            rootEntityType.UseSqlOutputClause(null);
+        }
+        else if (!HasAnyTriggers(entityType))
+        {
+            entityType.UseSqlOutputClause(null);
+        }
+    }
+
+    /// <inheritdoc />
+    public void ProcessEntityTypeBaseTypeChanged(
+        IConventionEntityTypeBuilder entityTypeBuilder,
+        IConventionEntityType? newBaseType,
+        IConventionEntityType? oldBaseType,
+        IConventionContext<IConventionEntityType> context)
+    {
+        var entityType = entityTypeBuilder.Metadata;
+
+        if (entityTypeBuilder.Metadata.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy)
+        {
+            // Update the old TPH root, removing or adding the setting if triggers are found anywhere in the hierarchy
+            if (oldBaseType?.GetRootType() is { } oldRootEntityType
+                && !oldRootEntityType.GetIsSqlOutputClauseUsed()
+                && !HasAnyTriggers(oldRootEntityType))
+            {
+                oldRootEntityType.UseSqlOutputClause(null);
+            }
+
+            if (newBaseType?.GetRootType() is { } newRootEntityType
+                && newRootEntityType.GetIsSqlOutputClauseUsed()
+                && HasAnyTriggers(entityType))
+            {
+                newRootEntityType.UseSqlOutputClause(false);
+            }
+        }
+
+        // If the entity was removed from a TPH hierarchy, we may need to add the setting to the entity itself
+        if (newBaseType is null)
+        {
+            entityType.UseSqlOutputClause(HasAnyTriggers(entityType) ? false : null);
+        }
+    }
+
+    private bool HasAnyTriggers(IConventionEntityType entityType)
+    {
+        if (entityType.GetDeclaredTriggers().Any())
+        {
+            return true;
+        }
+
+        if (entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy
+            && entityType.GetDerivedTypes().Any(t => t.GetDeclaredTriggers().Any()))
+        {
+            return true;
+        }
+
+        return false;
+    }
+}

src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationConvention.cs

@@ -71,8 +71,7 @@ public override void ProcessEntityTypeAnnotationChanged(
         IConventionAnnotation? oldAnnotation,
         IConventionContext<IConventionAnnotation> context)
     {
-        if ((name == SqlServerAnnotationNames.TemporalPeriodStartPropertyName
-                || name == SqlServerAnnotationNames.TemporalPeriodEndPropertyName)
+        if (name is SqlServerAnnotationNames.TemporalPeriodStartPropertyName or SqlServerAnnotationNames.TemporalPeriodEndPropertyName
             && annotation?.Value is string propertyName)
         {
             var periodProperty = entityTypeBuilder.Metadata.FindProperty(propertyName);

src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationNames.cs

@@ -258,4 +258,12 @@ public static class SqlServerAnnotationNames
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public const string ValueGenerationStrategy = Prefix + "ValueGenerationStrategy";
+
+    /// <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 const string UseSqlOutputClause = Prefix + "UseSqlOutputClause";
 }

src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs

@@ -282,10 +282,10 @@
     <value>SQL Server does not support releasing a savepoint.</value>
   </data>
   <data name="SaveChangesFailedBecauseOfComputedColumnWithFunction" xml:space="preserve">
-    <value>Could not save changes because the target table has computed column with a function that performs data access. Please configure your entity type accordingly, see https://aka.ms/efcore-docs-sqlserver-save-changes-and-computed-columns for more information.</value>
+    <value>Could not save changes because the target table has computed column with a function that performs data access. Please configure your table accordingly, see https://aka.ms/efcore-docs-sqlserver-save-changes-and-output-clause for more information.</value>
   </data>
   <data name="SaveChangesFailedBecauseOfTriggers" xml:space="preserve">
-    <value>Could not save changes because the target table has database triggers. Please configure your entity type accordingly, see https://aka.ms/efcore-docs-sqlserver-save-changes-and-triggers for more information.</value>
+    <value>Could not save changes because the target table has database triggers. Please configure your table accordingly, see https://aka.ms/efcore-docs-sqlserver-save-changes-and-output-clause for more information.</value>
   </data>
   <data name="SequenceBadType" xml:space="preserve">
     <value>SQL Server sequences cannot be used to generate values for the property '{property}' on entity type '{entityType}' because the property type is '{propertyType}'. Sequences can only be used with integer properties.</value>