Skip to content

Commit

Permalink
Allow opting out of RETURNING/OUTPUT clauses in SaveChanges
Browse files Browse the repository at this point in the history
Fixes #29916
  • Loading branch information
roji committed Jan 10, 2023
1 parent 16c1380 commit 8f95f80
Show file tree
Hide file tree
Showing 35 changed files with 1,073 additions and 147 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
56 changes: 56 additions & 0 deletions src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
98 changes: 98 additions & 0 deletions src/EFCore.SqlServer/Extensions/SqlServerTableBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ namespace Microsoft.EntityFrameworkCore;
/// </summary>
public static class SqlServerTableBuilderExtensions
{
#region IsTemporal

/// <summary>
/// Configures the table as temporal.
/// </summary>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
4 changes: 2 additions & 2 deletions src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/EFCore.SqlServer/Properties/SqlServerStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading

0 comments on commit 8f95f80

Please sign in to comment.