From 633dc0e70e88d49c9565d7c18273a00a4f45d6b5 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Fri, 25 Nov 2022 17:33:23 +0100 Subject: [PATCH] Allow opting out of RETURNING/OUTPUT clauses in SaveChanges Fixes #29916 --- .../RelationalEntityTypeExtensions.cs | 3 +- .../RelationalForeignKeyExtensions.cs | 18 +- .../Extensions/RelationalTriggerExtensions.cs | 16 +- .../RelationalModelValidator.cs | 10 +- .../OwnedNavigationSplitTableBuilder.cs | 6 + .../Metadata/Builders/SplitTableBuilder.cs | 6 + .../Properties/RelationalStrings.Designer.cs | 8 + .../Properties/RelationalStrings.resx | 3 + .../SqlServerEntityTypeExtensions.cs | 152 +++++++++++++++++ ...rverEntityTypeMappingFragmentExtensions.cs | 53 ++++++ .../SqlServerTableBuilderExtensions.cs | 144 +++++++++++++++- .../Extensions/SqlServerTableExtensions.cs | 34 ++++ .../Internal/SqlServerModelValidator.cs | 26 ++- .../SqlServerConventionSetBuilder.cs | 1 + .../Conventions/SqlServerIndexConvention.cs | 3 +- .../SqlServerOutputClauseConvention.cs | 67 ++++++++ .../SqlServerValueGenerationConvention.cs | 3 +- .../Internal/SqlServerAnnotationNames.cs | 8 + .../Properties/SqlServerStrings.Designer.cs | 14 +- .../Properties/SqlServerStrings.resx | 9 +- .../Internal/SqlServerUpdateSqlGenerator.cs | 36 ++-- .../Extensions/SqliteEntityTypeExtensions.cs | 156 ++++++++++++++++++ ...liteEntityTypeMappingFragmentExtensions.cs | 53 ++++++ .../SqliteServiceCollectionExtensions.cs | 12 +- .../SqliteTableBuilderExtensions.cs | 146 ++++++++++++++++ .../Extensions/SqliteTableExtensions.cs | 34 ++++ .../Internal/SqliteModelValidator.cs | 38 +++++ .../Internal/SqliteAnnotationNames.cs | 8 + .../Properties/SqliteStrings.Designer.cs | 8 + .../Properties/SqliteStrings.resx | 3 + .../SqliteLegacyUpdateSqlGenerator.cs | 89 +--------- .../Internal/SqliteUpdateSqlGenerator.cs | 115 ++++++++++++- .../Metadata/Conventions/ConventionSet.cs | 30 ++++ .../Conventions/ITriggerAddedConvention.cs | 22 +++ .../Conventions/ITriggerRemovedConvention.cs | 24 +++ .../ConventionDispatcher.ConventionScope.cs | 4 + ...entionDispatcher.DelayedConventionScope.cs | 45 +++++ ...tionDispatcher.ImmediateConventionScope.cs | 54 ++++++ .../Internal/ConventionDispatcher.cs | 21 +++ src/EFCore/Metadata/Internal/EntityType.cs | 4 +- .../Migrations/ModelSnapshotSqlServerTest.cs | 6 + .../Internal/CSharpDbContextGeneratorTest.cs | 6 +- .../CSharpRuntimeModelCodeGeneratorTest.cs | 5 +- .../RelationalModelValidatorTest.cs | 15 ++ ...tionIdentityWithoutOutputSqlServerTest.cs} | 10 +- ...tionSequenceWithoutOutputSqlServerTest.cs} | 10 +- ...enerationWithoutOutputSqlServerFixture.cs} | 4 +- ...nerationWithoutOutputSqlServerTestBase.cs} | 6 +- .../SqlServerModelValidatorTest.cs | 26 +++ .../SqlServerOutputClauseConventionTest.cs | 58 +++++++ .../SqlServerBuilderExtensionsTest.cs | 121 ++++++++++++++ .../SqliteMigrationsSqlGeneratorTest.cs | 132 +++++++++------ .../StoreValueGenerationLegacySqliteTest.cs | 22 ++- .../SqliteModelValidatorTest.cs | 15 ++ .../Builders/SqliteBuilderExtensionsTest.cs | 114 +++++++++++++ .../Conventions/ConventionDispatcherTest.cs | 141 ++++++++++++++++ 56 files changed, 1964 insertions(+), 213 deletions(-) create mode 100644 src/EFCore.SqlServer/Extensions/SqlServerEntityTypeMappingFragmentExtensions.cs create mode 100644 src/EFCore.SqlServer/Extensions/SqlServerTableExtensions.cs create mode 100644 src/EFCore.SqlServer/Metadata/Conventions/SqlServerOutputClauseConvention.cs create mode 100644 src/EFCore.Sqlite.Core/Extensions/SqliteEntityTypeExtensions.cs create mode 100644 src/EFCore.Sqlite.Core/Extensions/SqliteEntityTypeMappingFragmentExtensions.cs create mode 100644 src/EFCore.Sqlite.Core/Extensions/SqliteTableBuilderExtensions.cs create mode 100644 src/EFCore.Sqlite.Core/Extensions/SqliteTableExtensions.cs create mode 100644 src/EFCore/Metadata/Conventions/ITriggerAddedConvention.cs create mode 100644 src/EFCore/Metadata/Conventions/ITriggerRemovedConvention.cs rename test/EFCore.SqlServer.FunctionalTests/Update/{StoreValueGenerationIdentityTriggerSqlServerTest.cs => StoreValueGenerationIdentityWithoutOutputSqlServerTest.cs} (95%) rename test/EFCore.SqlServer.FunctionalTests/Update/{StoreValueGenerationSequenceTriggerSqlServerTest.cs => StoreValueGenerationSequenceWithoutOutputSqlServerTest.cs} (96%) rename test/EFCore.SqlServer.FunctionalTests/Update/{StoreValueGenerationTriggerSqlServerFixture.cs => StoreValueGenerationWithoutOutputSqlServerFixture.cs} (81%) rename test/EFCore.SqlServer.FunctionalTests/Update/{StoreValueGenerationTriggerSqlServerTestBase.cs => StoreValueGenerationWithoutOutputSqlServerTestBase.cs} (92%) create mode 100644 test/EFCore.SqlServer.Tests/Metadata/Conventions/SqlServerOutputClauseConventionTest.cs diff --git a/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs b/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs index 98c2acb0ef8..c2606990b1d 100644 --- a/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs +++ b/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(); } diff --git a/src/EFCore.Relational/Extensions/RelationalForeignKeyExtensions.cs b/src/EFCore.Relational/Extensions/RelationalForeignKeyExtensions.cs index fdc751fe5b3..c8f14ee2037 100644 --- a/src/EFCore.Relational/Extensions/RelationalForeignKeyExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalForeignKeyExtensions.cs @@ -154,9 +154,14 @@ public static IEnumerable GetMappedConstraints(this IFore this IReadOnlyForeignKey foreignKey, in StoreObjectIdentifier storeObject) { + if (foreignKey.PrincipalEntityType.GetTableName() is not { } principalTableName) + { + return null; + } + var foreignKeyName = foreignKey.GetConstraintName( storeObject, - StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName()!, foreignKey.PrincipalEntityType.GetSchema())); + StoreObjectIdentifier.Table(principalTableName, foreignKey.PrincipalEntityType.GetSchema())); var rootForeignKey = foreignKey; // Limit traversal to avoid getting stuck in a cycle (validation will throw for these later) @@ -168,11 +173,16 @@ public static IEnumerable GetMappedConstraints(this IFore .FindRowInternalForeignKeys(storeObject) .SelectMany(fk => fk.PrincipalEntityType.GetForeignKeys())) { + principalTableName = otherForeignKey.PrincipalEntityType.GetTableName(); + + if (principalTableName is null) + { + return null; + } + if (otherForeignKey.GetConstraintName( storeObject, - StoreObjectIdentifier.Table( - otherForeignKey.PrincipalEntityType.GetTableName()!, - otherForeignKey.PrincipalEntityType.GetSchema())) + StoreObjectIdentifier.Table(principalTableName, otherForeignKey.PrincipalEntityType.GetSchema())) == foreignKeyName) { linkedForeignKey = otherForeignKey; diff --git a/src/EFCore.Relational/Extensions/RelationalTriggerExtensions.cs b/src/EFCore.Relational/Extensions/RelationalTriggerExtensions.cs index 97ff9ebaf1d..13dfb438d11 100644 --- a/src/EFCore.Relational/Extensions/RelationalTriggerExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalTriggerExtensions.cs @@ -108,9 +108,19 @@ public static void SetDatabaseName(this IMutableTrigger trigger, string? name) /// /// The trigger. /// The name of the table on which this trigger is defined. - public static string? GetTableName(this IReadOnlyTrigger trigger) - => (string?)trigger.FindAnnotation(RelationalAnnotationNames.TableName)?.Value - ?? trigger.EntityType.GetTableName()!; + public static string GetTableName(this IReadOnlyTrigger trigger) + { + if (trigger.FindAnnotation(RelationalAnnotationNames.TableName) is { Value: string tableName }) + { + return tableName; + } + + var mainTableName = trigger.EntityType.GetTableName(); + + Check.DebugAssert(mainTableName is not null, "Trigger defined on entity not mapped to a table"); + + return mainTableName; + } /// /// Sets the name of the table on which this trigger is defined. diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index 152be9c0a67..a463a94d67b 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -2474,8 +2474,16 @@ protected override void ValidateTriggers( IModel model, IDiagnosticsLogger logger) { - foreach (var entityType in model.GetEntityTypes()) + foreach (var entityType in model.GetEntityTypes().Where(e => e.GetDeclaredTriggers().Any())) { + if (entityType.BaseType is not null + && entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy) + { + throw new InvalidOperationException( + RelationalStrings.CannotConfigureTriggerNonRootTphEntity( + entityType.DisplayName(), entityType.GetRootType().DisplayName())); + } + var tableName = entityType.GetTableName(); var tableSchema = entityType.GetSchema(); diff --git a/src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder.cs b/src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder.cs index 4628d5fb0ce..d74937c0ba4 100644 --- a/src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder.cs +++ b/src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder.cs @@ -57,6 +57,12 @@ public virtual string? Schema public virtual IMutableEntityTypeMappingFragment MappingFragment => InternalMappingFragment; + /// + /// The entity type being configured. + /// + public virtual IMutableEntityType Metadata + => OwnedNavigationBuilder.OwnedEntityType; + private OwnedNavigationBuilder OwnedNavigationBuilder { get; } /// diff --git a/src/EFCore.Relational/Metadata/Builders/SplitTableBuilder.cs b/src/EFCore.Relational/Metadata/Builders/SplitTableBuilder.cs index 62e276098f9..594e3b249d7 100644 --- a/src/EFCore.Relational/Metadata/Builders/SplitTableBuilder.cs +++ b/src/EFCore.Relational/Metadata/Builders/SplitTableBuilder.cs @@ -57,6 +57,12 @@ public virtual string? Schema public virtual IMutableEntityTypeMappingFragment MappingFragment => InternalMappingFragment; + /// + /// The entity type being configured. + /// + public virtual IMutableEntityType Metadata + => EntityTypeBuilder.Metadata; + private EntityTypeBuilder EntityTypeBuilder { get; } /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 758e4278d1b..5de1ff3e3fa 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -53,6 +53,14 @@ public static string BadSequenceType public static string CannotChangeWhenOpen => GetString("CannotChangeWhenOpen"); + /// + /// Can't configure a trigger on entity type '{entityType}', which is in a TPH hierarchy and isn't the root. Configure the trigger on the TPH root entity type '{rootEntityType}' instead. + /// + public static string CannotConfigureTriggerNonRootTphEntity(object? entityType, object? rootEntityType) + => string.Format( + GetString("CannotConfigureTriggerNonRootTphEntity", nameof(entityType), nameof(rootEntityType)), + entityType, rootEntityType); + /// /// Unable to translate the given 'GroupBy' pattern. Call 'AsEnumerable' before 'GroupBy' to evaluate it client-side. /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index bff3e0ea56d..e89cace8c09 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -130,6 +130,9 @@ The instance of DbConnection is currently in use. The connection can only be changed when the existing connection is not being used. + + Can't configure a trigger on entity type '{entityType}', which is in a TPH hierarchy and isn't the root. Configure the trigger on the TPH root entity type '{rootEntityType}' instead. + Unable to translate the given 'GroupBy' pattern. Call 'AsEnumerable' before 'GroupBy' to evaluate it client-side. diff --git a/src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs b/src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs index 57d28e17e22..e0a201d711b 100644 --- a/src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs +++ b/src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.EntityFrameworkCore.SqlServer.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Metadata.Internal; // ReSharper disable once CheckNamespace @@ -18,6 +19,8 @@ public static class SqlServerEntityTypeExtensions { private const string DefaultHistoryTableNameSuffix = "History"; + #region Memory-optimized table + /// /// Returns a value indicating whether the entity type is mapped to a memory-optimized table. /// @@ -58,6 +61,10 @@ public static void SetIsMemoryOptimized(this IMutableEntityType entityType, bool public static ConfigurationSource? GetIsMemoryOptimizedConfigurationSource(this IConventionEntityType entityType) => entityType.FindAnnotation(SqlServerAnnotationNames.MemoryOptimized)?.GetConfigurationSource(); + #endregion Memory-optimized table + + #region Temporal table + /// /// Returns a value indicating whether the entity type is mapped to a temporal table. /// @@ -271,4 +278,149 @@ public static void SetHistoryTableSchema(this IMutableEntityType entityType, str /// The configuration source for the temporal history table schema setting. public static ConfigurationSource? GetHistoryTableSchemaConfigurationSource(this IConventionEntityType entityType) => entityType.FindAnnotation(SqlServerAnnotationNames.TemporalHistoryTableSchema)?.GetConfigurationSource(); + + #endregion Temporal table + + #region SQL OUTPUT clause + + /// + /// 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. + /// + /// The entity type. + /// if the SQL OUTPUT clause is used to save changes to the table. + public static bool IsSqlOutputClauseUsed(this IReadOnlyEntityType entityType) + { + if (entityType.FindAnnotation(SqlServerAnnotationNames.UseSqlOutputClause) is { Value: bool useSqlOutputClause }) + { + return useSqlOutputClause; + } + + if (entityType.FindOwnership() is { } ownership + && StoreObjectIdentifier.Create(entityType, StoreObjectType.Table) is { } tableIdentifier + && ownership.FindSharedObjectRootForeignKey(tableIdentifier) is { } rootForeignKey) + { + return rootForeignKey.PrincipalEntityType.IsSqlOutputClauseUsed(); + } + + if (entityType.BaseType is not null && entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy) + { + return entityType.GetRootType().IsSqlOutputClauseUsed(); + } + + return true; + } + + /// + /// 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. + /// + /// The entity type. + /// The value to set. + public static void UseSqlOutputClause(this IMutableEntityType entityType, bool? useSqlOutputClause) + => entityType.SetOrRemoveAnnotation(SqlServerAnnotationNames.UseSqlOutputClause, useSqlOutputClause); + + /// + /// 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. + /// + /// The entity type. + /// The value to set. + /// Indicates whether the configuration was specified using a data annotation. + /// The configured value. + public static bool? UseSqlOutputClause( + this IConventionEntityType entityType, + bool? useSqlOutputClause, + bool fromDataAnnotation = false) + => (bool?)entityType.SetOrRemoveAnnotation( + SqlServerAnnotationNames.UseSqlOutputClause, + useSqlOutputClause, + fromDataAnnotation)?.Value; + + /// + /// Gets the configuration source for whether to use the SQL OUTPUT clause when saving changes to the table. + /// + /// The entity type. + /// The configuration source for the memory-optimized setting. + public static ConfigurationSource? GetUseSqlOutputClauseConfigurationSource(this IConventionEntityType entityType) + => entityType.FindAnnotation(SqlServerAnnotationNames.UseSqlOutputClause)?.GetConfigurationSource(); + + /// + /// Returns a value indicating whether to use the SQL OUTPUT clause when saving changes to the specified table. + /// The OUTPUT clause is incompatible with certain SQL Server features, such as tables with triggers. + /// + /// The entity type. + /// The identifier of the table-like store object. + /// A value indicating whether the SQL OUTPUT clause is used to save changes to the associated table. + public static bool IsSqlOutputClauseUsed(this IReadOnlyEntityType entityType, in StoreObjectIdentifier storeObject) + { + if (entityType.FindMappingFragment(storeObject) is { } overrides + && overrides.FindAnnotation(SqlServerAnnotationNames.UseSqlOutputClause) is { Value: bool useSqlOutputClause }) + { + return useSqlOutputClause; + } + + if (StoreObjectIdentifier.Create(entityType, storeObject.StoreObjectType) == storeObject) + { + return entityType.IsSqlOutputClauseUsed(); + } + + if (entityType.FindOwnership() is { } ownership + && ownership.FindSharedObjectRootForeignKey(storeObject) is { } rootForeignKey) + { + return rootForeignKey.PrincipalEntityType.IsSqlOutputClauseUsed(storeObject); + } + + if (entityType.BaseType is not null && entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy) + { + return entityType.GetRootType().IsSqlOutputClauseUsed(storeObject); + } + + return true; + } + + /// + /// 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. + /// + /// The entity type. + /// The value to set. + /// The identifier of the table-like store object. + public static void UseSqlOutputClause( + this IMutableEntityType entityType, + bool? useSqlOutputClause, + in StoreObjectIdentifier storeObject) + { + if (StoreObjectIdentifier.Create(entityType, storeObject.StoreObjectType) == storeObject) + { + entityType.UseSqlOutputClause(useSqlOutputClause); + return; + } + + entityType + .GetOrCreateMappingFragment(storeObject) + .UseSqlOutputClause(useSqlOutputClause); + } + + /// + /// 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. + /// + /// The entity type. + /// The value to set. + /// The identifier of the table-like store object. + /// Indicates whether the configuration was specified using a data annotation. + /// The configured value. + public static bool? UseSqlOutputClause( + this IConventionEntityType entityType, + bool? useSqlOutputClause, + in StoreObjectIdentifier storeObject, + bool fromDataAnnotation = false) + => StoreObjectIdentifier.Create(entityType, storeObject.StoreObjectType) == storeObject + ? entityType.UseSqlOutputClause(useSqlOutputClause, fromDataAnnotation) + : entityType + .GetOrCreateMappingFragment(storeObject, fromDataAnnotation) + .UseSqlOutputClause(useSqlOutputClause, fromDataAnnotation); + + #endregion SQL OUTPUT clause } diff --git a/src/EFCore.SqlServer/Extensions/SqlServerEntityTypeMappingFragmentExtensions.cs b/src/EFCore.SqlServer/Extensions/SqlServerEntityTypeMappingFragmentExtensions.cs new file mode 100644 index 00000000000..828d0123a3d --- /dev/null +++ b/src/EFCore.SqlServer/Extensions/SqlServerEntityTypeMappingFragmentExtensions.cs @@ -0,0 +1,53 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.SqlServer.Metadata.Internal; + +// ReSharper disable once CheckNamespace +namespace Microsoft.EntityFrameworkCore; + +/// +/// SQL Server specific extension methods for . +/// +public static class SqlServerEntityTypeMappingFragmentExtensions +{ + /// + /// Returns a value indicating whether to use the SQL OUTPUT clause when saving changes to the associated table. + /// The OUTPUT clause is incompatible with certain SQL Server features, such as tables with triggers. + /// + /// The entity type mapping fragment. + /// The configured value. + public static bool IsSqlOutputClauseUsed(this IReadOnlyEntityTypeMappingFragment fragment) + => fragment.FindAnnotation(SqlServerAnnotationNames.UseSqlOutputClause) is not { Value: false }; + + /// + /// Sets whether to use the SQL OUTPUT clause when saving changes to the associated table. + /// The OUTPUT clause is incompatible with certain SQL Server features, such as tables with triggers. + /// + /// The entity type mapping fragment. + /// The value to set. + public static void UseSqlOutputClause(this IMutableEntityTypeMappingFragment fragment, bool? useSqlOutputClause) + => fragment.SetAnnotation(SqlServerAnnotationNames.UseSqlOutputClause, useSqlOutputClause); + + /// + /// Sets whether to use the SQL OUTPUT clause when saving changes to the associated table. + /// The OUTPUT clause is incompatible with certain SQL Server features, such as tables with triggers. + /// + /// The entity type mapping fragment. + /// The value to set. + /// Indicates whether the configuration was specified using a data annotation. + /// The configured value. + public static bool? UseSqlOutputClause( + this IConventionEntityTypeMappingFragment fragment, + bool? useSqlOutputClause, + bool fromDataAnnotation = false) + => (bool?)fragment.SetAnnotation(SqlServerAnnotationNames.UseSqlOutputClause, useSqlOutputClause, fromDataAnnotation)?.Value; + + /// + /// Gets the configuration source for the setting whether to use the SQL OUTPUT clause when saving changes to the associated table. + /// + /// The entity type mapping fragment. + /// The configuration source for the configured value. + public static ConfigurationSource? GetUseSqlOutputClauseConfigurationSource(this IConventionEntityTypeMappingFragment fragment) + => fragment.FindAnnotation(SqlServerAnnotationNames.UseSqlOutputClause)?.GetConfigurationSource(); +} diff --git a/src/EFCore.SqlServer/Extensions/SqlServerTableBuilderExtensions.cs b/src/EFCore.SqlServer/Extensions/SqlServerTableBuilderExtensions.cs index 42a906cc866..6fe26ea7ace 100644 --- a/src/EFCore.SqlServer/Extensions/SqlServerTableBuilderExtensions.cs +++ b/src/EFCore.SqlServer/Extensions/SqlServerTableBuilderExtensions.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // ReSharper disable once CheckNamespace - namespace Microsoft.EntityFrameworkCore; /// @@ -10,6 +9,8 @@ namespace Microsoft.EntityFrameworkCore; /// public static class SqlServerTableBuilderExtensions { + #region IsTemporal + /// /// Configures the table as temporal. /// @@ -183,6 +184,10 @@ public static OwnedNavigationTableBuilder IsTemp return tableBuilder; } + #endregion IsTemporal + + #region IsMemoryOptimized + /// /// Configures the table that the entity maps to when targeting SQL Server as memory-optimized. /// @@ -264,4 +269,141 @@ public static OwnedNavigationTableBuilder IsMemo return tableBuilder; } + + #endregion IsMemoryOptimized + + #region UseSqlOutputClause + + /// + /// 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. + /// + /// + /// See Using the SQL OUTPUT clause with SQL Server + /// for more information and examples. + /// + /// The builder for the table being configured. + /// A value indicating whether to use the OUTPUT clause when saving changes to the table. + /// The same builder instance so that multiple calls can be chained. + public static TableBuilder UseSqlOutputClause( + this TableBuilder tableBuilder, + bool useSqlOutputClause = true) + { + UseSqlOutputClause(tableBuilder.Metadata, tableBuilder.Name, tableBuilder.Schema, useSqlOutputClause); + + return tableBuilder; + } + + /// + /// 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. + /// + /// + /// See Using the SQL OUTPUT clause with SQL Server + /// for more information and examples. + /// + /// The entity type being configured. + /// The builder for the table being configured. + /// A value indicating whether to use the OUTPUT clause when saving changes to the table. + /// The same builder instance so that multiple calls can be chained. + public static TableBuilder UseSqlOutputClause( + this TableBuilder tableBuilder, + bool useSqlOutputClause = true) + where TEntity : class + => (TableBuilder)((TableBuilder)tableBuilder).UseSqlOutputClause(useSqlOutputClause); + + /// + /// 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. + /// + /// + /// See Using the SQL OUTPUT clause with SQL Server + /// for more information and examples. + /// + /// The builder for the table being configured. + /// A value indicating whether to use the OUTPUT clause when saving changes to the table. + /// The same builder instance so that multiple calls can be chained. + public static SplitTableBuilder UseSqlOutputClause( + this SplitTableBuilder tableBuilder, + bool useSqlOutputClause = true) + { + UseSqlOutputClause(tableBuilder.Metadata, tableBuilder.Name, tableBuilder.Schema, useSqlOutputClause); + + return tableBuilder; + } + + /// + /// 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. + /// + /// + /// See Using the SQL OUTPUT clause with SQL Server + /// for more information and examples. + /// + /// The entity type being configured. + /// The builder for the table being configured. + /// A value indicating whether to use the OUTPUT clause when saving changes to the table. + /// The same builder instance so that multiple calls can be chained. + public static SplitTableBuilder UseSqlOutputClause( + this SplitTableBuilder tableBuilder, + bool useSqlOutputClause = true) + where TEntity : class + => (SplitTableBuilder)((SplitTableBuilder)tableBuilder).UseSqlOutputClause(useSqlOutputClause); + + /// + /// 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. + /// + /// + /// See Using the SQL OUTPUT clause with SQL Server + /// for more information and examples. + /// + /// The builder for the table being configured. + /// A value indicating whether to use the OUTPUT clause when saving changes to the table. + /// The same builder instance so that multiple calls can be chained. + public static OwnedNavigationTableBuilder UseSqlOutputClause( + this OwnedNavigationTableBuilder tableBuilder, + bool useSqlOutputClause = true) + { + UseSqlOutputClause(tableBuilder.Metadata, tableBuilder.Name, tableBuilder.Schema, useSqlOutputClause); + + return tableBuilder; + } + + /// + /// 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. + /// + /// + /// See Using the SQL OUTPUT clause with SQL Server + /// for more information and examples. + /// + /// The entity type owning the relationship. + /// The dependent entity type of the relationship. + /// The builder for the table being configured. + /// A value indicating whether to use the OUTPUT clause when saving changes to the table. + /// The same builder instance so that multiple calls can be chained. + public static OwnedNavigationTableBuilder UseSqlOutputClause( + this OwnedNavigationTableBuilder tableBuilder, + bool useSqlOutputClause = true) + where TOwnerEntity : class + where TDependentEntity : class + => (OwnedNavigationTableBuilder) + ((OwnedNavigationTableBuilder)tableBuilder).UseSqlOutputClause(useSqlOutputClause); + + private static void UseSqlOutputClause(IMutableEntityType entityType, string? tableName, string? tableSchema, bool useSqlOutputClause) + { + if (tableName is null) + { + entityType.UseSqlOutputClause(useSqlOutputClause); + } + else + { + entityType.UseSqlOutputClause( + useSqlOutputClause, + StoreObjectIdentifier.Table(tableName, tableSchema)); + } + } + + #endregion UseSqlOutputClause } diff --git a/src/EFCore.SqlServer/Extensions/SqlServerTableExtensions.cs b/src/EFCore.SqlServer/Extensions/SqlServerTableExtensions.cs new file mode 100644 index 00000000000..bd85d1bb249 --- /dev/null +++ b/src/EFCore.SqlServer/Extensions/SqlServerTableExtensions.cs @@ -0,0 +1,34 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.SqlServer.Metadata.Internal; + +// ReSharper disable once CheckNamespace +namespace Microsoft.EntityFrameworkCore; + +/// +/// SQL Server specific extension methods for . +/// +public static class SqlServerTableExtensions +{ + /// + /// 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. + /// + /// The table. + /// if the SQL OUTPUT clause is used to save changes to the table. + public static bool IsSqlOutputClauseUsed(this ITable table) + { + if (table.FindRuntimeAnnotation(SqlServerAnnotationNames.UseSqlOutputClause) is { Value: bool isSqlOutputClauseUsed } ) + { + return isSqlOutputClauseUsed; + } + + isSqlOutputClauseUsed = table.EntityTypeMappings.All( + e => e.EntityType.IsSqlOutputClauseUsed(StoreObjectIdentifier.Table(table.Name, table.Schema))); + + table.SetRuntimeAnnotation(SqlServerAnnotationNames.UseSqlOutputClause, isSqlOutputClauseUsed); + + return isSqlOutputClauseUsed; + } +} diff --git a/src/EFCore.SqlServer/Infrastructure/Internal/SqlServerModelValidator.cs b/src/EFCore.SqlServer/Infrastructure/Internal/SqlServerModelValidator.cs index c366050fa42..580ab776269 100644 --- a/src/EFCore.SqlServer/Infrastructure/Internal/SqlServerModelValidator.cs +++ b/src/EFCore.SqlServer/Infrastructure/Internal/SqlServerModelValidator.cs @@ -315,11 +315,33 @@ protected override void ValidateSharedTableCompatibility( } } + bool? firstSqlOutputSetting = null; + firstMappedType = null; + foreach (var mappedType in mappedTypes) + { + if (((IConventionEntityType)mappedType).GetUseSqlOutputClauseConfigurationSource() is null) + { + continue; + } + + if (firstSqlOutputSetting is null) + { + (firstSqlOutputSetting, firstMappedType) = (mappedType.IsSqlOutputClauseUsed(), mappedType); + } + else if (mappedType.IsSqlOutputClauseUsed() != firstSqlOutputSetting) + { + throw new InvalidOperationException( + SqlServerStrings.IncompatibleSqlOutputClauseMismatch( + storeObject.DisplayName(), firstMappedType!.DisplayName(), mappedType.DisplayName(), + firstSqlOutputSetting.Value ? firstMappedType.DisplayName() : mappedType.DisplayName(), + !firstSqlOutputSetting.Value ? firstMappedType.DisplayName() : mappedType.DisplayName())); + } + } + if (mappedTypes.Any(t => t.IsTemporal()) && mappedTypes.Select(t => t.GetRootType()).Distinct().Count() > 1) { - // table splitting is only supported when all entites mapped to this table - // have consistent temporal period mappings also + // table splitting is only supported when all entities mapped to this table have consistent temporal period mappings also var expectedPeriodStartColumnName = default(string); var expectedPeriodEndColumnName = default(string); diff --git a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerConventionSetBuilder.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerConventionSetBuilder.cs index adb92dd6dc3..3638209f525 100644 --- a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerConventionSetBuilder.cs +++ b/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( new SqlServerOnDeleteConvention(Dependencies, RelationalDependencies)); diff --git a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerIndexConvention.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerIndexConvention.cs index 65529376aa8..16df1332b66 100644 --- a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerIndexConvention.cs +++ b/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 nullableColumns - && nullableColumns.Count > 0) + && GetNullableColumns(index) is { Count: > 0 } nullableColumns) { if (columnNameChanged || index.GetFilter() == null) diff --git a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOutputClauseConvention.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOutputClauseConvention.cs new file mode 100644 index 00000000000..ba21e538459 --- /dev/null +++ b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOutputClauseConvention.cs @@ -0,0 +1,67 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.SqlServer.Internal; + +// ReSharper disable once CheckNamespace +namespace Microsoft.EntityFrameworkCore.Metadata.Conventions; + +/// +/// A convention that configures tables with triggers to not use the OUTPUT clause when saving changes. +/// +/// +/// See Model building conventions, and +/// Accessing SQL Server and SQL Azure databases with EF Core +/// for more information and examples. +/// +public class SqlServerOutputClauseConvention : ITriggerAddedConvention, ITriggerRemovedConvention +{ + /// + /// Creates a new instance of . + /// + /// Parameter object containing dependencies for this convention. + /// Parameter object containing relational dependencies for this convention. + public SqlServerOutputClauseConvention( + ProviderConventionSetBuilderDependencies dependencies, + RelationalConventionSetBuilderDependencies relationalDependencies) + { + Dependencies = dependencies; + RelationalDependencies = relationalDependencies; + } + + /// + /// Dependencies for this service. + /// + protected virtual ProviderConventionSetBuilderDependencies Dependencies { get; } + + /// + /// Relational provider-specific dependencies for this service. + /// + protected virtual RelationalConventionSetBuilderDependencies RelationalDependencies { get; } + + /// + public virtual void ProcessTriggerAdded(IConventionTriggerBuilder triggerBuilder, IConventionContext context) + { + var trigger = triggerBuilder.Metadata; + var entityType = trigger.EntityType; + var triggerTableIdentifier = StoreObjectIdentifier.Table(trigger.GetTableName(), trigger.GetTableSchema()); + + entityType.UseSqlOutputClause(false, triggerTableIdentifier); + } + + /// + public virtual void ProcessTriggerRemoved( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionTrigger trigger, + IConventionContext context) + { + var entityType = entityTypeBuilder.Metadata; + var triggerTableIdentifier = StoreObjectIdentifier.Table(trigger.GetTableName(), trigger.GetTableSchema()); + + if (!entityType.GetDeclaredTriggers().Any( + t => t.GetTableName() == trigger.GetTableName() && t.GetTableSchema() == trigger.GetTableSchema())) + { + entityType.UseSqlOutputClause(null, triggerTableIdentifier); + } + } +} diff --git a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationConvention.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationConvention.cs index 52728c8041a..cb6cfc9761d 100644 --- a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationConvention.cs +++ b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationConvention.cs @@ -71,8 +71,7 @@ public override void ProcessEntityTypeAnnotationChanged( IConventionAnnotation? oldAnnotation, IConventionContext 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); diff --git a/src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationNames.cs b/src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationNames.cs index 291c07d5bbc..8bf33226c28 100644 --- a/src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationNames.cs +++ b/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. /// public const string ValueGenerationStrategy = Prefix + "ValueGenerationStrategy"; + + /// + /// 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. + /// + public const string UseSqlOutputClause = Prefix + "UseSqlOutputClause"; } diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs index bac235b3049..ec120fdeef9 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -36,7 +36,7 @@ public static string AlterMemoryOptimizedTable => GetString("AlterMemoryOptimizedTable"); /// - /// Can't produce unterminated SQL with comments when generating migrations SQL for {operation}, . + /// Can't produce unterminated SQL with comments when generating migrations SQL for {operation}. /// public static string CannotProduceUnterminatedSQLWithComments(object? operation) => string.Format( @@ -155,6 +155,14 @@ public static string IncludePropertyNotFound(object? property, object? index, ob GetString("IncludePropertyNotFound", nameof(property), nameof(index), nameof(entityType)), property, index, entityType); + /// + /// Cannot use table '{table}' for entity type '{entityType}' since it is being used for entity type '{otherEntityType}' and entity type '{entityTypeWithSqlOutputClause}' is configured to use the SQL OUTPUT clause, but entity type '{entityTypeWithoutSqlOutputClause}' is not. + /// + public static string IncompatibleSqlOutputClauseMismatch(object? table, object? entityType, object? otherEntityType, object? entityTypeWithSqlOutputClause, object? entityTypeWithoutSqlOutputClause) + => string.Format( + GetString("IncompatibleSqlOutputClauseMismatch", nameof(table), nameof(entityType), nameof(otherEntityType), nameof(entityTypeWithSqlOutputClause), nameof(entityTypeWithoutSqlOutputClause)), + table, entityType, otherEntityType, entityTypeWithSqlOutputClause, entityTypeWithoutSqlOutputClause); + /// /// Cannot use table '{table}' for entity type '{entityType}' since it is being used for entity type '{otherEntityType}' and entity type '{memoryOptimizedEntityType}' is marked as memory-optimized, but entity type '{nonMemoryOptimizedEntityType}' is not. /// @@ -204,13 +212,13 @@ public static string NoSavepointRelease => GetString("NoSavepointRelease"); /// - /// 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. + /// 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. /// public static string SaveChangesFailedBecauseOfComputedColumnWithFunction => GetString("SaveChangesFailedBecauseOfComputedColumnWithFunction"); /// - /// 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. + /// 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. /// public static string SaveChangesFailedBecauseOfTriggers => GetString("SaveChangesFailedBecauseOfTriggers"); diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx index f31e4b331b9..be1db71e321 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -124,7 +124,7 @@ To change the memory-optimized setting on a table, the table needs to be dropped and recreated. - Can't produce unterminated SQL with comments when generating migrations SQL for {operation}, . + Can't produce unterminated SQL with comments when generating migrations SQL for {operation}. '{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}', but are configured with different identity increment values. @@ -168,6 +168,9 @@ The include property '{property}' specified on the index {index} was not found on entity type '{entityType}'. + + Cannot use table '{table}' for entity type '{entityType}' since it is being used for entity type '{otherEntityType}' and entity type '{entityTypeWithSqlOutputClause}' is configured to use the SQL OUTPUT clause, but entity type '{entityTypeWithoutSqlOutputClause}' is not. + Cannot use table '{table}' for entity type '{entityType}' since it is being used for entity type '{otherEntityType}' and entity type '{memoryOptimizedEntityType}' is marked as memory-optimized, but entity type '{nonMemoryOptimizedEntityType}' is not. @@ -282,10 +285,10 @@ SQL Server does not support releasing a savepoint. - 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. + 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. - 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. + 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. 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. diff --git a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs index a38012e1f94..8572056f688 100644 --- a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs +++ b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs @@ -47,9 +47,9 @@ public override ResultSetMapping AppendInsertOperation( out bool requiresTransaction) { // If no database-generated columns need to be read back, just do a simple INSERT (default behavior). - // If there are generated columns but there are no triggers defined on the table, we can do a simple INSERT ... OUTPUT - // (without INTO), which is also the default behavior, doesn't require a transaction and is the most efficient. - if (command.ColumnModifications.All(o => !o.IsRead) || !HasAnyTriggers(command)) + // If there are generated columns but we can use OUTPUT without INTO (i.e. no triggers), we can do a simple INSERT ... OUTPUT, + // which is also the default behavior, doesn't require a transaction and is the most efficient. + if (command.ColumnModifications.All(o => !o.IsRead) || CanUseOutputClause(command)) { return AppendInsertReturningOperation(commandStringBuilder, command, commandPosition, out requiresTransaction); } @@ -107,10 +107,10 @@ public override ResultSetMapping AppendUpdateOperation( int commandPosition, out bool requiresTransaction) // We normally do a simple UPDATE with an OUTPUT clause (either for the generated columns, or for "1" for concurrency checking). - // However, if there are triggers defined, OUTPUT (without INTO) is not supported, so we do UPDATE+SELECT. - => HasAnyTriggers(command) - ? AppendUpdateAndSelectOperation(commandStringBuilder, command, commandPosition, out requiresTransaction) - : AppendUpdateReturningOperation(commandStringBuilder, command, commandPosition, out requiresTransaction); + // However, if OUTPUT (without INTO) isn't supported (e.g. there are triggers), we do UPDATE+SELECT. + => CanUseOutputClause(command) + ? AppendUpdateReturningOperation(commandStringBuilder, command, commandPosition, out requiresTransaction) + : AppendUpdateAndSelectOperation(commandStringBuilder, command, commandPosition, out requiresTransaction); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -201,10 +201,10 @@ public override ResultSetMapping AppendDeleteOperation( int commandPosition, out bool requiresTransaction) // We normally do a simple DELETE, with an OUTPUT clause emitting "1" for concurrency checking. - // However, if there are triggers defined, OUTPUT (without INTO) is not supported, so we do UPDATE+SELECT. - => HasAnyTriggers(command) - ? AppendDeleteAndSelectOperation(commandStringBuilder, command, commandPosition, out requiresTransaction) - : AppendDeleteReturningOperation(commandStringBuilder, command, commandPosition, out requiresTransaction); + // However, if OUTPUT (without INTO) isn't supported (e.g. there are triggers), we do DELETE+SELECT. + => CanUseOutputClause(command) + ? AppendDeleteReturningOperation(commandStringBuilder, command, commandPosition, out requiresTransaction) + : AppendDeleteAndSelectOperation(commandStringBuilder, command, commandPosition, out requiresTransaction); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -329,9 +329,8 @@ public virtual ResultSetMapping AppendBulkInsertOperation( } // We default to using MERGE ... OUTPUT (without INTO), projecting back a synthetic _Position column to know the order back - // at the client and propagate database-generated values correctly. However, if any triggers are defined, OUTPUT without INTO - // doesn't work. - if (!HasAnyTriggers(firstCommand)) + // at the client and propagate database-generated values correctly. + if (CanUseOutputClause(firstCommand)) { // MERGE ... OUTPUT returns rows whose ordering isn't guaranteed. So this technique projects back a position int with each row, // to allow mapping the rows back for value propagation. @@ -339,7 +338,7 @@ public virtual ResultSetMapping AppendBulkInsertOperation( commandStringBuilder, modificationCommands, writeOperations, readOperations, out requiresTransaction); } - // We have a trigger, so can't use a simple OUTPUT clause. + // We can't use the OUTPUT (without INTO) clause (e.g. triggers are defined). // If we have an IDENTITY column, then multiple batched SELECT+INSERTs are faster up to a certain threshold (4), and then // MERGE ... OUTPUT INTO is faster. if (modificationCommands.Count < MergeIntoMinimumThreshold @@ -995,9 +994,6 @@ protected override void AppendRowsAffectedWhereCondition(StringBuilder commandSt .Append("@@ROWCOUNT = ") .Append(expectedRowsAffected.ToString(CultureInfo.InvariantCulture)); - private static bool HasAnyTriggers(IReadOnlyModificationCommand command) - // Data seeding doesn't provide any entries, so we we don't know if the table has triggers; assume it does to generate SQL - // that works everywhere. - => command.Entries.Count == 0 - || command.Table!.Triggers.Any(); + private static bool CanUseOutputClause(IReadOnlyModificationCommand command) + => command.Table?.IsSqlOutputClauseUsed() == true; } diff --git a/src/EFCore.Sqlite.Core/Extensions/SqliteEntityTypeExtensions.cs b/src/EFCore.Sqlite.Core/Extensions/SqliteEntityTypeExtensions.cs new file mode 100644 index 00000000000..02d732c030f --- /dev/null +++ b/src/EFCore.Sqlite.Core/Extensions/SqliteEntityTypeExtensions.cs @@ -0,0 +1,156 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.Sqlite.Metadata.Internal; + +// ReSharper disable once CheckNamespace +namespace Microsoft.EntityFrameworkCore; + +/// +/// Entity type extension methods for Sqlite-specific metadata. +/// +/// +/// See Modeling entity types and relationships, and +/// Accessing Sqlite databases with EF Core for more information and examples. +/// +public static class SqliteEntityTypeExtensions +{ + /// + /// Returns a value indicating whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// The entity type. + /// if the SQL RETURNING clause is used to save changes to the table. + public static bool IsSqlReturningClauseUsed(this IReadOnlyEntityType entityType) + { + if (entityType.FindAnnotation(SqliteAnnotationNames.UseSqlReturningClause) is { Value: bool useSqlOutputClause }) + { + return useSqlOutputClause; + } + + if (entityType.FindOwnership() is { } ownership + && StoreObjectIdentifier.Create(entityType, StoreObjectType.Table) is { } tableIdentifier + && ownership.FindSharedObjectRootForeignKey(tableIdentifier) is { } rootForeignKey) + { + return rootForeignKey.PrincipalEntityType.IsSqlReturningClauseUsed(); + } + + if (entityType.BaseType is not null && entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy) + { + return entityType.GetRootType().IsSqlReturningClauseUsed(); + } + + return true; + } + + /// + /// Sets a value indicating whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// The entity type. + /// The value to set. + public static void UseSqlReturningClause(this IMutableEntityType entityType, bool? useSqlReturningClause) + => entityType.SetOrRemoveAnnotation(SqliteAnnotationNames.UseSqlReturningClause, useSqlReturningClause); + + /// + /// Sets a value indicating whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// The entity type. + /// The value to set. + /// Indicates whether the configuration was specified using a data annotation. + /// The configured value. + public static bool? UseSqlReturningClause( + this IConventionEntityType entityType, + bool? useSqlReturningClause, + bool fromDataAnnotation = false) + => (bool?)entityType.SetOrRemoveAnnotation( + SqliteAnnotationNames.UseSqlReturningClause, + useSqlReturningClause, + fromDataAnnotation)?.Value; + + /// + /// Gets the configuration source for whether to use the SQL RETURNING clause when saving changes to the table. + /// + /// The entity type. + /// The configuration source for the memory-optimized setting. + public static ConfigurationSource? GetUseSqlReturningClauseConfigurationSource(this IConventionEntityType entityType) + => entityType.FindAnnotation(SqliteAnnotationNames.UseSqlReturningClause)?.GetConfigurationSource(); + + /// + /// Returns a value indicating whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// The entity type. + /// The identifier of the table-like store object. + /// if the SQL RETURNING clause is used to save changes to the table. + public static bool IsSqlReturningClauseUsed(this IReadOnlyEntityType entityType, in StoreObjectIdentifier storeObject) + { + if (entityType.FindMappingFragment(storeObject) is { } overrides + && overrides.FindAnnotation(SqliteAnnotationNames.UseSqlReturningClause) is { Value: bool useSqlOutputClause }) + { + return useSqlOutputClause; + } + + if (StoreObjectIdentifier.Create(entityType, storeObject.StoreObjectType) == storeObject) + { + return entityType.IsSqlReturningClauseUsed(); + } + + if (entityType.FindOwnership() is { } ownership + && ownership.FindSharedObjectRootForeignKey(storeObject) is { } rootForeignKey) + { + return rootForeignKey.PrincipalEntityType.IsSqlReturningClauseUsed(storeObject); + } + + if (entityType.BaseType is not null && entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy) + { + return entityType.GetRootType().IsSqlReturningClauseUsed(storeObject); + } + + return true; + } + + /// + /// Sets a value indicating whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// The entity type. + /// The value to set. + /// The identifier of the table-like store object. + public static void UseSqlReturningClause( + this IMutableEntityType entityType, + bool? useSqlReturningClause, + in StoreObjectIdentifier storeObject) + { + if (StoreObjectIdentifier.Create(entityType, storeObject.StoreObjectType) == storeObject) + { + entityType.UseSqlReturningClause(useSqlReturningClause); + return; + } + + entityType + .GetOrCreateMappingFragment(storeObject) + .UseSqlReturningClause(useSqlReturningClause); + } + + /// + /// Sets a value indicating whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// The entity type. + /// The value to set. + /// The identifier of the table-like store object. + /// Indicates whether the configuration was specified using a data annotation. + /// The configured value. + public static bool? UseSqlReturningClause( + this IConventionEntityType entityType, + bool? useSqlReturningClause, + in StoreObjectIdentifier storeObject, + bool fromDataAnnotation = false) + => StoreObjectIdentifier.Create(entityType, storeObject.StoreObjectType) == storeObject + ? entityType.UseSqlReturningClause(useSqlReturningClause, fromDataAnnotation) + : entityType + .GetOrCreateMappingFragment(storeObject, fromDataAnnotation) + .UseSqlReturningClause(useSqlReturningClause, fromDataAnnotation); +} diff --git a/src/EFCore.Sqlite.Core/Extensions/SqliteEntityTypeMappingFragmentExtensions.cs b/src/EFCore.Sqlite.Core/Extensions/SqliteEntityTypeMappingFragmentExtensions.cs new file mode 100644 index 00000000000..2d2ee626bd2 --- /dev/null +++ b/src/EFCore.Sqlite.Core/Extensions/SqliteEntityTypeMappingFragmentExtensions.cs @@ -0,0 +1,53 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.Sqlite.Metadata.Internal; + +// ReSharper disable once CheckNamespace +namespace Microsoft.EntityFrameworkCore; + +/// +/// SQLite specific extension methods for . +/// +public static class SqliteEntityTypeMappingFragmentExtensions +{ + /// + /// Returns a value indicating whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// The entity type mapping fragment. + /// The configured value. + public static bool IsSqlReturningClauseUsed(this IReadOnlyEntityTypeMappingFragment fragment) + => fragment.FindAnnotation(SqliteAnnotationNames.UseSqlReturningClause) is not { Value: false }; + + /// + /// Sets a value indicating whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// The entity type mapping fragment. + /// The value to set. + public static void UseSqlReturningClause(this IMutableEntityTypeMappingFragment fragment, bool? useSqlReturningClause) + => fragment.SetAnnotation(SqliteAnnotationNames.UseSqlReturningClause, useSqlReturningClause); + + /// + /// Sets a value indicating whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// The entity type mapping fragment. + /// The value to set. + /// Indicates whether the configuration was specified using a data annotation. + /// The configured value. + public static bool? UseSqlReturningClause( + this IConventionEntityTypeMappingFragment fragment, + bool? useSqlReturningClause, + bool fromDataAnnotation = false) + => (bool?)fragment.SetAnnotation(SqliteAnnotationNames.UseSqlReturningClause, useSqlReturningClause, fromDataAnnotation)?.Value; + + /// + /// Gets the configuration source for whether to use the SQL RETURNING clause when saving changes to the associated table. + /// + /// The entity type mapping fragment. + /// The configuration source for the configured value. + public static ConfigurationSource? GetUseSqlReturningClauseConfigurationSource(this IConventionEntityTypeMappingFragment fragment) + => fragment.FindAnnotation(SqliteAnnotationNames.UseSqlReturningClause)?.GetConfigurationSource(); +} diff --git a/src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs b/src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs index 3bc77f951c2..ac63f282567 100644 --- a/src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs +++ b/src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs @@ -112,17 +112,7 @@ public static IServiceCollection AddEntityFrameworkSqlite(this IServiceCollectio .TryAdd() .TryAdd() .TryAdd() - .TryAdd( - sp => - { - // Support for the RETURNING clause on INSERT/UPDATE/DELETE was added in Sqlite 3.35. - // Detect which version we're using, and fall back to the older INSERT/UPDATE+SELECT behavior on legacy versions. - var dependencies = sp.GetRequiredService(); - - return new Version(new SqliteConnection().ServerVersion) < new Version(3, 35) - ? new SqliteLegacyUpdateSqlGenerator(dependencies) - : new SqliteUpdateSqlGenerator(dependencies); - }) + .TryAdd() .TryAdd() .TryAdd() .TryAddProviderSpecificServices( diff --git a/src/EFCore.Sqlite.Core/Extensions/SqliteTableBuilderExtensions.cs b/src/EFCore.Sqlite.Core/Extensions/SqliteTableBuilderExtensions.cs new file mode 100644 index 00000000000..089ba9d110a --- /dev/null +++ b/src/EFCore.Sqlite.Core/Extensions/SqliteTableBuilderExtensions.cs @@ -0,0 +1,146 @@ +// 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; + +/// +/// Sqlite-specific extension methods for . +/// +public static class SqliteTableBuilderExtensions +{ + /// + /// Configures whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// + /// See Using the SQL RETURNING clause with Sqlite for more + /// information and examples. + /// + /// The builder for the table being configured. + /// A value indicating whether to use the RETURNING clause when saving changes to the table. + /// The same builder instance so that multiple calls can be chained. + public static TableBuilder UseSqlReturningClause( + this TableBuilder tableBuilder, + bool useSqlReturningClause = true) + { + UseSqlReturningClause(tableBuilder.Metadata, tableBuilder.Name, tableBuilder.Schema, useSqlReturningClause); + + return tableBuilder; + } + + /// + /// Configures whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// + /// See Using the SQL RETURNING clause with Sqlite for more + /// information and examples. + /// + /// The entity type being configured. + /// The builder for the table being configured. + /// A value indicating whether to use the RETURNING clause when saving changes to the table. + /// The same builder instance so that multiple calls can be chained. + public static TableBuilder UseSqlReturningClause( + this TableBuilder tableBuilder, + bool useSqlReturningClause = true) + where TEntity : class + => (TableBuilder)((TableBuilder)tableBuilder).UseSqlReturningClause(useSqlReturningClause); + + /// + /// Configures whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// + /// See Using the SQL RETURNING clause with Sqlite for more + /// information and examples. + /// + /// The builder for the table being configured. + /// A value indicating whether to use the RETURNING clause when saving changes to the table. + /// The same builder instance so that multiple calls can be chained. + public static SplitTableBuilder UseSqlReturningClause( + this SplitTableBuilder tableBuilder, + bool useSqlReturningClause = true) + { + UseSqlReturningClause(tableBuilder.Metadata, tableBuilder.Name, tableBuilder.Schema, useSqlReturningClause); + + return tableBuilder; + } + + /// + /// Configures whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// + /// See Using the SQL RETURNING clause with Sqlite for more + /// information and examples. + /// + /// The entity type being configured. + /// The builder for the table being configured. + /// A value indicating whether to use the RETURNING clause when saving changes to the table. + /// The same builder instance so that multiple calls can be chained. + public static SplitTableBuilder UseSqlReturningClause( + this SplitTableBuilder tableBuilder, + bool useSqlReturningClause = true) + where TEntity : class + => (SplitTableBuilder)((SplitTableBuilder)tableBuilder).UseSqlReturningClause(useSqlReturningClause); + + /// + /// Configures whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// + /// See Using the SQL RETURNING clause with Sqlite for more + /// information and examples. + /// + /// The builder for the table being configured. + /// A value indicating whether to use the RETURNING clause when saving changes to the table. + /// The same builder instance so that multiple calls can be chained. + public static OwnedNavigationTableBuilder UseSqlReturningClause( + this OwnedNavigationTableBuilder tableBuilder, + bool useSqlReturningClause = true) + { + UseSqlReturningClause(tableBuilder.Metadata, tableBuilder.Name, tableBuilder.Schema, useSqlReturningClause); + + return tableBuilder; + } + + /// + /// Configures whether to use the SQL RETURNING clause when saving changes to the table. The RETURNING clause is incompatible with + /// certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// + /// See Using the SQL RETURNING clause with Sqlite for more + /// information and examples. + /// + /// The entity type owning the relationship. + /// The dependent entity type of the relationship. + /// The builder for the table being configured. + /// A value indicating whether to use the RETURNING clause when saving changes to the table. + /// The same builder instance so that multiple calls can be chained. + public static OwnedNavigationTableBuilder UseSqlReturningClause( + this OwnedNavigationTableBuilder tableBuilder, + bool useSqlReturningClause = true) + where TOwnerEntity : class + where TDependentEntity : class + => (OwnedNavigationTableBuilder) + ((OwnedNavigationTableBuilder)tableBuilder).UseSqlReturningClause(useSqlReturningClause); + + private static void UseSqlReturningClause( + IMutableEntityType entityType, + string? tableName, + string? tableSchema, + bool useSqlReturningClause) + { + if (tableName is null) + { + entityType.UseSqlReturningClause(useSqlReturningClause); + } + else + { + entityType.UseSqlReturningClause( + useSqlReturningClause, + StoreObjectIdentifier.Table(tableName, tableSchema)); + } + } +} diff --git a/src/EFCore.Sqlite.Core/Extensions/SqliteTableExtensions.cs b/src/EFCore.Sqlite.Core/Extensions/SqliteTableExtensions.cs new file mode 100644 index 00000000000..a8f7a9948bb --- /dev/null +++ b/src/EFCore.Sqlite.Core/Extensions/SqliteTableExtensions.cs @@ -0,0 +1,34 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.Sqlite.Metadata.Internal; + +// ReSharper disable once CheckNamespace +namespace Microsoft.EntityFrameworkCore; + +/// +/// SQLite specific extension methods for . +/// +public static class SqliteTableExtensions +{ + /// + /// Returns a value indicating whether to use the SQL RETURNING clause when saving changes to the table. + /// The RETURNING clause is incompatible with certain Sqlite features, such as virtual tables or tables with AFTER triggers. + /// + /// The table. + /// if the SQL RETURNING clause is used to save changes to the table. + public static bool IsSqlReturningClauseUsed(this ITable table) + { + if (table.FindRuntimeAnnotation(SqliteAnnotationNames.UseSqlReturningClause) is { Value: bool isSqlOutputClauseUsed } ) + { + return isSqlOutputClauseUsed; + } + + isSqlOutputClauseUsed = table.EntityTypeMappings.All( + e => e.EntityType.IsSqlReturningClauseUsed(StoreObjectIdentifier.Table(table.Name, table.Schema))); + + table.SetRuntimeAnnotation(SqliteAnnotationNames.UseSqlReturningClause, isSqlOutputClauseUsed); + + return isSqlOutputClauseUsed; + } +} diff --git a/src/EFCore.Sqlite.Core/Infrastructure/Internal/SqliteModelValidator.cs b/src/EFCore.Sqlite.Core/Infrastructure/Internal/SqliteModelValidator.cs index 5152b25318c..873bd1b3436 100644 --- a/src/EFCore.Sqlite.Core/Infrastructure/Internal/SqliteModelValidator.cs +++ b/src/EFCore.Sqlite.Core/Infrastructure/Internal/SqliteModelValidator.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.EntityFrameworkCore.Sqlite.Internal; +using Microsoft.EntityFrameworkCore.Sqlite.Metadata.Internal; namespace Microsoft.EntityFrameworkCore.Sqlite.Infrastructure.Internal; @@ -150,4 +151,41 @@ protected override void ValidateValueGeneration( logger.CompositeKeyWithValueGeneration(key); } } + + /// + /// 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. + /// + protected override void ValidateSharedTableCompatibility( + IReadOnlyList mappedTypes, + in StoreObjectIdentifier storeObject, + IDiagnosticsLogger logger) + { + bool? firstSqlOutputSetting = null; + IEntityType? firstMappedType = null; + foreach (var mappedType in mappedTypes) + { + if (((IConventionEntityType)mappedType).GetUseSqlReturningClauseConfigurationSource() is null) + { + continue; + } + + if (firstSqlOutputSetting is null) + { + (firstSqlOutputSetting, firstMappedType) = (mappedType.IsSqlReturningClauseUsed(), mappedType); + } + else if (mappedType.IsSqlReturningClauseUsed() != firstSqlOutputSetting) + { + throw new InvalidOperationException( + SqliteStrings.IncompatibleSqlReturningClauseMismatch( + storeObject.DisplayName(), firstMappedType!.DisplayName(), mappedType.DisplayName(), + firstSqlOutputSetting.Value ? firstMappedType.DisplayName() : mappedType.DisplayName(), + !firstSqlOutputSetting.Value ? firstMappedType.DisplayName() : mappedType.DisplayName())); + } + } + + base.ValidateSharedTableCompatibility(mappedTypes, storeObject, logger); + } } diff --git a/src/EFCore.Sqlite.Core/Metadata/Internal/SqliteAnnotationNames.cs b/src/EFCore.Sqlite.Core/Metadata/Internal/SqliteAnnotationNames.cs index 49214ac3288..6e74bb68d4e 100644 --- a/src/EFCore.Sqlite.Core/Metadata/Internal/SqliteAnnotationNames.cs +++ b/src/EFCore.Sqlite.Core/Metadata/Internal/SqliteAnnotationNames.cs @@ -66,4 +66,12 @@ public static class SqliteAnnotationNames /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public const string Srid = Prefix + "Srid"; + + /// + /// 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. + /// + public const string UseSqlReturningClause = Prefix + "UseSqlReturningClause"; } diff --git a/src/EFCore.Sqlite.Core/Properties/SqliteStrings.Designer.cs b/src/EFCore.Sqlite.Core/Properties/SqliteStrings.Designer.cs index 82e837b30f6..347b66fae15 100644 --- a/src/EFCore.Sqlite.Core/Properties/SqliteStrings.Designer.cs +++ b/src/EFCore.Sqlite.Core/Properties/SqliteStrings.Designer.cs @@ -51,6 +51,14 @@ public static string DuplicateColumnNameSridMismatch(object? entityType1, object GetString("DuplicateColumnNameSridMismatch", nameof(entityType1), nameof(property1), nameof(entityType2), nameof(property2), nameof(columnName), nameof(table)), entityType1, property1, entityType2, property2, columnName, table); + /// + /// Cannot use table '{table}' for entity type '{entityType}' since it is being used for entity type '{otherEntityType}' and entity type '{entityTypeWithSqlReturningClause}' is configured to use the SQL RETURNING clause, but entity type '{entityTypeWithoutSqlReturningClause}' is not. + /// + public static string IncompatibleSqlReturningClauseMismatch(object? table, object? entityType, object? otherEntityType, object? entityTypeWithSqlReturningClause, object? entityTypeWithoutSqlReturningClause) + => string.Format( + GetString("IncompatibleSqlReturningClauseMismatch", nameof(table), nameof(entityType), nameof(otherEntityType), nameof(entityTypeWithSqlReturningClause), nameof(entityTypeWithoutSqlReturningClause)), + table, entityType, otherEntityType, entityTypeWithSqlReturningClause, entityTypeWithoutSqlReturningClause); + /// /// SQLite does not support this migration operation ('{operation}'). See http://go.microsoft.com/fwlink/?LinkId=723262 for more information and examples. /// diff --git a/src/EFCore.Sqlite.Core/Properties/SqliteStrings.resx b/src/EFCore.Sqlite.Core/Properties/SqliteStrings.resx index f18928093f5..681c42f2ce3 100644 --- a/src/EFCore.Sqlite.Core/Properties/SqliteStrings.resx +++ b/src/EFCore.Sqlite.Core/Properties/SqliteStrings.resx @@ -129,6 +129,9 @@ '{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}', but are configured with different SRIDs. + + Cannot use table '{table}' for entity type '{entityType}' since it is being used for entity type '{otherEntityType}' and entity type '{entityTypeWithSqlReturningClause}' is configured to use the SQL RETURNING clause, but entity type '{entityTypeWithoutSqlReturningClause}' is not. + SQLite does not support this migration operation ('{operation}'). See http://go.microsoft.com/fwlink/?LinkId=723262 for more information and examples. diff --git a/src/EFCore.Sqlite.Core/Update/Internal/SqliteLegacyUpdateSqlGenerator.cs b/src/EFCore.Sqlite.Core/Update/Internal/SqliteLegacyUpdateSqlGenerator.cs index 1d5d31db4e4..c2e2f8f7797 100644 --- a/src/EFCore.Sqlite.Core/Update/Internal/SqliteLegacyUpdateSqlGenerator.cs +++ b/src/EFCore.Sqlite.Core/Update/Internal/SqliteLegacyUpdateSqlGenerator.cs @@ -1,88 +1,15 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Text; -using Microsoft.EntityFrameworkCore.Sqlite.Internal; - namespace Microsoft.EntityFrameworkCore.Sqlite.Update.Internal; -/// -/// 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. -/// -public class SqliteLegacyUpdateSqlGenerator : UpdateAndSelectSqlGenerator + /// + /// 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. + /// +[Obsolete("Call UseSqlReturningClause(false) instead, see https://aka.ms/efcore-docs-sqlite-save-changes-and-returning-clause")] +public class SqliteLegacyUpdateSqlGenerator { - /// - /// 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. - /// - public SqliteLegacyUpdateSqlGenerator(UpdateSqlGeneratorDependencies dependencies) - : base(dependencies) - { - } - - /// - /// Appends a WHERE condition for the identity (i.e. key value) of the given column. - /// - /// The builder to which the SQL should be appended. - /// The column for which the condition is being generated. - protected override void AppendIdentityWhereCondition(StringBuilder commandStringBuilder, IColumnModification columnModification) - { - Check.NotNull(commandStringBuilder, nameof(commandStringBuilder)); - Check.NotNull(columnModification, nameof(columnModification)); - - SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, "rowid"); - commandStringBuilder.Append(" = ") - .Append("last_insert_rowid()"); - } - - /// - /// Appends a SQL command for selecting the number of rows affected. - /// - /// The builder to which the SQL should be appended. - /// The name of the table. - /// The table schema, or to use the default schema. - /// The ordinal of the command for which rows affected it being returned. - /// The for this command. - protected override ResultSetMapping AppendSelectAffectedCountCommand( - StringBuilder commandStringBuilder, - string name, - string? schema, - int commandPosition) - { - Check.NotNull(commandStringBuilder, nameof(commandStringBuilder)); - Check.NotEmpty(name, nameof(name)); - - commandStringBuilder - .Append("SELECT changes()") - .AppendLine(SqlGenerationHelper.StatementTerminator) - .AppendLine(); - - return ResultSetMapping.LastInResultSet | ResultSetMapping.ResultSetWithRowsAffectedOnly; - } - - /// - /// Appends a WHERE condition checking rows affected. - /// - /// The builder to which the SQL should be appended. - /// The expected number of rows affected. - protected override void AppendRowsAffectedWhereCondition(StringBuilder commandStringBuilder, int expectedRowsAffected) - { - Check.NotNull(commandStringBuilder, nameof(commandStringBuilder)); - - commandStringBuilder.Append("changes() = ").Append(expectedRowsAffected); - } - - /// - /// 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. - /// - public override string GenerateNextSequenceValueOperation(string name, string? schema) - => throw new NotSupportedException(SqliteStrings.SequencesNotSupported); } diff --git a/src/EFCore.Sqlite.Core/Update/Internal/SqliteUpdateSqlGenerator.cs b/src/EFCore.Sqlite.Core/Update/Internal/SqliteUpdateSqlGenerator.cs index 48300417097..fa7a13740d8 100644 --- a/src/EFCore.Sqlite.Core/Update/Internal/SqliteUpdateSqlGenerator.cs +++ b/src/EFCore.Sqlite.Core/Update/Internal/SqliteUpdateSqlGenerator.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Text; +using Microsoft.Data.Sqlite; using Microsoft.EntityFrameworkCore.Sqlite.Internal; namespace Microsoft.EntityFrameworkCore.Sqlite.Update.Internal; @@ -11,8 +13,10 @@ namespace Microsoft.EntityFrameworkCore.Sqlite.Update.Internal; /// 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. /// -public class SqliteUpdateSqlGenerator : UpdateSqlGenerator +public class SqliteUpdateSqlGenerator : UpdateAndSelectSqlGenerator { + private readonly bool _isReturningClauseSupported; + /// /// 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 @@ -22,6 +26,112 @@ public class SqliteUpdateSqlGenerator : UpdateSqlGenerator public SqliteUpdateSqlGenerator(UpdateSqlGeneratorDependencies dependencies) : base(dependencies) { + // Support for the RETURNING clause on INSERT/UPDATE/DELETE was added in Sqlite 3.35. + // Detect which version we're using, and fall back to the older INSERT/UPDATE+SELECT behavior on legacy versions. + _isReturningClauseSupported = new Version(new SqliteConnection().ServerVersion) >= new Version(3, 35); + } + + /// + /// 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. + /// + public override ResultSetMapping AppendInsertOperation( + StringBuilder commandStringBuilder, + IReadOnlyModificationCommand command, + int commandPosition, + out bool requiresTransaction) + // We normally do a simple INSERT, with a RETURNING clause for generated columns or with "1" for concurrency checking. + // However, older SQLite versions and virtual tables don't support RETURNING, so we do INSERT+SELECT. + => CanUseReturningClause(command) + ? AppendInsertReturningOperation(commandStringBuilder, command, commandPosition, out requiresTransaction) + : AppendInsertAndSelectOperation(commandStringBuilder, command, commandPosition, out requiresTransaction); + + /// + /// 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. + /// + public override ResultSetMapping AppendUpdateOperation( + StringBuilder commandStringBuilder, + IReadOnlyModificationCommand command, + int commandPosition, + out bool requiresTransaction) + // We normally do a simple UPDATE, with a RETURNING clause for generated columns or with "1" for concurrency checking. + // However, older SQLite versions and virtual tables don't support RETURNING, so we do UPDATE+SELECT. + => CanUseReturningClause(command) + ? AppendUpdateReturningOperation(commandStringBuilder, command, commandPosition, out requiresTransaction) + : AppendUpdateAndSelectOperation(commandStringBuilder, command, commandPosition, out requiresTransaction); + + /// + /// 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. + /// + public override ResultSetMapping AppendDeleteOperation( + StringBuilder commandStringBuilder, + IReadOnlyModificationCommand command, + int commandPosition, + out bool requiresTransaction) + // We normally do a simple DELETE, with a RETURNING clause with "1" for concurrency checking. + // However, older SQLite versions and virtual tables don't support RETURNING, so we do DELETE+SELECT. + => CanUseReturningClause(command) + ? AppendDeleteReturningOperation(commandStringBuilder, command, commandPosition, out requiresTransaction) + : AppendDeleteAndSelectOperation(commandStringBuilder, command, commandPosition, out requiresTransaction); + + /// + /// Appends a WHERE condition for the identity (i.e. key value) of the given column. + /// + /// The builder to which the SQL should be appended. + /// The column for which the condition is being generated. + protected override void AppendIdentityWhereCondition(StringBuilder commandStringBuilder, IColumnModification columnModification) + { + Check.NotNull(commandStringBuilder, nameof(commandStringBuilder)); + Check.NotNull(columnModification, nameof(columnModification)); + + SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, "rowid"); + commandStringBuilder.Append(" = ") + .Append("last_insert_rowid()"); + } + + /// + /// Appends a SQL command for selecting the number of rows affected. + /// + /// The builder to which the SQL should be appended. + /// The name of the table. + /// The table schema, or to use the default schema. + /// The ordinal of the command for which rows affected it being returned. + /// The for this command. + protected override ResultSetMapping AppendSelectAffectedCountCommand( + StringBuilder commandStringBuilder, + string name, + string? schema, + int commandPosition) + { + Check.NotNull(commandStringBuilder, nameof(commandStringBuilder)); + Check.NotEmpty(name, nameof(name)); + + commandStringBuilder + .Append("SELECT changes()") + .AppendLine(SqlGenerationHelper.StatementTerminator) + .AppendLine(); + + return ResultSetMapping.LastInResultSet | ResultSetMapping.ResultSetWithRowsAffectedOnly; + } + + /// + /// Appends a WHERE condition checking rows affected. + /// + /// The builder to which the SQL should be appended. + /// The expected number of rows affected. + protected override void AppendRowsAffectedWhereCondition(StringBuilder commandStringBuilder, int expectedRowsAffected) + { + Check.NotNull(commandStringBuilder, nameof(commandStringBuilder)); + + commandStringBuilder.Append("changes() = ").Append(expectedRowsAffected); } /// @@ -32,4 +142,7 @@ public SqliteUpdateSqlGenerator(UpdateSqlGeneratorDependencies dependencies) /// public override string GenerateNextSequenceValueOperation(string name, string? schema) => throw new NotSupportedException(SqliteStrings.SequencesNotSupported); + + private bool CanUseReturningClause(IReadOnlyModificationCommand command) + => _isReturningClauseSupported && command.Table?.IsSqlReturningClauseUsed() == true; } diff --git a/src/EFCore/Metadata/Conventions/ConventionSet.cs b/src/EFCore/Metadata/Conventions/ConventionSet.cs index 09105279488..3c4c0c5e96c 100644 --- a/src/EFCore/Metadata/Conventions/ConventionSet.cs +++ b/src/EFCore/Metadata/Conventions/ConventionSet.cs @@ -157,6 +157,16 @@ public class ConventionSet /// public virtual List SkipNavigationRemovedConventions { get; } = new(); + /// + /// Conventions to run when a trigger property is added. + /// + public virtual List TriggerAddedConventions { get; } = new(); + + /// + /// Conventions to run when a trigger property is removed. + /// + public virtual List TriggerRemovedConventions { get; } = new(); + /// /// Conventions to run when a key is added. /// @@ -677,6 +687,16 @@ public virtual void Add(IConvention convention) SkipNavigationRemovedConventions.Add(skipNavigationRemovedConvention); } + if (convention is ITriggerAddedConvention triggerAddedConvention) + { + TriggerAddedConventions.Add(triggerAddedConvention); + } + + if (convention is ITriggerRemovedConvention triggerRemovedConvention) + { + TriggerRemovedConventions.Add(triggerRemovedConvention); + } + if (convention is IKeyAddedConvention keyAddedConvention) { KeyAddedConventions.Add(keyAddedConvention); @@ -950,6 +970,16 @@ public virtual void Remove(Type conventionType) Remove(SkipNavigationRemovedConventions, conventionType); } + if (typeof(ITriggerAddedConvention).IsAssignableFrom(conventionType)) + { + Remove(TriggerAddedConventions, conventionType); + } + + if (typeof(ITriggerRemovedConvention).IsAssignableFrom(conventionType)) + { + Remove(TriggerRemovedConventions, conventionType); + } + if (typeof(IKeyAddedConvention).IsAssignableFrom(conventionType)) { Remove(KeyAddedConventions, conventionType); diff --git a/src/EFCore/Metadata/Conventions/ITriggerAddedConvention.cs b/src/EFCore/Metadata/Conventions/ITriggerAddedConvention.cs new file mode 100644 index 00000000000..727efdb02c8 --- /dev/null +++ b/src/EFCore/Metadata/Conventions/ITriggerAddedConvention.cs @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.Metadata.Conventions; + +/// +/// Represents an operation that should be performed when a trigger is added to the entity type. +/// +/// +/// See Model building conventions for more information and examples. +/// +public interface ITriggerAddedConvention : IConvention +{ + /// + /// Called after a trigger is added to the entity type. + /// + /// The builder for the trigger. + /// Additional information associated with convention execution. + void ProcessTriggerAdded( + IConventionTriggerBuilder triggerBuilder, + IConventionContext context); +} diff --git a/src/EFCore/Metadata/Conventions/ITriggerRemovedConvention.cs b/src/EFCore/Metadata/Conventions/ITriggerRemovedConvention.cs new file mode 100644 index 00000000000..d23acb61cf8 --- /dev/null +++ b/src/EFCore/Metadata/Conventions/ITriggerRemovedConvention.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.Metadata.Conventions; + +/// +/// Represents an operation that should be performed when a trigger is removed from the entity type. +/// +/// +/// See Model building conventions for more information and examples. +/// +public interface ITriggerRemovedConvention : IConvention +{ + /// + /// Called after a trigger is removed from the entity type. + /// + /// The builder for the entity type that contained the trigger. + /// The removed trigger. + /// Additional information associated with convention execution. + void ProcessTriggerRemoved( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionTrigger trigger, + IConventionContext context); +} diff --git a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ConventionScope.cs b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ConventionScope.cs index 6ea81d79fec..853b40898cb 100644 --- a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ConventionScope.cs +++ b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ConventionScope.cs @@ -203,5 +203,9 @@ public int GetLeafCount() public abstract IConventionProperty? OnPropertyRemoved( IConventionEntityTypeBuilder entityTypeBuilder, IConventionProperty property); + + public abstract IConventionTriggerBuilder? OnTriggerAdded(IConventionTriggerBuilder triggerBuilder); + + public abstract IConventionTrigger? OnTriggerRemoved(IConventionEntityTypeBuilder entityTypeBuilder, IConventionTrigger trigger); } } diff --git a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.DelayedConventionScope.cs b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.DelayedConventionScope.cs index 8e471dba230..cadd461ed35 100644 --- a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.DelayedConventionScope.cs +++ b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.DelayedConventionScope.cs @@ -269,6 +269,21 @@ public override IConventionSkipNavigation OnSkipNavigationRemoved( return navigation; } + public override IConventionTriggerBuilder OnTriggerAdded( + IConventionTriggerBuilder navigationBuilder) + { + Add(new OnTriggerAddedNode(navigationBuilder)); + return navigationBuilder; + } + + public override IConventionTrigger OnTriggerRemoved( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionTrigger navigation) + { + Add(new OnTriggerRemovedNode(entityTypeBuilder, navigation)); + return navigation; + } + public override IReadOnlyList OnForeignKeyPropertiesChanged( IConventionForeignKeyBuilder relationshipBuilder, IReadOnlyList oldDependentProperties, @@ -793,6 +808,36 @@ public override void Run(ConventionDispatcher dispatcher) => dispatcher._immediateConventionScope.OnSkipNavigationRemoved(EntityTypeBuilder, Navigation); } + private sealed class OnTriggerAddedNode : ConventionNode + { + public OnTriggerAddedNode(IConventionTriggerBuilder triggerBuilder) + { + TriggerBuilder = triggerBuilder; + } + + public IConventionTriggerBuilder TriggerBuilder { get; } + + public override void Run(ConventionDispatcher dispatcher) + => dispatcher._immediateConventionScope.OnTriggerAdded(TriggerBuilder); + } + + private sealed class OnTriggerRemovedNode : ConventionNode + { + public OnTriggerRemovedNode( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionTrigger trigger) + { + EntityTypeBuilder = entityTypeBuilder; + Trigger = trigger; + } + + public IConventionEntityTypeBuilder EntityTypeBuilder { get; } + public IConventionTrigger Trigger { get; } + + public override void Run(ConventionDispatcher dispatcher) + => dispatcher._immediateConventionScope.OnTriggerRemoved(EntityTypeBuilder, Trigger); + } + private sealed class OnKeyAddedNode : ConventionNode { public OnKeyAddedNode(IConventionKeyBuilder keyBuilder) diff --git a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs index 1f12cc57e63..827cea1dbc0 100644 --- a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs +++ b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs @@ -29,6 +29,8 @@ private sealed class ImmediateConventionScope : ConventionScope private readonly ConventionContext _propertyBuilderConventionContext; private readonly ConventionContext _propertyConventionContext; private readonly ConventionContext _modelBuilderConventionContext; + private readonly ConventionContext _triggerBuilderConventionContext; + private readonly ConventionContext _triggerConventionContext; private readonly ConventionContext _annotationConventionContext; private readonly ConventionContext> _propertyListConventionContext; private readonly ConventionContext _stringConventionContext; @@ -55,6 +57,8 @@ public ImmediateConventionScope(ConventionSet conventionSet, ConventionDispatche _propertyBuilderConventionContext = new ConventionContext(dispatcher); _propertyConventionContext = new ConventionContext(dispatcher); _modelBuilderConventionContext = new ConventionContext(dispatcher); + _triggerBuilderConventionContext = new ConventionContext(dispatcher); + _triggerConventionContext = new ConventionContext(dispatcher); _annotationConventionContext = new ConventionContext(dispatcher); _propertyListConventionContext = new ConventionContext>(dispatcher); _stringConventionContext = new ConventionContext(dispatcher); @@ -931,6 +935,56 @@ public IConventionModelBuilder OnModelInitialized(IConventionModelBuilder modelB return navigation; } + public override IConventionTriggerBuilder? OnTriggerAdded(IConventionTriggerBuilder triggerBuilder) + { + if (!triggerBuilder.Metadata.EntityType.IsInModel) + { + return null; + } + + using (_dispatcher.DelayConventions()) + { + _triggerBuilderConventionContext.ResetState(triggerBuilder); + foreach (var triggerConvention in _conventionSet.TriggerAddedConventions) + { + if (!triggerBuilder.Metadata.IsInModel) + { + return null; + } + + triggerConvention.ProcessTriggerAdded(triggerBuilder, _triggerBuilderConventionContext); + if (_triggerBuilderConventionContext.ShouldStopProcessing()) + { + return _triggerBuilderConventionContext.Result; + } +#if DEBUG + Check.DebugAssert(triggerBuilder.Metadata.IsInModel, + $"Convention {triggerConvention.GetType().Name} changed value without terminating"); +#endif + } + } + + return !triggerBuilder.Metadata.IsInModel ? null : triggerBuilder; + } + + public override IConventionTrigger? OnTriggerRemoved(IConventionEntityTypeBuilder entityTypeBuilder, IConventionTrigger trigger) + { + using (_dispatcher.DelayConventions()) + { + _triggerConventionContext.ResetState(trigger); + foreach (var triggerConvention in _conventionSet.TriggerRemovedConventions) + { + triggerConvention.ProcessTriggerRemoved(entityTypeBuilder, trigger, _triggerConventionContext); + if (_triggerConventionContext.ShouldStopProcessing()) + { + return _triggerConventionContext.Result; + } + } + } + + return trigger; + } + public override IConventionKeyBuilder? OnKeyAdded(IConventionKeyBuilder keyBuilder) { if (!keyBuilder.Metadata.DeclaringEntityType.IsInModel) diff --git a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.cs b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.cs index 69ef6bd3cd1..8eef98c96b8 100644 --- a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.cs +++ b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.cs @@ -400,6 +400,27 @@ public virtual IConventionModelBuilder OnModelFinalizing(IConventionModelBuilder oldAnnotation); } + /// + /// 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. + /// + public virtual IConventionTriggerBuilder? OnTriggerAdded( + IConventionTriggerBuilder triggerBuilder) + => _scope.OnTriggerAdded(triggerBuilder); + + /// + /// 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. + /// + public virtual IConventionTrigger? OnTriggerRemoved( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionTrigger trigger) + => _scope.OnTriggerRemoved(entityTypeBuilder, trigger); + /// /// 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 diff --git a/src/EFCore/Metadata/Internal/EntityType.cs b/src/EFCore/Metadata/Internal/EntityType.cs index 253d772c9f3..7fcc23afc26 100644 --- a/src/EFCore/Metadata/Internal/EntityType.cs +++ b/src/EFCore/Metadata/Internal/EntityType.cs @@ -3010,7 +3010,7 @@ public virtual IEnumerable GetDerivedServiceProperties() _triggers.Add(modelName, trigger); - return trigger; + return (Trigger?)Model.ConventionDispatcher.OnTriggerAdded(trigger.Builder)?.Metadata; } /// @@ -3058,7 +3058,7 @@ public virtual IEnumerable GetDeclaredTriggers() trigger.SetRemovedFromModel(); - return trigger; + return (Trigger?)Model.ConventionDispatcher.OnTriggerRemoved(Builder, trigger); } #endregion diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs index cca686448c9..a05d59d8942 100644 --- a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs +++ b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs @@ -849,6 +849,8 @@ public virtual void Entity_splitting_is_stored_in_snapshot_with_tables() t.HasAnnotation("foo", "bar"); }); + + b.HasAnnotation("SqlServer:UseSqlOutputClause", false); }); modelBuilder.Entity("Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order", b => @@ -1481,6 +1483,8 @@ public virtual void Trigger_is_stored_in_snapshot() .HasDatabaseName("SomeTrg") .HasAnnotation("foo", "bar"); }); + + b.HasAnnotation("SqlServer:UseSqlOutputClause", false); }); """), o => @@ -1526,6 +1530,8 @@ public virtual void Triggers_and_ExcludeFromMigrations_are_stored_in_snapshot() t.HasTrigger("SomeTrigger2"); }); + + b.HasAnnotation("SqlServer:UseSqlOutputClause", false); }); """), o => diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs index 861e1df2636..edd3486d8a5 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs @@ -1196,11 +1196,13 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) { modelBuilder.Entity(entity => { - entity.ToTable(tb => + entity + .ToTable(tb => { tb.HasTrigger("Trigger1"); tb.HasTrigger("Trigger2"); - }); + }) + .HasAnnotation("SqlServer:UseSqlOutputClause", false); }); OnModelCreatingPartial(modelBuilder); diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs index 603cb8d6e12..4f51092fe7b 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs @@ -923,9 +923,7 @@ protected override bool ShouldUseFullName(Type type) => base.ShouldUseFullName(type); protected override bool ShouldUseFullName(string shortTypeName) - => base.ShouldUseFullName(shortTypeName) - || shortTypeName == nameof(Index) - || shortTypeName == nameof(Internal); + => base.ShouldUseFullName(shortTypeName) || shortTypeName is nameof(Index) or nameof(Internal); } [ConditionalFact] @@ -4338,6 +4336,7 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) runtimeEntityType.AddAnnotation("Relational:TableName", "Data"); runtimeEntityType.AddAnnotation("Relational:ViewName", null); runtimeEntityType.AddAnnotation("Relational:ViewSchema", null); + runtimeEntityType.AddAnnotation("SqlServer:UseSqlOutputClause", false); Customize(runtimeEntityType); } diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index babe29cae97..8e9c7c14a26 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -3665,6 +3665,21 @@ public virtual void Store_generated_in_composite_key() Assert.Equal(ValueGenerated.OnAdd, keyProperties[1].ValueGenerated); } + [ConditionalFact] + public void Detects_trigger_on_TPH_non_root() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder.Entity(); + modelBuilder.Entity().ToTable(tb => tb.HasTrigger("SomeTrigger")); + + VerifyError( + RelationalStrings.CannotConfigureTriggerNonRootTphEntity( + modelBuilder.Model.FindEntityType(typeof(Cat))!.DisplayName(), + modelBuilder.Model.FindEntityType(typeof(Animal))!.DisplayName()), + modelBuilder); + } + private class TpcBase { public int Id { get; set; } diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentityTriggerSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentityWithoutOutputSqlServerTest.cs similarity index 95% rename from test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentityTriggerSqlServerTest.cs rename to test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentityWithoutOutputSqlServerTest.cs index a3d1fb5723f..38c7b104be9 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentityTriggerSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentityWithoutOutputSqlServerTest.cs @@ -5,11 +5,11 @@ namespace Microsoft.EntityFrameworkCore.Update; #nullable enable -public class StoreValueGenerationIdentityTriggerSqlServerTest : StoreValueGenerationTriggerSqlServerTestBase< - StoreValueGenerationIdentityTriggerSqlServerTest.StoreValueGenerationIdentityWithTriggerSqlServerFixture> +public class StoreValueGenerationIdentityWithoutOutputSqlServerTest : StoreValueGenerationWithoutOutputSqlServerTestBase< + StoreValueGenerationIdentityWithoutOutputSqlServerTest.StoreValueGenerationIdentityWithWithoutOutputSqlServerFixture> { - public StoreValueGenerationIdentityTriggerSqlServerTest( - StoreValueGenerationIdentityWithTriggerSqlServerFixture fixture, + public StoreValueGenerationIdentityWithoutOutputSqlServerTest( + StoreValueGenerationIdentityWithWithoutOutputSqlServerFixture fixture, ITestOutputHelper testOutputHelper) : base(fixture) { @@ -471,7 +471,7 @@ protected override async Task Test( } } - public class StoreValueGenerationIdentityWithTriggerSqlServerFixture : StoreValueGenerationTriggerSqlServerFixture + public class StoreValueGenerationIdentityWithWithoutOutputSqlServerFixture : StoreValueGenerationWithoutOutputSqlServerFixture { protected override string StoreName => "StoreValueGenerationIdentityWithTriggerTest"; diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceTriggerSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceWithoutOutputSqlServerTest.cs similarity index 96% rename from test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceTriggerSqlServerTest.cs rename to test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceWithoutOutputSqlServerTest.cs index e4eca41006f..5e7749f0b38 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceTriggerSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceWithoutOutputSqlServerTest.cs @@ -7,11 +7,11 @@ namespace Microsoft.EntityFrameworkCore.Update; #nullable enable -public class StoreValueGenerationSequenceTriggerSqlServerTest : StoreValueGenerationTriggerSqlServerTestBase< - StoreValueGenerationSequenceTriggerSqlServerTest.StoreValueGenerationSequenceWithTriggerSqlServerFixture> +public class StoreValueGenerationSequenceWithoutOutputSqlServerTest : StoreValueGenerationWithoutOutputSqlServerTestBase< + StoreValueGenerationSequenceWithoutOutputSqlServerTest.StoreValueGenerationSequenceWithWithoutOutputSqlServerFixture> { - public StoreValueGenerationSequenceTriggerSqlServerTest( - StoreValueGenerationSequenceWithTriggerSqlServerFixture fixture, + public StoreValueGenerationSequenceWithoutOutputSqlServerTest( + StoreValueGenerationSequenceWithWithoutOutputSqlServerFixture fixture, ITestOutputHelper testOutputHelper) : base(fixture) { @@ -481,7 +481,7 @@ protected override async Task Test( } } - public class StoreValueGenerationSequenceWithTriggerSqlServerFixture : StoreValueGenerationTriggerSqlServerFixture + public class StoreValueGenerationSequenceWithWithoutOutputSqlServerFixture : StoreValueGenerationWithoutOutputSqlServerFixture { protected override string StoreName => "StoreValueGenerationSequenceWithTriggerTest"; diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationTriggerSqlServerFixture.cs b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationWithoutOutputSqlServerFixture.cs similarity index 81% rename from test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationTriggerSqlServerFixture.cs rename to test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationWithoutOutputSqlServerFixture.cs index f54e95a9e9d..a4dabc636cb 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationTriggerSqlServerFixture.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationWithoutOutputSqlServerFixture.cs @@ -5,7 +5,7 @@ namespace Microsoft.EntityFrameworkCore.Update; -public abstract class StoreValueGenerationTriggerSqlServerFixture : StoreValueGenerationSqlServerFixtureBase +public abstract class StoreValueGenerationWithoutOutputSqlServerFixture : StoreValueGenerationSqlServerFixtureBase { protected override void Seed(StoreValueGenerationContext context) { @@ -32,7 +32,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con foreach (var entity in modelBuilder.Model.GetEntityTypes()) { - modelBuilder.Entity(entity.Name).ToTable(b => b.HasTrigger(entity.GetTableName() + "_Trigger")); + modelBuilder.Entity(entity.Name).ToTable(b => b.UseSqlOutputClause(false)); } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationTriggerSqlServerTestBase.cs b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationWithoutOutputSqlServerTestBase.cs similarity index 92% rename from test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationTriggerSqlServerTestBase.cs rename to test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationWithoutOutputSqlServerTestBase.cs index 0286705b6b9..190ccb7b34e 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationTriggerSqlServerTestBase.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationWithoutOutputSqlServerTestBase.cs @@ -5,10 +5,10 @@ namespace Microsoft.EntityFrameworkCore.Update; -public abstract class StoreValueGenerationTriggerSqlServerTestBase : StoreValueGenerationTestBase - where TFixture : StoreValueGenerationTriggerSqlServerFixture +public abstract class StoreValueGenerationWithoutOutputSqlServerTestBase : StoreValueGenerationTestBase + where TFixture : StoreValueGenerationWithoutOutputSqlServerFixture { - protected StoreValueGenerationTriggerSqlServerTestBase(TFixture fixture) + protected StoreValueGenerationWithoutOutputSqlServerTestBase(TFixture fixture) : base(fixture) { } diff --git a/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs b/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs index 758e2f72885..0e2c71e8591 100644 --- a/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs +++ b/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs @@ -465,6 +465,32 @@ public virtual void Detects_incompatible_memory_optimized_shared_table() modelBuilder); } + [ConditionalFact] + public virtual void Detects_incompatible_sql_output_clause_shared_table() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(a => a.Id).HasPrincipalKey(b => b.Id).IsRequired(); + + modelBuilder.Entity().ToTable("Table", tb => tb.UseSqlOutputClause(false)); + modelBuilder.Entity().ToTable("Table", tb => tb.UseSqlOutputClause()); + + VerifyError( + SqlServerStrings.IncompatibleSqlOutputClauseMismatch("Table", nameof(A), nameof(B), nameof(B), nameof(A)), + modelBuilder); + } + + [ConditionalFact] + public virtual void Passes_for_shared_table_with_only_one_entity_trigger_definition() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder.Entity().ToTable("Table", tb => tb.HasTrigger("SomeTrigger")); + modelBuilder.Entity().OwnsOne(o => o.OrderDetails).ToTable("Table"); + + Validate(modelBuilder); + } + [ConditionalFact] public virtual void Detects_incompatible_non_clustered_shared_key() { diff --git a/test/EFCore.SqlServer.Tests/Metadata/Conventions/SqlServerOutputClauseConventionTest.cs b/test/EFCore.SqlServer.Tests/Metadata/Conventions/SqlServerOutputClauseConventionTest.cs new file mode 100644 index 00000000000..c479e0993dc --- /dev/null +++ b/test/EFCore.SqlServer.Tests/Metadata/Conventions/SqlServerOutputClauseConventionTest.cs @@ -0,0 +1,58 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.SqlServer.Internal; + +namespace Microsoft.EntityFrameworkCore.Metadata.Conventions; + +#nullable enable + +public class SqlServerOutputClauseConventionTest +{ + [ConditionalFact] + public void Output_clause_is_enabled_by_default() + { + var modelBuilder = SqlServerTestHelpers.Instance.CreateConventionBuilder(); + modelBuilder.Entity(); + var model = modelBuilder.Model; + + var entityType = model.FindEntityType(typeof(Order))!; + var tableIdentifier = StoreObjectIdentifier.Create(entityType, StoreObjectType.Table)!.Value; + + Assert.True(entityType.IsSqlOutputClauseUsed(tableIdentifier)); + } + + [ConditionalFact] + public void Trigger_disables_output_clause() + { + var modelBuilder = SqlServerTestHelpers.Instance.CreateConventionBuilder(); + var model = modelBuilder.Model; + + var entityTypeBuilder = modelBuilder.Entity(); + var entityType = model.FindEntityType(typeof(Order))!; + var tableIdentifier = StoreObjectIdentifier.Create(entityType, StoreObjectType.Table)!.Value; + + Assert.True(entityType.IsSqlOutputClauseUsed(tableIdentifier)); + + entityTypeBuilder.ToTable(t => t.HasTrigger("Trigger1")); + Assert.False(entityType.IsSqlOutputClauseUsed(tableIdentifier)); + entityTypeBuilder.ToTable(t => t.HasTrigger("Trigger2")); + Assert.False(entityType.IsSqlOutputClauseUsed(tableIdentifier)); + + entityTypeBuilder.Metadata.RemoveTrigger("Trigger1"); + Assert.False(entityType.IsSqlOutputClauseUsed(tableIdentifier)); + entityTypeBuilder.Metadata.RemoveTrigger("Trigger2"); + Assert.True(entityType.IsSqlOutputClauseUsed(tableIdentifier)); + } + + private class Order + { + public int Id { get; set; } + public int CustomerId { get; set; } + } + + private class SpecialOrder : Order + { + public int SpecialProperty { get; set; } + } +} diff --git a/test/EFCore.SqlServer.Tests/Metadata/SqlServerBuilderExtensionsTest.cs b/test/EFCore.SqlServer.Tests/Metadata/SqlServerBuilderExtensionsTest.cs index b0a9f3e63a2..1c338a09f9f 100644 --- a/test/EFCore.SqlServer.Tests/Metadata/SqlServerBuilderExtensionsTest.cs +++ b/test/EFCore.SqlServer.Tests/Metadata/SqlServerBuilderExtensionsTest.cs @@ -3,6 +3,8 @@ // ReSharper disable InconsistentNaming +using Microsoft.EntityFrameworkCore.SqlServer.Internal; + namespace Microsoft.EntityFrameworkCore.Metadata; public class SqlServerBuilderExtensionsTest @@ -1078,6 +1080,120 @@ public void Throws_if_attempt_to_set_fillfactor_with_argument_out_of_range(int f }); } + #region UseSqlOutputClause + + [ConditionalFact] + public void Can_set_UseSqlOutputClause() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder.Entity(); + var entityType = modelBuilder.Model.FindEntityType(typeof(Customer))!; + + Assert.True(entityType.IsSqlOutputClauseUsed()); + + modelBuilder + .Entity() + .ToTable(tb => tb.UseSqlOutputClause(false)); + + Assert.False(entityType.IsSqlOutputClauseUsed()); + + modelBuilder + .Entity() + .ToTable(tb => tb.UseSqlOutputClause()); + + Assert.True(entityType.IsSqlOutputClauseUsed()); + } + + [ConditionalFact] + public void Can_set_UseSqlOutputClause_with_table_name_and_one_table() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder + .Entity() + .ToTable("foo"); + var entityType = modelBuilder.Model.FindEntityType(typeof(Customer))!; + var tableIdentifier = StoreObjectIdentifier.Table("foo"); + + Assert.True(entityType.IsSqlOutputClauseUsed(tableIdentifier)); + Assert.True(entityType.IsSqlOutputClauseUsed()); + + modelBuilder + .Entity() + .ToTable("foo", tb => tb.UseSqlOutputClause(false)); + + Assert.False(entityType.IsSqlOutputClauseUsed(tableIdentifier)); + Assert.False(entityType.IsSqlOutputClauseUsed()); + + modelBuilder + .Entity() + .ToTable("foo", tb => tb.UseSqlOutputClause()); + + Assert.True(entityType.IsSqlOutputClauseUsed(tableIdentifier)); + Assert.True(entityType.IsSqlOutputClauseUsed()); + } + + [ConditionalFact] + public void Can_set_UseSqlOutputClause_with_table_name_and_two_tables() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder + .Entity() + .ToTable("foo") + .SplitToTable("bar", tb => tb.Property(c => c.Offset)); + + var entityType = modelBuilder.Model.FindEntityType(typeof(Customer))!; + var fooTableIdentifier = StoreObjectIdentifier.Table("foo"); + var barTableIdentifier = StoreObjectIdentifier.Table("bar"); + + Assert.True(entityType.IsSqlOutputClauseUsed(fooTableIdentifier)); + Assert.True(entityType.IsSqlOutputClauseUsed(barTableIdentifier)); + Assert.True(entityType.IsSqlOutputClauseUsed()); + + modelBuilder + .Entity() + .SplitToTable("bar", tb => tb.UseSqlOutputClause(false)); + + Assert.False(entityType.IsSqlOutputClauseUsed(barTableIdentifier)); + Assert.True(entityType.IsSqlOutputClauseUsed(fooTableIdentifier)); + Assert.True(entityType.IsSqlOutputClauseUsed()); + + modelBuilder + .Entity() + .SplitToTable("bar", tb => tb.UseSqlOutputClause()); + + Assert.True(entityType.IsSqlOutputClauseUsed(barTableIdentifier)); + Assert.True(entityType.IsSqlOutputClauseUsed(fooTableIdentifier)); + Assert.True(entityType.IsSqlOutputClauseUsed()); + } + + [ConditionalFact] + public void Can_set_UseSqlOutputClause_non_generic() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder.Entity(typeof(Customer)); + var entityType = modelBuilder.Model.FindEntityType(typeof(Customer))!; + + Assert.True(entityType.IsSqlOutputClauseUsed()); + + modelBuilder + .Entity(typeof(Customer)) + .ToTable(tb => tb.UseSqlOutputClause(false)); + + Assert.False(entityType.IsSqlOutputClauseUsed()); + + modelBuilder + .Entity(typeof(Customer)) + .ToTable(tb => tb.UseSqlOutputClause()); + + Assert.True(entityType.IsSqlOutputClauseUsed()); + } + + #endregion UseSqlOutputClause + private void AssertIsGeneric(EntityTypeBuilder _) { } @@ -1119,4 +1235,9 @@ private class OrderDetails public int OrderId { get; set; } public Order Order { get; set; } } + + private class SpecialCustomer : Customer + { + public int SpecialProperty { get; set; } + } } diff --git a/test/EFCore.Sqlite.FunctionalTests/Migrations/SqliteMigrationsSqlGeneratorTest.cs b/test/EFCore.Sqlite.FunctionalTests/Migrations/SqliteMigrationsSqlGeneratorTest.cs index 0ea3751096b..65d876f43ef 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Migrations/SqliteMigrationsSqlGeneratorTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Migrations/SqliteMigrationsSqlGeneratorTest.cs @@ -396,20 +396,35 @@ public override void InsertDataOperation_all_args_spatial() """ INSERT INTO "People" ("Id", "Full Name", "Geometry") VALUES (0, NULL, NULL); +SELECT changes(); + INSERT INTO "People" ("Id", "Full Name", "Geometry") VALUES (1, 'Daenerys Targaryen', NULL); +SELECT changes(); + INSERT INTO "People" ("Id", "Full Name", "Geometry") VALUES (2, 'John Snow', NULL); +SELECT changes(); + INSERT INTO "People" ("Id", "Full Name", "Geometry") VALUES (3, 'Arya Stark', NULL); +SELECT changes(); + INSERT INTO "People" ("Id", "Full Name", "Geometry") VALUES (4, 'Harry Strickland', NULL); +SELECT changes(); + INSERT INTO "People" ("Id", "Full Name", "Geometry") VALUES (5, 'The Imp', NULL); +SELECT changes(); + INSERT INTO "People" ("Id", "Full Name", "Geometry") VALUES (6, 'The Kingslayer', NULL); +SELECT changes(); + INSERT INTO "People" ("Id", "Full Name", "Geometry") VALUES (7, 'Aemon Targaryen', GeomFromText('GEOMETRYCOLLECTION Z(LINESTRING Z(1.1 2.2 NaN, 2.2 2.2 NaN, 2.2 1.1 NaN, 7.1 7.2 NaN), LINESTRING Z(7.1 7.2 NaN, 20.2 20.2 NaN, 20.2 1.1 NaN, 70.1 70.2 NaN), MULTIPOINT Z((1.1 2.2 NaN), (2.2 2.2 NaN), (2.2 1.1 NaN)), POLYGON Z((1.1 2.2 NaN, 2.2 2.2 NaN, 2.2 1.1 NaN, 1.1 2.2 NaN)), POLYGON Z((10.1 20.2 NaN, 20.2 20.2 NaN, 20.2 10.1 NaN, 10.1 20.2 NaN)), POINT Z(1.1 2.2 3.3), MULTILINESTRING Z((1.1 2.2 NaN, 2.2 2.2 NaN, 2.2 1.1 NaN, 7.1 7.2 NaN), (7.1 7.2 NaN, 20.2 20.2 NaN, 20.2 1.1 NaN, 70.1 70.2 NaN)), MULTIPOLYGON Z(((10.1 20.2 NaN, 20.2 20.2 NaN, 20.2 10.1 NaN, 10.1 20.2 NaN)), ((1.1 2.2 NaN, 2.2 2.2 NaN, 2.2 1.1 NaN, 1.1 2.2 NaN))))', 4326)); +SELECT changes(); """); } @@ -424,6 +439,7 @@ public override void InsertDataOperation_required_args() """ INSERT INTO "People" ("First Name") VALUES ('John'); +SELECT changes(); """); } @@ -435,6 +451,7 @@ public override void InsertDataOperation_required_args_composite() """ INSERT INTO "People" ("First Name", "Last Name") VALUES ('John', 'Snow'); +SELECT changes(); """); } @@ -446,8 +463,11 @@ public override void InsertDataOperation_required_args_multiple_rows() """ INSERT INTO "People" ("First Name") VALUES ('John'); +SELECT changes(); + INSERT INTO "People" ("First Name") VALUES ('Daenerys'); +SELECT changes(); """); } @@ -463,20 +483,24 @@ public override void DeleteDataOperation_all_args() AssertSql( """ DELETE FROM "People" -WHERE "First Name" = 'Hodor' -RETURNING 1; +WHERE "First Name" = 'Hodor'; +SELECT changes(); + DELETE FROM "People" -WHERE "First Name" = 'Daenerys' -RETURNING 1; +WHERE "First Name" = 'Daenerys'; +SELECT changes(); + DELETE FROM "People" -WHERE "First Name" = 'John' -RETURNING 1; +WHERE "First Name" = 'John'; +SELECT changes(); + DELETE FROM "People" -WHERE "First Name" = 'Arya' -RETURNING 1; +WHERE "First Name" = 'Arya'; +SELECT changes(); + DELETE FROM "People" -WHERE "First Name" = 'Harry' -RETURNING 1; +WHERE "First Name" = 'Harry'; +SELECT changes(); """); } @@ -487,20 +511,24 @@ public override void DeleteDataOperation_all_args_composite() AssertSql( """ DELETE FROM "People" -WHERE "First Name" = 'Hodor' AND "Last Name" IS NULL -RETURNING 1; +WHERE "First Name" = 'Hodor' AND "Last Name" IS NULL; +SELECT changes(); + DELETE FROM "People" -WHERE "First Name" = 'Daenerys' AND "Last Name" = 'Targaryen' -RETURNING 1; +WHERE "First Name" = 'Daenerys' AND "Last Name" = 'Targaryen'; +SELECT changes(); + DELETE FROM "People" -WHERE "First Name" = 'John' AND "Last Name" = 'Snow' -RETURNING 1; +WHERE "First Name" = 'John' AND "Last Name" = 'Snow'; +SELECT changes(); + DELETE FROM "People" -WHERE "First Name" = 'Arya' AND "Last Name" = 'Stark' -RETURNING 1; +WHERE "First Name" = 'Arya' AND "Last Name" = 'Stark'; +SELECT changes(); + DELETE FROM "People" -WHERE "First Name" = 'Harry' AND "Last Name" = 'Strickland' -RETURNING 1; +WHERE "First Name" = 'Harry' AND "Last Name" = 'Strickland'; +SELECT changes(); """); } @@ -511,8 +539,8 @@ public override void DeleteDataOperation_required_args() AssertSql( """ DELETE FROM "People" -WHERE "Last Name" = 'Snow' -RETURNING 1; +WHERE "Last Name" = 'Snow'; +SELECT changes(); """); } @@ -523,8 +551,8 @@ public override void DeleteDataOperation_required_args_composite() AssertSql( """ DELETE FROM "People" -WHERE "First Name" = 'John' AND "Last Name" = 'Snow' -RETURNING 1; +WHERE "First Name" = 'John' AND "Last Name" = 'Snow'; +SELECT changes(); """); } @@ -535,11 +563,12 @@ public override void UpdateDataOperation_all_args() AssertSql( """ UPDATE "People" SET "Birthplace" = 'Winterfell', "House Allegiance" = 'Stark', "Culture" = 'Northmen' -WHERE "First Name" = 'Hodor' -RETURNING 1; +WHERE "First Name" = 'Hodor'; +SELECT changes(); + UPDATE "People" SET "Birthplace" = 'Dragonstone', "House Allegiance" = 'Targaryen', "Culture" = 'Valyrian' -WHERE "First Name" = 'Daenerys' -RETURNING 1; +WHERE "First Name" = 'Daenerys'; +SELECT changes(); """); } @@ -550,11 +579,12 @@ public override void UpdateDataOperation_all_args_composite() AssertSql( """ UPDATE "People" SET "House Allegiance" = 'Stark' -WHERE "First Name" = 'Hodor' AND "Last Name" IS NULL -RETURNING 1; +WHERE "First Name" = 'Hodor' AND "Last Name" IS NULL; +SELECT changes(); + UPDATE "People" SET "House Allegiance" = 'Targaryen' -WHERE "First Name" = 'Daenerys' AND "Last Name" = 'Targaryen' -RETURNING 1; +WHERE "First Name" = 'Daenerys' AND "Last Name" = 'Targaryen'; +SELECT changes(); """); } @@ -565,11 +595,12 @@ public override void UpdateDataOperation_all_args_composite_multi() AssertSql( """ UPDATE "People" SET "Birthplace" = 'Winterfell', "House Allegiance" = 'Stark', "Culture" = 'Northmen' -WHERE "First Name" = 'Hodor' AND "Last Name" IS NULL -RETURNING 1; +WHERE "First Name" = 'Hodor' AND "Last Name" IS NULL; +SELECT changes(); + UPDATE "People" SET "Birthplace" = 'Dragonstone', "House Allegiance" = 'Targaryen', "Culture" = 'Valyrian' -WHERE "First Name" = 'Daenerys' AND "Last Name" = 'Targaryen' -RETURNING 1; +WHERE "First Name" = 'Daenerys' AND "Last Name" = 'Targaryen'; +SELECT changes(); """); } @@ -580,8 +611,8 @@ public override void UpdateDataOperation_all_args_multi() AssertSql( """ UPDATE "People" SET "Birthplace" = 'Dragonstone', "House Allegiance" = 'Targaryen', "Culture" = 'Valyrian' -WHERE "First Name" = 'Daenerys' -RETURNING 1; +WHERE "First Name" = 'Daenerys'; +SELECT changes(); """); } @@ -592,8 +623,8 @@ public override void UpdateDataOperation_required_args() AssertSql( """ UPDATE "People" SET "House Allegiance" = 'Targaryen' -WHERE "First Name" = 'Daenerys' -RETURNING 1; +WHERE "First Name" = 'Daenerys'; +SELECT changes(); """); } @@ -604,8 +635,8 @@ public override void UpdateDataOperation_required_args_composite() AssertSql( """ UPDATE "People" SET "House Allegiance" = 'Targaryen' -WHERE "First Name" = 'Daenerys' AND "Last Name" = 'Targaryen' -RETURNING 1; +WHERE "First Name" = 'Daenerys' AND "Last Name" = 'Targaryen'; +SELECT changes(); """); } @@ -616,8 +647,8 @@ public override void UpdateDataOperation_required_args_composite_multi() AssertSql( """ UPDATE "People" SET "Birthplace" = 'Dragonstone', "House Allegiance" = 'Targaryen', "Culture" = 'Valyrian' -WHERE "First Name" = 'Daenerys' AND "Last Name" = 'Targaryen' -RETURNING 1; +WHERE "First Name" = 'Daenerys' AND "Last Name" = 'Targaryen'; +SELECT changes(); """); } @@ -628,8 +659,8 @@ public override void UpdateDataOperation_required_args_multi() AssertSql( """ UPDATE "People" SET "Birthplace" = 'Dragonstone', "House Allegiance" = 'Targaryen', "Culture" = 'Valyrian' -WHERE "First Name" = 'Daenerys' -RETURNING 1; +WHERE "First Name" = 'Daenerys'; +SELECT changes(); """); } @@ -640,11 +671,12 @@ public override void UpdateDataOperation_required_args_multiple_rows() AssertSql( """ UPDATE "People" SET "House Allegiance" = 'Stark' -WHERE "First Name" = 'Hodor' -RETURNING 1; +WHERE "First Name" = 'Hodor'; +SELECT changes(); + UPDATE "People" SET "House Allegiance" = 'Targaryen' -WHERE "First Name" = 'Daenerys' -RETURNING 1; +WHERE "First Name" = 'Daenerys'; +SELECT changes(); """); } diff --git a/test/EFCore.Sqlite.FunctionalTests/Update/StoreValueGenerationLegacySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Update/StoreValueGenerationLegacySqliteTest.cs index 7cdb2995b11..77c577444c3 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Update/StoreValueGenerationLegacySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Update/StoreValueGenerationLegacySqliteTest.cs @@ -5,12 +5,11 @@ namespace Microsoft.EntityFrameworkCore.Update; #nullable enable -// Old Sqlite versions don't support the RETURNING clause, so we use the INSERT/UPDATE+SELECT behavior for fetching back database- -// generated values and rows affected. -[SqliteVersionCondition(Max = "3.34.999")] -public class StoreValueGenerationLegacySqliteTest : StoreValueGenerationTestBase +public class StoreValueGenerationWithoutReturningSqliteTest : StoreValueGenerationTestBase + { - public StoreValueGenerationLegacySqliteTest(StoreValueGenerationSqliteFixture fixture, ITestOutputHelper testOutputHelper) + public StoreValueGenerationWithoutReturningSqliteTest( + StoreValueGenerationWithoutReturningSqliteFixture fixture, ITestOutputHelper testOutputHelper) : base(fixture) { fixture.TestSqlLoggerFactory.Clear(); @@ -438,4 +437,17 @@ DELETE FROM "WithSomeDatabaseGenerated2" } #endregion Same two operations with different entity types + + public class StoreValueGenerationWithoutReturningSqliteFixture : StoreValueGenerationSqliteFixture + { + protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context) + { + base.OnModelCreating(modelBuilder, context); + + foreach (var entity in modelBuilder.Model.GetEntityTypes()) + { + modelBuilder.Entity(entity.Name).ToTable(b => b.UseSqlReturningClause(false)); + } + } + } } diff --git a/test/EFCore.Sqlite.Tests/Infrastructure/SqliteModelValidatorTest.cs b/test/EFCore.Sqlite.Tests/Infrastructure/SqliteModelValidatorTest.cs index 60e8853a4fe..aaf37a8b97d 100644 --- a/test/EFCore.Sqlite.Tests/Infrastructure/SqliteModelValidatorTest.cs +++ b/test/EFCore.Sqlite.Tests/Infrastructure/SqliteModelValidatorTest.cs @@ -85,6 +85,21 @@ public void Detects_delete_stored_procedures() VerifyError(SqliteStrings.StoredProceduresNotSupported(nameof(Person)), modelBuilder); } + [ConditionalFact] + public virtual void Detects_incompatible_sql_returning_clause_shared_table() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(a => a.Id).HasPrincipalKey(b => b.Id).IsRequired(); + + modelBuilder.Entity().ToTable("Table", tb => tb.UseSqlReturningClause(false)); + modelBuilder.Entity().ToTable("Table", tb => tb.UseSqlReturningClause()); + + VerifyError( + SqliteStrings.IncompatibleSqlReturningClauseMismatch("Table", nameof(A), nameof(B), nameof(B), nameof(A)), + modelBuilder); + } + public override void Passes_for_stored_procedure_without_parameter_for_insert_non_save_property() { var exception = diff --git a/test/EFCore.Sqlite.Tests/Metadata/Builders/SqliteBuilderExtensionsTest.cs b/test/EFCore.Sqlite.Tests/Metadata/Builders/SqliteBuilderExtensionsTest.cs index 3e453aa7e56..d0d39e8b2f7 100644 --- a/test/EFCore.Sqlite.Tests/Metadata/Builders/SqliteBuilderExtensionsTest.cs +++ b/test/EFCore.Sqlite.Tests/Metadata/Builders/SqliteBuilderExtensionsTest.cs @@ -59,6 +59,120 @@ public void Can_set_srid_convention() Assert.Equal(1, property.GetSrid()); } + #region UseSqlReturningClause + + [ConditionalFact] + public void Can_set_UseSqlReturningClause() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder.Entity(); + var entityType = modelBuilder.Model.FindEntityType(typeof(Customer))!; + + Assert.True(entityType.IsSqlReturningClauseUsed()); + + modelBuilder + .Entity() + .ToTable(tb => tb.UseSqlReturningClause(false)); + + Assert.False(entityType.IsSqlReturningClauseUsed()); + + modelBuilder + .Entity() + .ToTable(tb => tb.UseSqlReturningClause()); + + Assert.True(entityType.IsSqlReturningClauseUsed()); + } + + [ConditionalFact] + public void Can_set_UseSqlReturningClause_with_table_name_and_one_table() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder + .Entity() + .ToTable("foo"); + var entityType = modelBuilder.Model.FindEntityType(typeof(Customer))!; + var tableIdentifier = StoreObjectIdentifier.Table("foo"); + + Assert.True(entityType.IsSqlReturningClauseUsed(tableIdentifier)); + Assert.True(entityType.IsSqlReturningClauseUsed()); + + modelBuilder + .Entity() + .ToTable("foo", tb => tb.UseSqlReturningClause(false)); + + Assert.False(entityType.IsSqlReturningClauseUsed(tableIdentifier)); + Assert.False(entityType.IsSqlReturningClauseUsed()); + + modelBuilder + .Entity() + .ToTable("foo", tb => tb.UseSqlReturningClause()); + + Assert.True(entityType.IsSqlReturningClauseUsed(tableIdentifier)); + Assert.True(entityType.IsSqlReturningClauseUsed()); + } + + [ConditionalFact] + public void Can_set_UseSqlReturningClause_with_table_name_and_two_tables() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder + .Entity() + .ToTable("foo") + .SplitToTable("bar", tb => tb.Property(c => c.Geometry)); + + var entityType = modelBuilder.Model.FindEntityType(typeof(Customer))!; + var fooTableIdentifier = StoreObjectIdentifier.Table("foo"); + var barTableIdentifier = StoreObjectIdentifier.Table("bar"); + + Assert.True(entityType.IsSqlReturningClauseUsed(fooTableIdentifier)); + Assert.True(entityType.IsSqlReturningClauseUsed(barTableIdentifier)); + Assert.True(entityType.IsSqlReturningClauseUsed()); + + modelBuilder + .Entity() + .SplitToTable("bar", tb => tb.UseSqlReturningClause(false)); + + Assert.False(entityType.IsSqlReturningClauseUsed(barTableIdentifier)); + Assert.True(entityType.IsSqlReturningClauseUsed(fooTableIdentifier)); + Assert.True(entityType.IsSqlReturningClauseUsed()); + + modelBuilder + .Entity() + .SplitToTable("bar", tb => tb.UseSqlReturningClause()); + + Assert.True(entityType.IsSqlReturningClauseUsed(barTableIdentifier)); + Assert.True(entityType.IsSqlReturningClauseUsed(fooTableIdentifier)); + Assert.True(entityType.IsSqlReturningClauseUsed()); + } + + [ConditionalFact] + public void Can_set_UseSqlReturningClause_non_generic() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder.Entity(typeof(Customer)); + var entityType = modelBuilder.Model.FindEntityType(typeof(Customer))!; + + Assert.True(entityType.IsSqlReturningClauseUsed()); + + modelBuilder + .Entity(typeof(Customer)) + .ToTable(tb => tb.UseSqlReturningClause(false)); + + Assert.False(entityType.IsSqlReturningClauseUsed()); + + modelBuilder + .Entity(typeof(Customer)) + .ToTable(tb => tb.UseSqlReturningClause()); + + Assert.True(entityType.IsSqlReturningClauseUsed()); + } + + #endregion UseSqlReturningClause + protected virtual ModelBuilder CreateConventionModelBuilder() => SqliteTestHelpers.Instance.CreateConventionBuilder(); diff --git a/test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs b/test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs index 5df55c3decd..6e7ec72499a 100644 --- a/test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs @@ -2502,6 +2502,147 @@ public void ProcessSkipNavigationRemoved( } } + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(true, true)] + [ConditionalTheory] + public void OnTriggerAdded_calls_conventions_in_order(bool useBuilder, bool useScope) + { + var conventions = new ConventionSet(); + + var convention1 = new TriggerAddedConvention(terminate: false); + var convention2 = new TriggerAddedConvention(terminate: true); + var convention3 = new TriggerAddedConvention(terminate: false); + conventions.TriggerAddedConventions.Add(convention1); + conventions.TriggerAddedConventions.Add(convention2); + conventions.TriggerAddedConventions.Add(convention3); + + var builder = new InternalModelBuilder(new Model(conventions)); + var entityBuilder = builder.Entity(typeof(Order), ConfigurationSource.Convention); + + var scope = useScope ? builder.Metadata.ConventionDispatcher.DelayConventions() : null; + + if (useBuilder) + { + var result = entityBuilder.HasTrigger("MyTrigger", ConfigurationSource.Convention); + + Assert.Equal(!useScope, result == null); + } + else + { + var result = entityBuilder.Metadata.AddTrigger("MyTrigger", ConfigurationSource.Convention); + + Assert.Equal(!useScope, result == null); + } + + if (useScope) + { + Assert.Empty(convention1.Calls); + Assert.Empty(convention2.Calls); + scope.Dispose(); + } + + Assert.Equal(new[] { "MyTrigger" }, convention1.Calls); + Assert.Equal(new[] { "MyTrigger" }, convention2.Calls); + Assert.Empty(convention3.Calls); + } + + private class TriggerAddedConvention : ITriggerAddedConvention + { + private readonly bool _terminate; + public readonly List Calls = new(); + + public TriggerAddedConvention(bool terminate) + { + _terminate = terminate; + } + + public void ProcessTriggerAdded(IConventionTriggerBuilder triggerBuilder, IConventionContext context) + { + Assert.True(triggerBuilder.Metadata.IsInModel); + + Calls.Add(triggerBuilder.Metadata.ModelName); + + if (_terminate) + { + triggerBuilder.Metadata.EntityType.RemoveTrigger(triggerBuilder.Metadata.ModelName); + + context.StopProcessing(); + } + } + } + + [InlineData(false)] + [InlineData(true)] + [ConditionalTheory] + public void OnTriggerRemoved_calls_conventions_in_order(bool useScope) + { + var conventions = new ConventionSet(); + + var convention1 = new TriggerRemovedConvention(terminate: false); + var convention2 = new TriggerRemovedConvention(terminate: true); + var convention3 = new TriggerRemovedConvention(terminate: false); + conventions.TriggerRemovedConventions.Add(convention1); + conventions.TriggerRemovedConventions.Add(convention2); + conventions.TriggerRemovedConventions.Add(convention3); + + var builder = new InternalModelBuilder(new Model(conventions)); + var entityBuilder = builder.Entity(typeof(Order), ConfigurationSource.Convention); + + var trigger = entityBuilder.Metadata.AddTrigger("MyTrigger", ConfigurationSource.Convention); + + var scope = useScope ? builder.Metadata.ConventionDispatcher.DelayConventions() : null; + + var result = entityBuilder.Metadata.RemoveTrigger(trigger.ModelName); + + if (useScope) + { + Assert.Same(trigger, result); + } + else + { + Assert.Null(result); + } + + if (useScope) + { + Assert.Empty(convention1.Calls); + Assert.Empty(convention2.Calls); + scope.Dispose(); + } + + Assert.Equal(new[] { "MyTrigger" }, convention1.Calls); + Assert.Equal(new[] { "MyTrigger" }, convention2.Calls); + Assert.Empty(convention3.Calls); + } + + private class TriggerRemovedConvention : ITriggerRemovedConvention + { + private readonly bool _terminate; + public readonly List Calls = new(); + + public TriggerRemovedConvention(bool terminate) + { + _terminate = terminate; + } + + public void ProcessTriggerRemoved( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionTrigger trigger, + IConventionContext context) + { + Assert.NotNull(entityTypeBuilder.Metadata.Builder); + + Calls.Add(trigger.ModelName); + + if (_terminate) + { + context.StopProcessing(); + } + } + } + [InlineData(false, false)] [InlineData(true, false)] [InlineData(false, true)]