From a3efd1a9a84b530e655f2948cc6716d948afe2ec 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 --- .../SqlServerEntityTypeExtensions.cs | 43 +++++++ .../SqlServerTableBuilderExtensions.cs | 98 ++++++++++++++ .../SqlServerConventionSetBuilder.cs | 1 + .../SqlServerOutputClauseConvention.cs | 52 ++++++++ .../Internal/SqlServerAnnotationNames.cs | 8 ++ .../Properties/SqlServerStrings.Designer.cs | 4 +- .../Properties/SqlServerStrings.resx | 4 +- .../Internal/SqlServerUpdateSqlGenerator.cs | 35 +++-- .../Extensions/SqliteEntityTypeExtensions.cs | 59 +++++++++ .../SqliteServiceCollectionExtensions.cs | 12 +- .../SqliteTableBuilderExtensions.cs | 96 ++++++++++++++ .../Internal/SqliteAnnotationNames.cs | 8 ++ .../SqliteLegacyUpdateSqlGenerator.cs | 88 ------------- .../Internal/SqliteUpdateSqlGenerator.cs | 120 +++++++++++++++++- ...tionIdentityWithoutOutputSqlServerTest.cs} | 10 +- ...tionSequenceWithoutOutputSqlServerTest.cs} | 10 +- ...enerationWithoutOutputSqlServerFixture.cs} | 4 +- ...nerationWithoutOutputSqlServerTestBase.cs} | 6 +- .../SqlServerOutputClauseConventionTest.cs | 35 +++++ .../StoreValueGenerationLegacySqliteTest.cs | 24 +++- 20 files changed, 575 insertions(+), 142 deletions(-) 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/SqliteTableBuilderExtensions.cs delete mode 100644 src/EFCore.Sqlite.Core/Update/Internal/SqliteLegacyUpdateSqlGenerator.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.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs b/src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs index 57d28e17e22..1c47cd4ec38 100644 --- a/src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs +++ b/src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs @@ -271,4 +271,47 @@ 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(); + + /// + /// 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 UseSqlOutputClause(this IReadOnlyEntityType entityType) + => entityType[SqlServerAnnotationNames.UseSqlOutputClause] as bool? ?? 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(); } diff --git a/src/EFCore.SqlServer/Extensions/SqlServerTableBuilderExtensions.cs b/src/EFCore.SqlServer/Extensions/SqlServerTableBuilderExtensions.cs index 42a906cc866..504d7f9af8d 100644 --- a/src/EFCore.SqlServer/Extensions/SqlServerTableBuilderExtensions.cs +++ b/src/EFCore.SqlServer/Extensions/SqlServerTableBuilderExtensions.cs @@ -10,6 +10,8 @@ namespace Microsoft.EntityFrameworkCore; /// public static class SqlServerTableBuilderExtensions { + #region IsTemporal + /// /// Configures the table as temporal. /// @@ -183,6 +185,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 +270,96 @@ 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) + { + tableBuilder.Metadata.UseSqlOutputClause(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.Metadata.UseSqlOutputClause(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 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) + { + tableBuilder.Metadata.UseSqlOutputClause(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 + { + tableBuilder.Metadata.UseSqlOutputClause(useSqlOutputClause); + + return tableBuilder; + } + + #endregion UseSqlOutputClause } 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/SqlServerOutputClauseConvention.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOutputClauseConvention.cs new file mode 100644 index 00000000000..39d354f5c16 --- /dev/null +++ b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOutputClauseConvention.cs @@ -0,0 +1,52 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// ReSharper disable once CheckNamespace +namespace Microsoft.EntityFrameworkCore.Metadata.Conventions; + +/// +/// 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 : IModelFinalizingConvention +{ + /// + /// 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 void ProcessModelFinalizing(IConventionModelBuilder modelBuilder, IConventionContext context) + { + foreach (var entityType in modelBuilder.Metadata.GetEntityTypes()) + { + // TODO: inheritance? + if (entityType.GetDeclaredTriggers().Any()) + { + entityType.UseSqlOutputClause(false); + } + } + } +} 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..424547566a5 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -204,13 +204,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..5a92242f140 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -282,10 +282,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..535f5c7f537 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,9 @@ protected override void AppendRowsAffectedWhereCondition(StringBuilder commandSt .Append("@@ROWCOUNT = ") .Append(expectedRowsAffected.ToString(CultureInfo.InvariantCulture)); - private static bool HasAnyTriggers(IReadOnlyModificationCommand command) + private static bool CanUseOutputClause(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(); + => command.Entries.Count > 0 + && command.Entries[0].EntityType.UseSqlOutputClause(); } diff --git a/src/EFCore.Sqlite.Core/Extensions/SqliteEntityTypeExtensions.cs b/src/EFCore.Sqlite.Core/Extensions/SqliteEntityTypeExtensions.cs new file mode 100644 index 00000000000..fc18196e09e --- /dev/null +++ b/src/EFCore.Sqlite.Core/Extensions/SqliteEntityTypeExtensions.cs @@ -0,0 +1,59 @@ +// 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; + +namespace Microsoft.EntityFrameworkCore.Sqlite.Extensions; + +/// +/// 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 UseSqlReturningClause(this IReadOnlyEntityType entityType) + => entityType[SqliteAnnotationNames.UseSqlReturningClause] as bool? ?? 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(); +} 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..9fc9aea6f40 --- /dev/null +++ b/src/EFCore.Sqlite.Core/Extensions/SqliteTableBuilderExtensions.cs @@ -0,0 +1,96 @@ +// 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.Sqlite.Extensions; + +/// +/// 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) + { + tableBuilder.Metadata.UseSqlReturningClause(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.Metadata.UseSqlReturningClause(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 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) + { + tableBuilder.Metadata.UseSqlReturningClause(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 + { + tableBuilder.Metadata.UseSqlReturningClause(useSqlReturningClause); + + return tableBuilder; + } +} 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/Update/Internal/SqliteLegacyUpdateSqlGenerator.cs b/src/EFCore.Sqlite.Core/Update/Internal/SqliteLegacyUpdateSqlGenerator.cs deleted file mode 100644 index 1d5d31db4e4..00000000000 --- a/src/EFCore.Sqlite.Core/Update/Internal/SqliteLegacyUpdateSqlGenerator.cs +++ /dev/null @@ -1,88 +0,0 @@ -// 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. - /// - 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..3067ac9fabc 100644 --- a/src/EFCore.Sqlite.Core/Update/Internal/SqliteUpdateSqlGenerator.cs +++ b/src/EFCore.Sqlite.Core/Update/Internal/SqliteUpdateSqlGenerator.cs @@ -1,6 +1,9 @@ // 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.Extensions; using Microsoft.EntityFrameworkCore.Sqlite.Internal; namespace Microsoft.EntityFrameworkCore.Sqlite.Update.Internal; @@ -11,8 +14,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 +27,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 +143,11 @@ public SqliteUpdateSqlGenerator(UpdateSqlGeneratorDependencies dependencies) /// public override string GenerateNextSequenceValueOperation(string name, string? schema) => throw new NotSupportedException(SqliteStrings.SequencesNotSupported); + + // Data seeding doesn't provide any entries, so we we don't know if the target table is compatible or not; assume it isn't to generate + // SQL that works everywhere. + private bool CanUseReturningClause(IReadOnlyModificationCommand command) + => _isReturningClauseSupported + && command.Entries.Count > 0 + && command.Entries[0].EntityType.UseSqlReturningClause(); } 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/Metadata/Conventions/SqlServerOutputClauseConventionTest.cs b/test/EFCore.SqlServer.Tests/Metadata/Conventions/SqlServerOutputClauseConventionTest.cs new file mode 100644 index 00000000000..066e8c0f280 --- /dev/null +++ b/test/EFCore.SqlServer.Tests/Metadata/Conventions/SqlServerOutputClauseConventionTest.cs @@ -0,0 +1,35 @@ +// 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; + +#nullable enable + +public class SqlServerOutputClauseConventionTest +{ + [ConditionalFact] + public void Output_clause_is_enabled_by_default() + { + var modelBuilder = SqlServerTestHelpers.Instance.CreateConventionBuilder(); + modelBuilder.Entity(); + var model = modelBuilder.FinalizeModel(); + + Assert.True(model.FindEntityType(typeof(Order))!.UseSqlOutputClause()); + } + + [ConditionalFact] + public void Trigger_disables_output_clause() + { + var modelBuilder = SqlServerTestHelpers.Instance.CreateConventionBuilder(); + modelBuilder.Entity().ToTable(t => t.HasTrigger("SomeTrigger")); + var model = modelBuilder.FinalizeModel(); + + Assert.False(model.FindEntityType(typeof(Order))!.UseSqlOutputClause()); + } + + private class Order + { + public int Id { get; set; } + public int CustomerId { get; set; } + } +} diff --git a/test/EFCore.Sqlite.FunctionalTests/Update/StoreValueGenerationLegacySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Update/StoreValueGenerationLegacySqliteTest.cs index 7cdb2995b11..24baf438098 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Update/StoreValueGenerationLegacySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Update/StoreValueGenerationLegacySqliteTest.cs @@ -1,16 +1,17 @@ // 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.Extensions; + 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 +439,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)); + } + } + } }