From e24ad8ef38d9e3b619addeb6b784da730fc02f81 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Wed, 2 Mar 2022 15:08:31 +0100 Subject: [PATCH] Use RETURNING/OUTPUT clause for UPDATE/DELETE Closes #27547 --- .../AffectedCountModificationCommandBatch.cs | 3 +- .../Update/UpdateSqlGenerator.cs | 69 ++--- .../Internal/SqlServerUpdateSqlGenerator.cs | 145 ++++++++++- .../Internal/SqliteUpdateSqlGenerator.cs | 20 -- .../Update/StoreValueGenerationTestBase.cs | 17 +- .../Update/UpdateSqlGeneratorTestBase.cs | 242 ++---------------- .../FakeProvider/FakeSqlGenerator.cs | 12 - .../Update/UpdateSqlGeneratorTest.cs | 48 ++++ .../DataAnnotationSqlServerTest.cs | 8 +- .../OptimisticConcurrencySqlServerTest.cs | 4 +- ...eteMappingInheritanceQuerySqlServerTest.cs | 8 +- .../Query/InheritanceQuerySqlServerTest.cs | 8 +- .../Query/TPTInheritanceQuerySqlServerTest.cs | 16 +- .../TPTTableSplittingSqlServerTest.cs | 8 +- .../TableSplittingSqlServerTest.cs | 8 +- .../Update/SqlServerUpdateSqlGeneratorTest.cs | 50 ++++ ...oreValueGenerationIdentitySqlServerTest.cs | 78 ++---- ...eGenerationIdentityTriggerSqlServerTest.cs | 2 +- ...oreValueGenerationSequenceSqlServerTest.cs | 71 ++--- ...eGenerationSequenceTriggerSqlServerTest.cs | 2 +- .../UpdatesSqlServerTest.cs | 24 +- .../Update/SqliteUpdateSqlGeneratorTest.cs | 50 ++++ .../Update/StoreValueGenerationSqliteTest.cs | 70 +++-- 23 files changed, 475 insertions(+), 488 deletions(-) diff --git a/src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs b/src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs index 88b0101dfb3..beae8113198 100644 --- a/src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs +++ b/src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs @@ -5,8 +5,7 @@ namespace Microsoft.EntityFrameworkCore.Update; /// /// -/// A for providers which append an SQL query to find out -/// how many rows were affected (see ). +/// A for providers which return values to find out how many rows were affected. /// /// /// This type is typically used by database providers; it is generally not used in application code. diff --git a/src/EFCore.Relational/Update/UpdateSqlGenerator.cs b/src/EFCore.Relational/Update/UpdateSqlGenerator.cs index 5ba908b3037..0d18aff8a37 100644 --- a/src/EFCore.Relational/Update/UpdateSqlGenerator.cs +++ b/src/EFCore.Relational/Update/UpdateSqlGenerator.cs @@ -97,20 +97,13 @@ public virtual ResultSetMapping AppendUpdateOperation( var conditionOperations = operations.Where(o => o.IsCondition).ToList(); var readOperations = operations.Where(o => o.IsRead).ToList(); - AppendUpdateCommand(commandStringBuilder, name, schema, writeOperations, conditionOperations); - - if (readOperations.Count > 0) - { - var keyOperations = operations.Where(o => o.IsKey).ToList(); - - requiresTransaction = true; - - return AppendSelectAffectedCommand(commandStringBuilder, name, schema, readOperations, keyOperations, commandPosition); - } - requiresTransaction = false; - return AppendSelectAffectedCountCommand(commandStringBuilder, name, schema, commandPosition); + AppendUpdateCommand( + commandStringBuilder, name, schema, writeOperations, readOperations, conditionOperations, + additionalReadValues: readOperations.Count == 0 ? "1" : null); + + return ResultSetMapping.LastInResultSet; } /// @@ -131,11 +124,12 @@ public virtual ResultSetMapping AppendDeleteOperation( var schema = command.Schema; var conditionOperations = command.ColumnModifications.Where(o => o.IsCondition).ToList(); - AppendDeleteCommand(commandStringBuilder, name, schema, conditionOperations); - requiresTransaction = false; - return AppendSelectAffectedCountCommand(commandStringBuilder, name, schema, commandPosition); + AppendDeleteCommand( + commandStringBuilder, name, schema, Array.Empty(), conditionOperations, additionalReadValues: "1"); + + return ResultSetMapping.LastInResultSet; } /// @@ -167,16 +161,21 @@ protected virtual void AppendInsertCommand( /// The name of the table. /// The table schema, or to use the default schema. /// The operations for each column. + /// The operations for column values to be read back. /// The operations used to generate the WHERE clause for the update. + /// Additional values to be read back. protected virtual void AppendUpdateCommand( StringBuilder commandStringBuilder, string name, string? schema, IReadOnlyList writeOperations, - IReadOnlyList conditionOperations) + IReadOnlyList readOperations, + IReadOnlyList conditionOperations, + string? additionalReadValues = null) { AppendUpdateCommandHeader(commandStringBuilder, name, schema, writeOperations); AppendWhereClause(commandStringBuilder, conditionOperations); + AppendReturningClause(commandStringBuilder, readOperations, additionalReadValues); commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator); } @@ -186,33 +185,23 @@ protected virtual void AppendUpdateCommand( /// The builder to which the SQL should be appended. /// The name of the table. /// The table schema, or to use the default schema. + /// The operations for column values to be read back. /// The operations used to generate the WHERE clause for the delete. + /// Additional values to be read back. protected virtual void AppendDeleteCommand( StringBuilder commandStringBuilder, string name, string? schema, - IReadOnlyList conditionOperations) + IReadOnlyList readOperations, + IReadOnlyList conditionOperations, + string? additionalReadValues = null) { AppendDeleteCommandHeader(commandStringBuilder, name, schema); AppendWhereClause(commandStringBuilder, conditionOperations); + AppendReturningClause(commandStringBuilder, readOperations, additionalReadValues); commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator); } - /// - /// 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 virtual ResultSetMapping AppendSelectAffectedCountCommand( - StringBuilder commandStringBuilder, - string name, - string? schema, - int commandPosition) - => ResultSetMapping.NoResultSet; - /// /// Appends a SQL command for selecting affected data. /// @@ -411,11 +400,13 @@ protected virtual void AppendValues( /// /// The builder to which the SQL should be appended. /// The operations for column values to be read back. + /// Additional values to be read back. protected virtual void AppendReturningClause( StringBuilder commandStringBuilder, - IReadOnlyList operations) + IReadOnlyList operations, + string? additionalValues = null) { - if (operations.Count > 0) + if (operations.Count > 0 || additionalValues is not null) { commandStringBuilder .AppendLine() @@ -424,6 +415,16 @@ protected virtual void AppendReturningClause( operations, SqlGenerationHelper, (sb, o, helper) => helper.DelimitIdentifier(sb, o.ColumnName)); + + if (additionalValues is not null) + { + if (operations.Count > 0) + { + commandStringBuilder.Append(", "); + } + + commandStringBuilder.Append(additionalValues); + } } } diff --git a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs index 7d08eb12835..c633fc85db5 100644 --- a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs +++ b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs @@ -47,8 +47,7 @@ public override ResultSetMapping AppendInsertOperation( // 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) - || !command.Entries[0].EntityType.Model.GetRelationalModel().FindTable(command.TableName, command.Schema)!.Triggers.Any()) + if (command.ColumnModifications.All(o => !o.IsRead) || !HasAnyTriggers(command)) { return base.AppendInsertOperation(commandStringBuilder, command, commandPosition, out requiresTransaction); } @@ -94,6 +93,124 @@ protected override void AppendInsertCommand( commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator); } + /// + /// 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 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. + if (!HasAnyTriggers(command)) + { + return base.AppendUpdateOperation(commandStringBuilder, command, commandPosition, out requiresTransaction); + } + + var name = command.TableName; + var schema = command.Schema; + var operations = command.ColumnModifications; + + var writeOperations = operations.Where(o => o.IsWrite).ToList(); + var conditionOperations = operations.Where(o => o.IsCondition).ToList(); + var readOperations = operations.Where(o => o.IsRead).ToList(); + + AppendUpdateCommand(commandStringBuilder, name, schema, writeOperations, Array.Empty(), conditionOperations); + + if (readOperations.Count > 0) + { + var keyOperations = operations.Where(o => o.IsKey).ToList(); + + requiresTransaction = true; + + return AppendSelectAffectedCommand(commandStringBuilder, name, schema, readOperations, keyOperations, commandPosition); + } + + requiresTransaction = false; + + return AppendSelectAffectedCountCommand(commandStringBuilder, name, schema, commandPosition); + } + + /// + /// 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 AppendUpdateCommand( + StringBuilder commandStringBuilder, + string name, + string? schema, + IReadOnlyList writeOperations, + IReadOnlyList readOperations, + IReadOnlyList conditionOperations, + string? additionalReadValues = null) + { + // In SQL Server the OUTPUT clause is placed differently (before the WHERE instead of at the end) + AppendUpdateCommandHeader(commandStringBuilder, name, schema, writeOperations); + AppendOutputClause(commandStringBuilder, readOperations, additionalReadValues); + AppendWhereClause(commandStringBuilder, conditionOperations); + commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator); + } + + /// + /// 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 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. + if (!HasAnyTriggers(command)) + { + return base.AppendDeleteOperation(commandStringBuilder, command, commandPosition, out requiresTransaction); + } + + var name = command.TableName; + var schema = command.Schema; + var operations = command.ColumnModifications; + + var conditionOperations = operations.Where(o => o.IsCondition).ToList(); + + requiresTransaction = false; + + AppendDeleteCommand(commandStringBuilder, name, schema, Array.Empty(), conditionOperations); + + return AppendSelectAffectedCountCommand(commandStringBuilder, name, schema, commandPosition); + } + + /// + /// 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 AppendDeleteCommand( + StringBuilder commandStringBuilder, + string name, + string? schema, + IReadOnlyList readOperations, + IReadOnlyList conditionOperations, + string? additionalReadValues = null) + { + // In SQL Server the OUTPUT clause is placed differently (before the WHERE instead of at the end) + AppendDeleteCommandHeader(commandStringBuilder, name, schema); + AppendOutputClause(commandStringBuilder, readOperations, additionalReadValues); + AppendWhereClause(commandStringBuilder, conditionOperations); + commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator); + } + /// /// 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 @@ -200,8 +317,7 @@ 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 (!firstCommand.Entries[0].EntityType.Model.GetRelationalModel().FindTable(firstCommand.TableName, firstCommand.Schema)! - .Triggers.Any()) + if (!HasAnyTriggers(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. @@ -578,16 +694,17 @@ private static string GetTypeNameForCopy(IProperty property) /// protected override void AppendReturningClause( StringBuilder commandStringBuilder, - IReadOnlyList operations) - => AppendOutputClause(commandStringBuilder, operations); + IReadOnlyList operations, + string? additionalValues = null) + => AppendOutputClause(commandStringBuilder, operations, additionalValues); // ReSharper disable once ParameterTypeCanBeEnumerable.Local private void AppendOutputClause( StringBuilder commandStringBuilder, IReadOnlyList operations, - string? additionalColumns = null) + string? additionalReadValues = null) { - if (operations.Count > 0 || additionalColumns is not null) + if (operations.Count > 0 || additionalReadValues is not null) { commandStringBuilder .AppendLine() @@ -601,14 +718,14 @@ private void AppendOutputClause( helper.DelimitIdentifier(sb, o.ColumnName); }); - if (additionalColumns != null) + if (additionalReadValues is not null) { if (operations.Count > 0) { commandStringBuilder.Append(", "); } - commandStringBuilder.Append(additionalColumns); + commandStringBuilder.Append(additionalReadValues); } } } @@ -728,7 +845,7 @@ private ResultSetMapping AppendSelectCommand( /// 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 ResultSetMapping AppendSelectAffectedCountCommand( + protected virtual ResultSetMapping AppendSelectAffectedCountCommand( StringBuilder commandStringBuilder, string name, string? schema, @@ -790,4 +907,10 @@ protected override void AppendRowsAffectedWhereCondition(StringBuilder commandSt => commandStringBuilder .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.Entries[0].EntityType.Model.GetRelationalModel().FindTable(command.TableName, command.Schema)!.Triggers.Any(); } diff --git a/src/EFCore.Sqlite.Core/Update/Internal/SqliteUpdateSqlGenerator.cs b/src/EFCore.Sqlite.Core/Update/Internal/SqliteUpdateSqlGenerator.cs index e92eb82e089..5ea20d8b67a 100644 --- a/src/EFCore.Sqlite.Core/Update/Internal/SqliteUpdateSqlGenerator.cs +++ b/src/EFCore.Sqlite.Core/Update/Internal/SqliteUpdateSqlGenerator.cs @@ -38,26 +38,6 @@ protected override void AppendIdentityWhereCondition(StringBuilder commandString .Append("last_insert_rowid()"); } - /// - /// 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 ResultSetMapping AppendSelectAffectedCountCommand( - StringBuilder commandStringBuilder, - string name, - string? schema, - int commandPosition) - { - commandStringBuilder - .Append("SELECT changes()") - .AppendLine(SqlGenerationHelper.StatementTerminator) - .AppendLine(); - - return ResultSetMapping.LastInResultSet; - } - /// /// 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/test/EFCore.Relational.Specification.Tests/Update/StoreValueGenerationTestBase.cs b/test/EFCore.Relational.Specification.Tests/Update/StoreValueGenerationTestBase.cs index 007f32d838c..f790bf1219f 100644 --- a/test/EFCore.Relational.Specification.Tests/Update/StoreValueGenerationTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Update/StoreValueGenerationTestBase.cs @@ -329,22 +329,7 @@ protected virtual bool ShouldCreateImplicitTransaction( EntityState? secondOperationType, GeneratedValues generatedValues, bool withSameEntityType) - { - // By default, two changes require a transaction - if (secondOperationType is not null) - { - return true; - } - - // Deletes don't ever need to bring back database-generated values, and inserts use the RETURNING clause - no transaction needed - if (firstOperationType is EntityState.Deleted or EntityState.Added) - { - return false; - } - - // Fetching back database-generated values from an update requires a transaction - return generatedValues != GeneratedValues.None; - } + => secondOperationType is not null; /// /// Providers can override this to specify how many commands (batches) are used to execute the update. diff --git a/test/EFCore.Relational.Specification.Tests/Update/UpdateSqlGeneratorTestBase.cs b/test/EFCore.Relational.Specification.Tests/Update/UpdateSqlGeneratorTestBase.cs index 8243c421905..d0544ef6232 100644 --- a/test/EFCore.Relational.Specification.Tests/Update/UpdateSqlGeneratorTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Update/UpdateSqlGeneratorTestBase.cs @@ -17,28 +17,11 @@ public virtual void AppendDeleteOperation_creates_full_delete_command_text() CreateSqlGenerator().AppendDeleteOperation(stringBuilder, command, 0); - Assert.Equal( - "DELETE FROM " - + SchemaPrefix - + OpenDelimiter - + "Ducks" - + CloseDelimiter - + "" - + Environment.NewLine - + "WHERE " - + OpenDelimiter - + "Id" - + CloseDelimiter - + " = @p0;" - + Environment.NewLine - + "SELECT " - + RowsAffected - + ";" - + Environment.NewLine - + Environment.NewLine, - stringBuilder.ToString()); + AppendDeleteOperation_creates_full_delete_command_text_verification(stringBuilder); } + protected abstract void AppendDeleteOperation_creates_full_delete_command_text_verification(StringBuilder stringBuilder); + [ConditionalFact] public virtual void AppendDeleteOperation_creates_full_delete_command_text_with_concurrency_check() { @@ -47,32 +30,12 @@ public virtual void AppendDeleteOperation_creates_full_delete_command_text_with_ CreateSqlGenerator().AppendDeleteOperation(stringBuilder, command, 0); - Assert.Equal( - "DELETE FROM " - + SchemaPrefix - + OpenDelimiter - + "Ducks" - + CloseDelimiter - + "" - + Environment.NewLine - + "WHERE " - + OpenDelimiter - + "Id" - + CloseDelimiter - + " = @p0 AND " - + OpenDelimiter - + "ConcurrencyToken" - + CloseDelimiter - + " IS NULL;" - + Environment.NewLine - + "SELECT " - + RowsAffected - + ";" - + Environment.NewLine - + Environment.NewLine, - stringBuilder.ToString()); + AppendDeleteOperation_creates_full_delete_command_text_with_concurrency_check_verification(stringBuilder); } + protected abstract void AppendDeleteOperation_creates_full_delete_command_text_with_concurrency_check_verification( + StringBuilder stringBuilder); + [ConditionalFact] public virtual void AppendInsertOperation_insert_if_store_generated_columns_exist() { @@ -177,114 +140,31 @@ public virtual void AppendInsertOperation_for_only_single_identity_columns() protected abstract void AppendInsertOperation_for_only_single_identity_columns_verification(StringBuilder stringBuilder); [ConditionalFact] - public virtual void AppendUpdateOperation_appends_update_and_select_if_store_generated_columns_exist() + public virtual void AppendUpdateOperation_if_store_generated_columns_exist() { var stringBuilder = new StringBuilder(); var command = CreateUpdateCommand(); CreateSqlGenerator().AppendUpdateOperation(stringBuilder, command, 0); - AppendUpdateOperation_appends_update_and_select_if_store_generated_columns_exist_verification(stringBuilder); + AppendUpdateOperation_if_store_generated_columns_exist_verification(stringBuilder); } - protected virtual void AppendUpdateOperation_appends_update_and_select_if_store_generated_columns_exist_verification( - StringBuilder stringBuilder) - => Assert.Equal( - "UPDATE " - + SchemaPrefix - + OpenDelimiter - + "Ducks" - + CloseDelimiter - + " SET " - + OpenDelimiter - + "Name" - + CloseDelimiter - + " = @p0, " - + OpenDelimiter - + "Quacks" - + CloseDelimiter - + " = @p1, " - + OpenDelimiter - + "ConcurrencyToken" - + CloseDelimiter - + " = @p2" - + Environment.NewLine - + "WHERE " - + OpenDelimiter - + "Id" - + CloseDelimiter - + " = @p3 AND " - + OpenDelimiter - + "ConcurrencyToken" - + CloseDelimiter - + " IS NULL;" - + Environment.NewLine - + "SELECT " - + OpenDelimiter - + "Computed" - + CloseDelimiter - + "" - + Environment.NewLine - + "FROM " - + SchemaPrefix - + OpenDelimiter - + "Ducks" - + CloseDelimiter - + "" - + Environment.NewLine - + "WHERE " - + RowsAffected - + " = 1 AND " - + OpenDelimiter - + "Id" - + CloseDelimiter - + " = @p3;" - + Environment.NewLine - + Environment.NewLine, - stringBuilder.ToString()); + protected abstract void AppendUpdateOperation_if_store_generated_columns_exist_verification(StringBuilder stringBuilder); [ConditionalFact] - public virtual void AppendUpdateOperation_appends_update_and_select_rowcount_if_store_generated_columns_dont_exist() + public virtual void AppendUpdateOperation_if_store_generated_columns_dont_exist() { var stringBuilder = new StringBuilder(); var command = CreateUpdateCommand(false, false); CreateSqlGenerator().AppendUpdateOperation(stringBuilder, command, 0); - Assert.Equal( - "UPDATE " - + SchemaPrefix - + OpenDelimiter - + "Ducks" - + CloseDelimiter - + " SET " - + OpenDelimiter - + "Name" - + CloseDelimiter - + " = @p0, " - + OpenDelimiter - + "Quacks" - + CloseDelimiter - + " = @p1, " - + OpenDelimiter - + "ConcurrencyToken" - + CloseDelimiter - + " = @p2" - + Environment.NewLine - + "WHERE " - + OpenDelimiter - + "Id" - + CloseDelimiter - + " = @p3;" - + Environment.NewLine - + "SELECT " - + RowsAffected - + ";" - + Environment.NewLine - + Environment.NewLine, - stringBuilder.ToString()); + AppendUpdateOperation_if_store_generated_columns_dont_exist_verification(stringBuilder); } + protected abstract void AppendUpdateOperation_if_store_generated_columns_dont_exist_verification(StringBuilder stringBuilder); + [ConditionalFact] public virtual void AppendUpdateOperation_appends_where_for_concurrency_token() { @@ -293,105 +173,23 @@ public virtual void AppendUpdateOperation_appends_where_for_concurrency_token() CreateSqlGenerator().AppendUpdateOperation(stringBuilder, command, 0); - Assert.Equal( - "UPDATE " - + SchemaPrefix - + OpenDelimiter - + "Ducks" - + CloseDelimiter - + " SET " - + OpenDelimiter - + "Name" - + CloseDelimiter - + " = @p0, " - + OpenDelimiter - + "Quacks" - + CloseDelimiter - + " = @p1, " - + OpenDelimiter - + "ConcurrencyToken" - + CloseDelimiter - + " = @p2" - + Environment.NewLine - + "WHERE " - + OpenDelimiter - + "Id" - + CloseDelimiter - + " = @p3 AND " - + OpenDelimiter - + "ConcurrencyToken" - + CloseDelimiter - + " IS NULL;" - + Environment.NewLine - + "SELECT " - + RowsAffected - + ";" - + Environment.NewLine - + Environment.NewLine, - stringBuilder.ToString()); + AppendUpdateOperation_appends_where_for_concurrency_token_verification(stringBuilder); } + protected abstract void AppendUpdateOperation_appends_where_for_concurrency_token_verification(StringBuilder stringBuilder); + [ConditionalFact] - public virtual void AppendUpdateOperation_appends_select_for_computed_property() + public virtual void AppendUpdateOperation_for_computed_property() { var stringBuilder = new StringBuilder(); var command = CreateUpdateCommand(true, false); CreateSqlGenerator().AppendUpdateOperation(stringBuilder, command, 0); - AppendUpdateOperation_appends_select_for_computed_property_verification(stringBuilder); + AppendUpdateOperation_for_computed_property_verification(stringBuilder); } - protected virtual void AppendUpdateOperation_appends_select_for_computed_property_verification(StringBuilder stringBuilder) - => Assert.Equal( - "UPDATE " - + SchemaPrefix - + OpenDelimiter - + "Ducks" - + CloseDelimiter - + " SET " - + OpenDelimiter - + "Name" - + CloseDelimiter - + " = @p0, " - + OpenDelimiter - + "Quacks" - + CloseDelimiter - + " = @p1, " - + OpenDelimiter - + "ConcurrencyToken" - + CloseDelimiter - + " = @p2" - + Environment.NewLine - + "WHERE " - + OpenDelimiter - + "Id" - + CloseDelimiter - + " = @p3;" - + Environment.NewLine - + "SELECT " - + OpenDelimiter - + "Computed" - + CloseDelimiter - + "" - + Environment.NewLine - + "FROM " - + SchemaPrefix - + OpenDelimiter - + "Ducks" - + CloseDelimiter - + "" - + Environment.NewLine - + "WHERE " - + RowsAffected - + " = 1 AND " - + OpenDelimiter - + "Id" - + CloseDelimiter - + " = @p3;" - + Environment.NewLine - + Environment.NewLine, - stringBuilder.ToString()); + protected abstract void AppendUpdateOperation_for_computed_property_verification(StringBuilder stringBuilder); [ConditionalFact] public virtual void GenerateNextSequenceValueOperation_returns_statement_with_sanitized_sequence() diff --git a/test/EFCore.Relational.Tests/TestUtilities/FakeProvider/FakeSqlGenerator.cs b/test/EFCore.Relational.Tests/TestUtilities/FakeProvider/FakeSqlGenerator.cs index 3a45ae9629b..84e737066f4 100644 --- a/test/EFCore.Relational.Tests/TestUtilities/FakeProvider/FakeSqlGenerator.cs +++ b/test/EFCore.Relational.Tests/TestUtilities/FakeProvider/FakeSqlGenerator.cs @@ -57,18 +57,6 @@ protected override void AppendIdentityWhereCondition(StringBuilder commandString .Append(" = ") .Append("provider_specific_identity()"); - protected override ResultSetMapping AppendSelectAffectedCountCommand( - StringBuilder commandStringBuilder, - string name, - string schema, - int commandPosition) - { - commandStringBuilder - .Append("SELECT provider_specific_rowcount();").Append(Environment.NewLine).Append(Environment.NewLine); - - return ResultSetMapping.LastInResultSet; - } - protected override void AppendRowsAffectedWhereCondition(StringBuilder commandStringBuilder, int expectedRowsAffected) => commandStringBuilder .Append("provider_specific_rowcount() = ").Append(expectedRowsAffected); diff --git a/test/EFCore.Relational.Tests/Update/UpdateSqlGeneratorTest.cs b/test/EFCore.Relational.Tests/Update/UpdateSqlGeneratorTest.cs index 825deece69c..34c232b0733 100644 --- a/test/EFCore.Relational.Tests/Update/UpdateSqlGeneratorTest.cs +++ b/test/EFCore.Relational.Tests/Update/UpdateSqlGeneratorTest.cs @@ -16,6 +16,22 @@ protected override IUpdateSqlGenerator CreateSqlGenerator() TestServiceFactory.Instance.Create(), TestServiceFactory.Instance.Create()))); + protected override void AppendDeleteOperation_creates_full_delete_command_text_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"DELETE FROM ""dbo"".""Ducks"" +WHERE ""Id"" = @p0 +RETURNING 1; +", + stringBuilder.ToString()); + + protected override void AppendDeleteOperation_creates_full_delete_command_text_with_concurrency_check_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"DELETE FROM ""dbo"".""Ducks"" +WHERE ""Id"" = @p0 AND ""ConcurrencyToken"" IS NULL +RETURNING 1; +", + stringBuilder.ToString()); + protected override void AppendInsertOperation_insert_if_store_generated_columns_exist_verification(StringBuilder stringBuilder) => AssertBaseline( @"INSERT INTO ""dbo"".""Ducks"" (""Name"", ""Quacks"", ""ConcurrencyToken"") @@ -55,6 +71,38 @@ protected override void AppendInsertOperation_for_only_single_identity_columns_v @"INSERT INTO ""dbo"".""Ducks"" DEFAULT VALUES RETURNING ""Id""; +", + stringBuilder.ToString()); + + protected override void AppendUpdateOperation_if_store_generated_columns_exist_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"UPDATE ""dbo"".""Ducks"" SET ""Name"" = @p0, ""Quacks"" = @p1, ""ConcurrencyToken"" = @p2 +WHERE ""Id"" = @p3 AND ""ConcurrencyToken"" IS NULL +RETURNING ""Computed""; +", + stringBuilder.ToString()); + + protected override void AppendUpdateOperation_if_store_generated_columns_dont_exist_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"UPDATE ""dbo"".""Ducks"" SET ""Name"" = @p0, ""Quacks"" = @p1, ""ConcurrencyToken"" = @p2 +WHERE ""Id"" = @p3 +RETURNING 1; +", + stringBuilder.ToString()); + + protected override void AppendUpdateOperation_appends_where_for_concurrency_token_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"UPDATE ""dbo"".""Ducks"" SET ""Name"" = @p0, ""Quacks"" = @p1, ""ConcurrencyToken"" = @p2 +WHERE ""Id"" = @p3 AND ""ConcurrencyToken"" IS NULL +RETURNING 1; +", + stringBuilder.ToString()); + + protected override void AppendUpdateOperation_for_computed_property_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"UPDATE ""dbo"".""Ducks"" SET ""Name"" = @p0, ""Quacks"" = @p1, ""ConcurrencyToken"" = @p2 +WHERE ""Id"" = @p3 +RETURNING ""Computed""; ", stringBuilder.ToString()); diff --git a/test/EFCore.SqlServer.FunctionalTests/DataAnnotationSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/DataAnnotationSqlServerTest.cs index 77547e4d679..f8cab2a2ace 100644 --- a/test/EFCore.SqlServer.FunctionalTests/DataAnnotationSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/DataAnnotationSqlServerTest.cs @@ -167,8 +167,8 @@ FROM [Sample] AS [s] SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [Sample] SET [Name] = @p0, [RowVersion] = @p1 -WHERE [Unique_No] = @p2 AND [RowVersion] = @p3; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Unique_No] = @p2 AND [RowVersion] = @p3;", // @"@p2='1' @p0='ChangedData' (Nullable = false) (Size = 4000) @@ -178,8 +178,8 @@ FROM [Sample] AS [s] SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [Sample] SET [Name] = @p0, [RowVersion] = @p1 -WHERE [Unique_No] = @p2 AND [RowVersion] = @p3; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Unique_No] = @p2 AND [RowVersion] = @p3;"); } public override void DatabaseGeneratedAttribute_autogenerates_values_when_set_to_identity() diff --git a/test/EFCore.SqlServer.FunctionalTests/OptimisticConcurrencySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/OptimisticConcurrencySqlServerTest.cs index 469a2b2120b..21b4c363f26 100644 --- a/test/EFCore.SqlServer.FunctionalTests/OptimisticConcurrencySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/OptimisticConcurrencySqlServerTest.cs @@ -169,8 +169,8 @@ FROM [Engines] AS [e] SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [Engines] SET [Name] = @p0 -WHERE [Id] = @p1 AND [EngineSupplierId] = @p2 AND [Name] = @p3 AND [StorageLocation_Latitude] = @p4 AND [StorageLocation_Longitude] = @p5; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p1 AND [EngineSupplierId] = @p2 AND [Name] = @p3 AND [StorageLocation_Latitude] = @p4 AND [StorageLocation_Longitude] = @p5;"); } private void AssertSql(params string[] expected) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/IncompleteMappingInheritanceQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/IncompleteMappingInheritanceQuerySqlServerTest.cs index 85a40748b6c..98a6c6c600e 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/IncompleteMappingInheritanceQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/IncompleteMappingInheritanceQuerySqlServerTest.cs @@ -404,8 +404,8 @@ FROM [Animals] AS [a] SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [Animals] SET [EagleId] = @p0 -WHERE [Species] = @p1; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Species] = @p1;", // @"SELECT TOP(2) [a].[Species], [a].[CountryId], [a].[Discriminator], [a].[Name], [a].[EagleId], [a].[IsFlightless], [a].[FoundOn] FROM [Animals] AS [a] @@ -416,8 +416,8 @@ FROM [Animals] AS [a] SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; DELETE FROM [Animals] -WHERE [Species] = @p0; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Species] = @p0;", // @"SELECT COUNT(*) FROM [Animals] AS [a] diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/InheritanceQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/InheritanceQuerySqlServerTest.cs index 3f58c6ef539..cf4ed45898d 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/InheritanceQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/InheritanceQuerySqlServerTest.cs @@ -378,8 +378,8 @@ FROM [Animals] AS [a] SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [Animals] SET [EagleId] = @p0 -WHERE [Species] = @p1; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Species] = @p1;", // @"SELECT TOP(2) [a].[Species], [a].[CountryId], [a].[Discriminator], [a].[Name], [a].[EagleId], [a].[IsFlightless], [a].[FoundOn] FROM [Animals] AS [a] @@ -390,8 +390,8 @@ FROM [Animals] AS [a] SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; DELETE FROM [Animals] -WHERE [Species] = @p0; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Species] = @p0;", // @"SELECT COUNT(*) FROM [Animals] AS [a] diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TPTInheritanceQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TPTInheritanceQuerySqlServerTest.cs index 6c45fc18098..a31d28b8f50 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/TPTInheritanceQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/TPTInheritanceQuerySqlServerTest.cs @@ -136,8 +136,8 @@ FROM [Animals] AS [a] SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [Birds] SET [EagleId] = @p0 -WHERE [Species] = @p1; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Species] = @p1;", // @"SELECT TOP(2) [a].[Species], [a].[CountryId], [a].[Name], [b].[EagleId], [b].[IsFlightless], [k].[FoundOn] FROM [Animals] AS [a] @@ -150,24 +150,24 @@ FROM [Animals] AS [a] SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; DELETE FROM [Kiwi] -WHERE [Species] = @p0; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Species] = @p0;", // @"@p1='Apteryx owenii' (Nullable = false) (Size = 100) SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; DELETE FROM [Birds] -WHERE [Species] = @p1; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Species] = @p1;", // @"@p2='Apteryx owenii' (Nullable = false) (Size = 100) SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; DELETE FROM [Animals] -WHERE [Species] = @p2; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Species] = @p2;", // @"SELECT COUNT(*) FROM [Animals] AS [a] diff --git a/test/EFCore.SqlServer.FunctionalTests/TPTTableSplittingSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/TPTTableSplittingSqlServerTest.cs index 4f9bbb9011a..518ec2d7f57 100644 --- a/test/EFCore.SqlServer.FunctionalTests/TPTTableSplittingSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/TPTTableSplittingSqlServerTest.cs @@ -190,8 +190,8 @@ public override async Task Can_change_dependent_instance_non_derived() SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [Vehicles] SET [Operator_Name] = @p0 -WHERE [Name] = @p1; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Name] = @p1;", // @"@p2='Trek Pro Fit Madone 6 Series' (Nullable = false) (Size = 450) @p3='Repair' (Size = 4000) @@ -233,8 +233,8 @@ public override async Task Can_change_principal_instance_non_derived() SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [Vehicles] SET [SeatingCapacity] = @p0 -WHERE [Name] = @p1; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Name] = @p1;", // @"SELECT TOP(2) [v].[Name], [v].[SeatingCapacity], [c].[AttachedVehicleName], CASE WHEN [c].[Name] IS NOT NULL THEN N'CompositeVehicle' diff --git a/test/EFCore.SqlServer.FunctionalTests/TableSplittingSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/TableSplittingSqlServerTest.cs index 4fa4ba61080..30f4308c715 100644 --- a/test/EFCore.SqlServer.FunctionalTests/TableSplittingSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/TableSplittingSqlServerTest.cs @@ -213,8 +213,8 @@ public override async Task Can_change_dependent_instance_non_derived() SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [Vehicles] SET [Operator_Discriminator] = @p0, [LicenseType] = @p1, [Operator_Name] = @p2 -WHERE [Name] = @p3; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Name] = @p3;", // @"SELECT TOP(2) [v].[Name], [v].[Discriminator], [v].[SeatingCapacity], [v].[AttachedVehicleName], [t].[Name], [t].[Operator_Discriminator], [t].[Operator_Name], [t].[LicenseType] FROM [Vehicles] AS [v] @@ -237,8 +237,8 @@ public override async Task Can_change_principal_instance_non_derived() SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [Vehicles] SET [SeatingCapacity] = @p0 -WHERE [Name] = @p1; -SELECT @@ROWCOUNT;", +OUTPUT 1 +WHERE [Name] = @p1;", // @"SELECT TOP(2) [v].[Name], [v].[Discriminator], [v].[SeatingCapacity], [v].[AttachedVehicleName], [t].[Name], [t].[Operator_Discriminator], [t].[Operator_Name], [t].[LicenseType] FROM [Vehicles] AS [v] diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs index 2d18a03898d..b57726caabc 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs @@ -154,6 +154,56 @@ public void AppendBulkInsertOperation_appends_insert_if_no_store_generated_colum Assert.Equal(ResultSetMapping.NoResultSet, grouping); } + protected override void AppendUpdateOperation_for_computed_property_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"UPDATE [dbo].[Ducks] SET [Name] = @p0, [Quacks] = @p1, [ConcurrencyToken] = @p2 +OUTPUT INSERTED.[Computed] +WHERE [Id] = @p3; +", + stringBuilder.ToString()); + + protected override void AppendUpdateOperation_if_store_generated_columns_exist_verification( + StringBuilder stringBuilder) + => AssertBaseline( + @"UPDATE [dbo].[Ducks] SET [Name] = @p0, [Quacks] = @p1, [ConcurrencyToken] = @p2 +OUTPUT INSERTED.[Computed] +WHERE [Id] = @p3 AND [ConcurrencyToken] IS NULL; +", + stringBuilder.ToString()); + + protected override void AppendUpdateOperation_if_store_generated_columns_dont_exist_verification( + StringBuilder stringBuilder) + => AssertBaseline( + @"UPDATE [dbo].[Ducks] SET [Name] = @p0, [Quacks] = @p1, [ConcurrencyToken] = @p2 +OUTPUT 1 +WHERE [Id] = @p3; +", + stringBuilder.ToString()); + + protected override void AppendUpdateOperation_appends_where_for_concurrency_token_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"UPDATE [dbo].[Ducks] SET [Name] = @p0, [Quacks] = @p1, [ConcurrencyToken] = @p2 +OUTPUT 1 +WHERE [Id] = @p3 AND [ConcurrencyToken] IS NULL; +", + stringBuilder.ToString()); + + protected override void AppendDeleteOperation_creates_full_delete_command_text_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"DELETE FROM [dbo].[Ducks] +OUTPUT 1 +WHERE [Id] = @p0; +", + stringBuilder.ToString()); + + protected override void AppendDeleteOperation_creates_full_delete_command_text_with_concurrency_check_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"DELETE FROM [dbo].[Ducks] +OUTPUT 1 +WHERE [Id] = @p0 AND [ConcurrencyToken] IS NULL; +", + stringBuilder.ToString()); + protected override string RowsAffected => "@@ROWCOUNT"; diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentitySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentitySqlServerTest.cs index ec27e33ef3a..b68da2be60e 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentitySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentitySqlServerTest.cs @@ -1,8 +1,6 @@ // 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.TestModels.StoreValueGenerationModel; - namespace Microsoft.EntityFrameworkCore.Update; #nullable enable @@ -25,12 +23,6 @@ protected override bool ShouldCreateImplicitTransaction( GeneratedValues generatedValues, bool withSameEntityType) { - // Updates with generated values currently use SELECT to retrieve them, and so require transactions - if (firstOperationType == EntityState.Modified && generatedValues != GeneratedValues.None) - { - return true; - } - // For multiple operations, we specifically optimize multiple insertions of the same entity type with a single command (e.g. MERGE) // (as long as there are writable columns) if (firstOperationType is EntityState.Added @@ -96,12 +88,11 @@ public override async Task Modify_with_generated_values(bool async) @"@p1='1' @p0='1000' +SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [WithSomeDatabaseGenerated] SET [Data2] = @p0 -WHERE [Id] = @p1; -SELECT [Data1] -FROM [WithSomeDatabaseGenerated] -WHERE @@ROWCOUNT = 1 AND [Id] = @p1;"); +OUTPUT INSERTED.[Data1] +WHERE [Id] = @p1;"); } public override async Task Modify_with_no_generated_values(bool async) @@ -116,8 +107,8 @@ public override async Task Modify_with_no_generated_values(bool async) SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [WithNoDatabaseGenerated] SET [Data1] = @p0, [Data2] = @p1 -WHERE [Id] = @p2; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p2;"); } public override async Task Delete(bool async) @@ -130,8 +121,8 @@ public override async Task Delete(bool async) SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; DELETE FROM [WithSomeDatabaseGenerated] -WHERE [Id] = @p0; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p0;"); } #endregion Single operation @@ -202,16 +193,11 @@ public override async Task Modify_Modify_with_same_entity_type_and_generated_val SET NOCOUNT ON; UPDATE [WithSomeDatabaseGenerated] SET [Data2] = @p0 +OUTPUT INSERTED.[Data1] WHERE [Id] = @p1; -SELECT [Data1] -FROM [WithSomeDatabaseGenerated] -WHERE @@ROWCOUNT = 1 AND [Id] = @p1; - UPDATE [WithSomeDatabaseGenerated] SET [Data2] = @p2 -WHERE [Id] = @p3; -SELECT [Data1] -FROM [WithSomeDatabaseGenerated] -WHERE @@ROWCOUNT = 1 AND [Id] = @p3;"); +OUTPUT INSERTED.[Data1] +WHERE [Id] = @p3;"); } public override async Task Modify_Modify_with_same_entity_type_and_no_generated_values(bool async) @@ -228,12 +214,11 @@ public override async Task Modify_Modify_with_same_entity_type_and_no_generated_ SET NOCOUNT ON; UPDATE [WithNoDatabaseGenerated] SET [Data1] = @p0, [Data2] = @p1 +OUTPUT 1 WHERE [Id] = @p2; -SELECT @@ROWCOUNT; - UPDATE [WithNoDatabaseGenerated] SET [Data1] = @p3, [Data2] = @p4 -WHERE [Id] = @p5; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p5;"); } public override async Task Delete_Delete_with_same_entity_type(bool async) @@ -246,12 +231,11 @@ public override async Task Delete_Delete_with_same_entity_type(bool async) SET NOCOUNT ON; DELETE FROM [WithSomeDatabaseGenerated] +OUTPUT 1 WHERE [Id] = @p0; -SELECT @@ROWCOUNT; - DELETE FROM [WithSomeDatabaseGenerated] -WHERE [Id] = @p1; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p1;"); } #endregion Two operations with same entity type @@ -320,23 +304,19 @@ public override async Task Modify_Modify_with_different_entity_types_and_generat SET NOCOUNT ON; UPDATE [WithSomeDatabaseGenerated] SET [Data2] = @p0 +OUTPUT INSERTED.[Data1] WHERE [Id] = @p1; -SELECT [Data1] -FROM [WithSomeDatabaseGenerated] -WHERE @@ROWCOUNT = 1 AND [Id] = @p1; - UPDATE [WithSomeDatabaseGenerated2] SET [Data2] = @p2 -WHERE [Id] = @p3; -SELECT [Data1] -FROM [WithSomeDatabaseGenerated2] -WHERE @@ROWCOUNT = 1 AND [Id] = @p3;"); +OUTPUT INSERTED.[Data1] +WHERE [Id] = @p3;"); } public override async Task Modify_Modify_with_different_entity_types_and_no_generated_values(bool async) { await base.Modify_Modify_with_different_entity_types_and_no_generated_values(async); -AssertSql( - @"@p2='1' + + AssertSql( + @"@p2='1' @p0='1000' @p1='1000' @p5='2' @@ -345,12 +325,11 @@ public override async Task Modify_Modify_with_different_entity_types_and_no_gene SET NOCOUNT ON; UPDATE [WithNoDatabaseGenerated] SET [Data1] = @p0, [Data2] = @p1 +OUTPUT 1 WHERE [Id] = @p2; -SELECT @@ROWCOUNT; - UPDATE [WithNoDatabaseGenerated2] SET [Data1] = @p3, [Data2] = @p4 -WHERE [Id] = @p5; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p5;"); } public override async Task Delete_Delete_with_different_entity_types(bool async) @@ -363,12 +342,11 @@ public override async Task Delete_Delete_with_different_entity_types(bool async) SET NOCOUNT ON; DELETE FROM [WithSomeDatabaseGenerated] +OUTPUT 1 WHERE [Id] = @p0; -SELECT @@ROWCOUNT; - DELETE FROM [WithSomeDatabaseGenerated2] -WHERE [Id] = @p1; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p1;"); } #endregion Two operations with different entity types diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentityTriggerSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentityTriggerSqlServerTest.cs index 2f2fb166ba2..2dffa61a266 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentityTriggerSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationIdentityTriggerSqlServerTest.cs @@ -24,7 +24,7 @@ protected override bool ShouldCreateImplicitTransaction( bool withSameEntityType) { // We have triggers, so any insert/update retrieving a database-generated value must be enclosed in a transaction - // (we use INSERT+SELECT or INSERT ... OUTPUT INTO+SELECT) + // (e.g. we use INSERT/UPDATE+SELECT or INSERT ... OUTPUT INTO+SELECT) if (generatedValues is GeneratedValues.Some or GeneratedValues.All && firstOperationType is EntityState.Added or EntityState.Modified) { diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceSqlServerTest.cs index f56a6747fc9..b32fade1836 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceSqlServerTest.cs @@ -25,12 +25,6 @@ protected override bool ShouldCreateImplicitTransaction( GeneratedValues generatedValues, bool withSameEntityType) { - // Updates with generated values currently use SELECT to retrieve them, and so require transactions - if (firstOperationType == EntityState.Modified && generatedValues != GeneratedValues.None) - { - return true; - } - // For multiple operations, we specifically optimize multiple insertions of the same entity type with a single command (e.g. MERGE) // (as long as there are writable columns) if (firstOperationType is EntityState.Added @@ -96,12 +90,11 @@ public override async Task Modify_with_generated_values(bool async) @"@p1='5' @p0='1000' +SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [WithSomeDatabaseGenerated] SET [Data2] = @p0 -WHERE [Id] = @p1; -SELECT [Data1] -FROM [WithSomeDatabaseGenerated] -WHERE @@ROWCOUNT = 1 AND [Id] = @p1;"); +OUTPUT INSERTED.[Data1] +WHERE [Id] = @p1;"); } public override async Task Modify_with_no_generated_values(bool async) @@ -116,8 +109,8 @@ public override async Task Modify_with_no_generated_values(bool async) SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [WithNoDatabaseGenerated] SET [Data1] = @p0, [Data2] = @p1 -WHERE [Id] = @p2; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p2;"); } public override async Task Delete(bool async) @@ -130,8 +123,8 @@ public override async Task Delete(bool async) SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; DELETE FROM [WithSomeDatabaseGenerated] -WHERE [Id] = @p0; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p0;"); } #endregion Single operation @@ -204,16 +197,11 @@ public override async Task Modify_Modify_with_same_entity_type_and_generated_val SET NOCOUNT ON; UPDATE [WithSomeDatabaseGenerated] SET [Data2] = @p0 +OUTPUT INSERTED.[Data1] WHERE [Id] = @p1; -SELECT [Data1] -FROM [WithSomeDatabaseGenerated] -WHERE @@ROWCOUNT = 1 AND [Id] = @p1; - UPDATE [WithSomeDatabaseGenerated] SET [Data2] = @p2 -WHERE [Id] = @p3; -SELECT [Data1] -FROM [WithSomeDatabaseGenerated] -WHERE @@ROWCOUNT = 1 AND [Id] = @p3;"); +OUTPUT INSERTED.[Data1] +WHERE [Id] = @p3;"); } public override async Task Modify_Modify_with_same_entity_type_and_no_generated_values(bool async) @@ -230,12 +218,11 @@ public override async Task Modify_Modify_with_same_entity_type_and_no_generated_ SET NOCOUNT ON; UPDATE [WithNoDatabaseGenerated] SET [Data1] = @p0, [Data2] = @p1 +OUTPUT 1 WHERE [Id] = @p2; -SELECT @@ROWCOUNT; - UPDATE [WithNoDatabaseGenerated] SET [Data1] = @p3, [Data2] = @p4 -WHERE [Id] = @p5; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p5;"); } public override async Task Delete_Delete_with_same_entity_type(bool async) @@ -248,12 +235,11 @@ public override async Task Delete_Delete_with_same_entity_type(bool async) SET NOCOUNT ON; DELETE FROM [WithSomeDatabaseGenerated] +OUTPUT 1 WHERE [Id] = @p0; -SELECT @@ROWCOUNT; - DELETE FROM [WithSomeDatabaseGenerated] -WHERE [Id] = @p1; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p1;"); } #endregion Two operations with same entity type @@ -322,16 +308,11 @@ public override async Task Modify_Modify_with_different_entity_types_and_generat SET NOCOUNT ON; UPDATE [WithSomeDatabaseGenerated] SET [Data2] = @p0 +OUTPUT INSERTED.[Data1] WHERE [Id] = @p1; -SELECT [Data1] -FROM [WithSomeDatabaseGenerated] -WHERE @@ROWCOUNT = 1 AND [Id] = @p1; - UPDATE [WithSomeDatabaseGenerated2] SET [Data2] = @p2 -WHERE [Id] = @p3; -SELECT [Data1] -FROM [WithSomeDatabaseGenerated2] -WHERE @@ROWCOUNT = 1 AND [Id] = @p3;"); +OUTPUT INSERTED.[Data1] +WHERE [Id] = @p3;"); } public override async Task Modify_Modify_with_different_entity_types_and_no_generated_values(bool async) @@ -347,12 +328,11 @@ public override async Task Modify_Modify_with_different_entity_types_and_no_gene SET NOCOUNT ON; UPDATE [WithNoDatabaseGenerated] SET [Data1] = @p0, [Data2] = @p1 +OUTPUT 1 WHERE [Id] = @p2; -SELECT @@ROWCOUNT; - UPDATE [WithNoDatabaseGenerated2] SET [Data1] = @p3, [Data2] = @p4 -WHERE [Id] = @p5; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p5;"); } public override async Task Delete_Delete_with_different_entity_types(bool async) @@ -365,12 +345,11 @@ public override async Task Delete_Delete_with_different_entity_types(bool async) SET NOCOUNT ON; DELETE FROM [WithSomeDatabaseGenerated] +OUTPUT 1 WHERE [Id] = @p0; -SELECT @@ROWCOUNT; - DELETE FROM [WithSomeDatabaseGenerated2] -WHERE [Id] = @p1; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p1;"); } #endregion Two operations with different entity types diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceTriggerSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceTriggerSqlServerTest.cs index 8c1d5e85ce9..055a0351ca8 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceTriggerSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/StoreValueGenerationSequenceTriggerSqlServerTest.cs @@ -28,7 +28,7 @@ protected override bool ShouldCreateImplicitTransaction( bool withSameEntityType) { // We have triggers, so any insert/update retrieving a database-generated value must be enclosed in a transaction - // (we use INSERT+SELECT or INSERT ... OUTPUT INTO+SELECT) + // (e.g. we use INSERT/UPDATE+SELECT or INSERT ... OUTPUT INTO+SELECT) if (generatedValues is GeneratedValues.Some or GeneratedValues.All && firstOperationType is EntityState.Added or EntityState.Modified) { diff --git a/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs index 1d2c6b2b925..5e33fb7b297 100644 --- a/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs @@ -111,15 +111,33 @@ public override void Save_replaced_principal() { base.Save_replaced_principal(); - AssertContainsSql( + AssertSql( + @"SELECT TOP(2) [c].[Id], [c].[Name], [c].[PrincipalId] +FROM [Categories] AS [c]", + // + @"@__category_PrincipalId_0='778' (Nullable = true) + +SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[Name], [p].[Price] +FROM [ProductBase] AS [p] +WHERE [p].[Discriminator] = N'Product' AND [p].[DependentId] = @__category_PrincipalId_0", + // @"@p1='78' @p0='New Category' (Size = 4000) SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; UPDATE [Categories] SET [Name] = @p0 -WHERE [Id] = @p1; -SELECT @@ROWCOUNT;"); +OUTPUT 1 +WHERE [Id] = @p1;", + // + @"SELECT TOP(2) [c].[Id], [c].[Name], [c].[PrincipalId] +FROM [Categories] AS [c]", + // + @"@__category_PrincipalId_0='778' (Nullable = true) + +SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[Name], [p].[Price] +FROM [ProductBase] AS [p] +WHERE [p].[Discriminator] = N'Product' AND [p].[DependentId] = @__category_PrincipalId_0"); } public override void Identifiers_are_generated_correctly() diff --git a/test/EFCore.Sqlite.FunctionalTests/Update/SqliteUpdateSqlGeneratorTest.cs b/test/EFCore.Sqlite.FunctionalTests/Update/SqliteUpdateSqlGeneratorTest.cs index 94da6a946f2..c49b1709151 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Update/SqliteUpdateSqlGeneratorTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Update/SqliteUpdateSqlGeneratorTest.cs @@ -80,6 +80,56 @@ protected override void AppendInsertOperation_for_only_single_identity_columns_v @"INSERT INTO ""Ducks"" DEFAULT VALUES RETURNING ""Id""; +", + stringBuilder.ToString()); + + protected override void AppendUpdateOperation_for_computed_property_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"UPDATE ""Ducks"" SET ""Name"" = @p0, ""Quacks"" = @p1, ""ConcurrencyToken"" = @p2 +WHERE ""Id"" = @p3 +RETURNING ""Computed""; +", + stringBuilder.ToString()); + + protected override void AppendUpdateOperation_if_store_generated_columns_exist_verification( + StringBuilder stringBuilder) + => AssertBaseline( + @"UPDATE ""Ducks"" SET ""Name"" = @p0, ""Quacks"" = @p1, ""ConcurrencyToken"" = @p2 +WHERE ""Id"" = @p3 AND ""ConcurrencyToken"" IS NULL +RETURNING ""Computed""; +", + stringBuilder.ToString()); + + protected override void AppendUpdateOperation_if_store_generated_columns_dont_exist_verification( + StringBuilder stringBuilder) + => AssertBaseline( + @"UPDATE ""Ducks"" SET ""Name"" = @p0, ""Quacks"" = @p1, ""ConcurrencyToken"" = @p2 +WHERE ""Id"" = @p3 +RETURNING 1; +", + stringBuilder.ToString()); + + protected override void AppendUpdateOperation_appends_where_for_concurrency_token_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"UPDATE ""Ducks"" SET ""Name"" = @p0, ""Quacks"" = @p1, ""ConcurrencyToken"" = @p2 +WHERE ""Id"" = @p3 AND ""ConcurrencyToken"" IS NULL +RETURNING 1; +", + stringBuilder.ToString()); + + protected override void AppendDeleteOperation_creates_full_delete_command_text_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"DELETE FROM ""Ducks"" +WHERE ""Id"" = @p0 +RETURNING 1; +", + stringBuilder.ToString()); + + protected override void AppendDeleteOperation_creates_full_delete_command_text_with_concurrency_check_verification(StringBuilder stringBuilder) + => AssertBaseline( + @"DELETE FROM ""Ducks"" +WHERE ""Id"" = @p0 AND ""ConcurrencyToken"" IS NULL +RETURNING 1; ", stringBuilder.ToString()); diff --git a/test/EFCore.Sqlite.FunctionalTests/Update/StoreValueGenerationSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Update/StoreValueGenerationSqliteTest.cs index 3aa88b271a9..c4ece3530f3 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Update/StoreValueGenerationSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Update/StoreValueGenerationSqliteTest.cs @@ -68,10 +68,8 @@ public override async Task Modify_with_generated_values(bool async) @p0='1000' UPDATE ""WithSomeDatabaseGenerated"" SET ""Data2"" = @p0 -WHERE ""Id"" = @p1; -SELECT ""Data1"" -FROM ""WithSomeDatabaseGenerated"" -WHERE changes() = 1 AND ""Id"" = @p1;"); +WHERE ""Id"" = @p1 +RETURNING ""Data1"";"); } public override async Task Modify_with_no_generated_values(bool async) @@ -84,8 +82,8 @@ public override async Task Modify_with_no_generated_values(bool async) @p1='1000' UPDATE ""WithNoDatabaseGenerated"" SET ""Data1"" = @p0, ""Data2"" = @p1 -WHERE ""Id"" = @p2; -SELECT changes();"); +WHERE ""Id"" = @p2 +RETURNING 1;"); } public override async Task Delete(bool async) @@ -96,8 +94,8 @@ public override async Task Delete(bool async) @"@p0='1' DELETE FROM ""WithSomeDatabaseGenerated"" -WHERE ""Id"" = @p0; -SELECT changes();"); +WHERE ""Id"" = @p0 +RETURNING 1;"); } #endregion Single operation @@ -165,19 +163,15 @@ public override async Task Modify_Modify_with_same_entity_type_and_generated_val @p0='1000' UPDATE ""WithSomeDatabaseGenerated"" SET ""Data2"" = @p0 -WHERE ""Id"" = @p1; -SELECT ""Data1"" -FROM ""WithSomeDatabaseGenerated"" -WHERE changes() = 1 AND ""Id"" = @p1;", +WHERE ""Id"" = @p1 +RETURNING ""Data1"";", // @"@p1='2' @p0='1001' UPDATE ""WithSomeDatabaseGenerated"" SET ""Data2"" = @p0 -WHERE ""Id"" = @p1; -SELECT ""Data1"" -FROM ""WithSomeDatabaseGenerated"" -WHERE changes() = 1 AND ""Id"" = @p1;"); +WHERE ""Id"" = @p1 +RETURNING ""Data1"";"); } public override async Task Modify_Modify_with_same_entity_type_and_no_generated_values(bool async) @@ -190,16 +184,16 @@ public override async Task Modify_Modify_with_same_entity_type_and_no_generated_ @p1='1000' UPDATE ""WithNoDatabaseGenerated"" SET ""Data1"" = @p0, ""Data2"" = @p1 -WHERE ""Id"" = @p2; -SELECT changes();", +WHERE ""Id"" = @p2 +RETURNING 1;", // @"@p2='2' @p0='1001' @p1='1001' UPDATE ""WithNoDatabaseGenerated"" SET ""Data1"" = @p0, ""Data2"" = @p1 -WHERE ""Id"" = @p2; -SELECT changes();"); +WHERE ""Id"" = @p2 +RETURNING 1;"); } public override async Task Delete_Delete_with_same_entity_type(bool async) @@ -210,14 +204,14 @@ public override async Task Delete_Delete_with_same_entity_type(bool async) @"@p0='1' DELETE FROM ""WithSomeDatabaseGenerated"" -WHERE ""Id"" = @p0; -SELECT changes();", +WHERE ""Id"" = @p0 +RETURNING 1;", // @"@p0='2' DELETE FROM ""WithSomeDatabaseGenerated"" -WHERE ""Id"" = @p0; -SELECT changes();"); +WHERE ""Id"" = @p0 +RETURNING 1;"); } #endregion Two operations with same entity type @@ -285,19 +279,15 @@ public override async Task Modify_Modify_with_different_entity_types_and_generat @p0='1000' UPDATE ""WithSomeDatabaseGenerated"" SET ""Data2"" = @p0 -WHERE ""Id"" = @p1; -SELECT ""Data1"" -FROM ""WithSomeDatabaseGenerated"" -WHERE changes() = 1 AND ""Id"" = @p1;", +WHERE ""Id"" = @p1 +RETURNING ""Data1"";", // @"@p1='2' @p0='1001' UPDATE ""WithSomeDatabaseGenerated2"" SET ""Data2"" = @p0 -WHERE ""Id"" = @p1; -SELECT ""Data1"" -FROM ""WithSomeDatabaseGenerated2"" -WHERE changes() = 1 AND ""Id"" = @p1;"); +WHERE ""Id"" = @p1 +RETURNING ""Data1"";"); } public override async Task Modify_Modify_with_different_entity_types_and_no_generated_values(bool async) @@ -310,16 +300,16 @@ public override async Task Modify_Modify_with_different_entity_types_and_no_gene @p1='1000' UPDATE ""WithNoDatabaseGenerated"" SET ""Data1"" = @p0, ""Data2"" = @p1 -WHERE ""Id"" = @p2; -SELECT changes();", +WHERE ""Id"" = @p2 +RETURNING 1;", // @"@p2='2' @p0='1001' @p1='1001' UPDATE ""WithNoDatabaseGenerated2"" SET ""Data1"" = @p0, ""Data2"" = @p1 -WHERE ""Id"" = @p2; -SELECT changes();"); +WHERE ""Id"" = @p2 +RETURNING 1;"); } public override async Task Delete_Delete_with_different_entity_types(bool async) @@ -330,14 +320,14 @@ public override async Task Delete_Delete_with_different_entity_types(bool async) @"@p0='1' DELETE FROM ""WithSomeDatabaseGenerated"" -WHERE ""Id"" = @p0; -SELECT changes();", +WHERE ""Id"" = @p0 +RETURNING 1;", // @"@p0='2' DELETE FROM ""WithSomeDatabaseGenerated2"" -WHERE ""Id"" = @p0; -SELECT changes();"); +WHERE ""Id"" = @p0 +RETURNING 1;"); } #endregion Two operations with different entity types