From 43f34bf5298c70761a32716633b3b1d85562137f Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Fri, 13 Nov 2020 10:06:14 +0200 Subject: [PATCH] Disable savepoints on SQL Server when MARS is enabled Fixes #23269 --- All.sln.DotSettings | 1 + .../Internal/SqlServerLoggingDefinitions.cs | 8 ++++ .../Diagnostics/SqlServerEventId.cs | 12 ++++++ .../Internal/SqlServerLoggerExtensions.cs | 24 ++++++++++++ .../Properties/SqlServerStrings.Designer.cs | 24 ++++++++++++ .../Properties/SqlServerStrings.resx | 4 ++ .../Storage/Internal/SqlServerTransaction.cs | 19 ++++++++++ .../TransactionSqlServerTest.cs | 38 +++++++++++++++++++ 8 files changed, 130 insertions(+) diff --git a/All.sln.DotSettings b/All.sln.DotSettings index 10b8b9be4a6..402a497c2d1 100644 --- a/All.sln.DotSettings +++ b/All.sln.DotSettings @@ -136,6 +136,7 @@ Licensed under the Apache License, Version 2.0. See License.txt in the project r <Policy Inspect="True" Prefix="" Suffix="" Style="aa_bb" /> True FK + MARS $object$_On$event$ <Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /> <Policy><Descriptor Staticness="Static, Instance" AccessRightKinds="Public" Description="Test Methods"><ElementKinds><Kind Name="TEST_MEMBER" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="Aa_bb" /></Policy> diff --git a/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs b/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs index 2cea968ff22..f9e155987fa 100644 --- a/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs +++ b/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs @@ -149,6 +149,14 @@ public class SqlServerLoggingDefinitions : RelationalLoggingDefinitions /// public EventDefinitionBase LogReflexiveConstraintIgnored; + /// + /// 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 EventDefinitionBase LogSavepointsDisabledBecauseOfMARS; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs b/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs index 702e7bd9895..5f2c1adc72e 100644 --- a/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs +++ b/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs @@ -28,6 +28,7 @@ private enum Id ByteIdentityColumnWarning, ConflictingValueGenerationStrategiesWarning, DecimalTypeKeyWarning, + SavepointsDisabledBecauseOfMARS, // Scaffolding events ColumnFound = CoreEventId.ProviderDesignBaseId, @@ -211,5 +212,16 @@ private static EventId MakeScaffoldingId(Id id) /// This event is in the category. /// public static readonly EventId ReflexiveConstraintIgnored = MakeScaffoldingId(Id.ReflexiveConstraintIgnored); + + /// + /// + /// Savepoints have been disabled when saving changes with an external transaction, because Multiple Active Result Sets is + /// enabled. + /// + /// + /// This event is in the category. + /// + /// + public static readonly EventId SavepointsDisabledBecauseOfMARS = MakeValidationId(Id.SavepointsDisabledBecauseOfMARS); } } diff --git a/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs b/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs index 1b3fa22ed4e..983a2600fd3 100644 --- a/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs +++ b/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs @@ -510,5 +510,29 @@ public static void ReflexiveConstraintIgnored( // No DiagnosticsSource events because these are purely design-time messages } + + /// + /// Logs for the event. + /// + /// The diagnostics logger to use. + public static void SavepointsDisabledBecauseOfMARS( + [NotNull] this IDiagnosticsLogger diagnostics) + { + var definition = SqlServerResources.LogSavepointsDisabledBecauseOfMARS(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new EventData( + definition, + (d, p) => ((EventDefinition)d).GenerateMessage()); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } } } diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs index 365da24ea07..8cbf943be14 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -660,5 +660,29 @@ public static EventDefinition LogReflexiveConstraintIgnored([Not return (EventDefinition)definition; } + + /// + /// Savepoints are disabled because Multiple Active Result Sets is enabled. When saving changes, the transaction will not be rolled back if error occurs, and may be left in an unknown state. See https://docs.microsoft.com/en-us/ef/core/saving/transactions#savepoints for more information. To identify the code which triggers this warning, call 'ConfigureWarnings(w => w.Throw(RelationalEventId.SavepointsDisabledBecauseOfMARS))'. + /// + public static EventDefinition LogSavepointsDisabledBecauseOfMARS([NotNull] IDiagnosticsLogger logger) + { + var definition = ((Diagnostics.Internal.SqlServerLoggingDefinitions)logger.Definitions).LogSavepointsDisabledBecauseOfMARS; + if (definition == null) + { + definition = LazyInitializer.EnsureInitialized( + ref ((Diagnostics.Internal.SqlServerLoggingDefinitions)logger.Definitions).LogSavepointsDisabledBecauseOfMARS, + () => new EventDefinition( + logger.Options, + SqlServerEventId.SavepointsDisabledBecauseOfMARS, + LogLevel.Warning, + "SqlServerEventId.SavepointsDisabledBecauseOfMARS", + level => LoggerMessage.Define( + level, + SqlServerEventId.SavepointsDisabledBecauseOfMARS, + _resourceManager.GetString("LogSavepointsDisabledBecauseOfMARS")))); + } + + return (EventDefinition)definition; + } } } diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx index 2b60ac5ec04..3e774889148 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -249,6 +249,10 @@ Skipping foreign key '{foreignKeyName}' on table '{tableName}' since all of its columns reference themselves. Debug SqlServerEventId.ReflexiveConstraintIgnored string string + + Savepoints are disabled because Multiple Active Result Sets is enabled. When saving changes, the transaction will not be rolled back if error occurs, and may be left in an unknown state. See https://docs.microsoft.com/en-us/ef/core/saving/transactions#savepoints for more information. To identify the code which triggers this warning, call 'ConfigureWarnings(w => w.Throw(RelationalEventId.SavepointsDisabledBecauseOfMARS))'. + Warning SqlServerEventId.SavepointsDisabledBecauseOfMARS + The properties {properties} are configured to use 'Identity' value generator and are mapped to the same table '{table}'. Only one column per table can be configured as 'Identity'. Call 'ValueGeneratedNever' for properties that should not use 'Identity'. diff --git a/src/EFCore.SqlServer/Storage/Internal/SqlServerTransaction.cs b/src/EFCore.SqlServer/Storage/Internal/SqlServerTransaction.cs index 56ca777fa36..81fa31b5873 100644 --- a/src/EFCore.SqlServer/Storage/Internal/SqlServerTransaction.cs +++ b/src/EFCore.SqlServer/Storage/Internal/SqlServerTransaction.cs @@ -7,6 +7,7 @@ using System.Threading.Tasks; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.SqlServer.Internal; using Microsoft.EntityFrameworkCore.Storage; namespace Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal @@ -43,6 +44,24 @@ protected override string GetCreateSavepointSql(string name) protected override string GetRollbackToSavepointSql(string name) => "ROLLBACK TRANSACTION " + name; + /// + public override bool SupportsSavepoints + { + get + { + if (Connection is ISqlServerConnection sqlServerConnection && sqlServerConnection.IsMultipleActiveResultSetsEnabled) + { + Logger.SavepointsDisabledBecauseOfMARS(); + + return false; + } + + return true; + } + } + + // SQL Server doesn't support releasing savepoints. Override to do nothing. + /// public override void ReleaseSavepoint(string name) { } diff --git a/test/EFCore.SqlServer.FunctionalTests/TransactionSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/TransactionSqlServerTest.cs index fce8f6710fe..480fde163d1 100644 --- a/test/EFCore.SqlServer.FunctionalTests/TransactionSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/TransactionSqlServerTest.cs @@ -1,9 +1,15 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Threading.Tasks; +using Microsoft.Data.SqlClient; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal; using Microsoft.EntityFrameworkCore.TestUtilities; +using Xunit; + +// ReSharper disable MethodHasAsyncOverload namespace Microsoft.EntityFrameworkCore { @@ -14,6 +20,37 @@ public TransactionSqlServerTest(TransactionSqlServerFixture fixture) { } + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] + public virtual async Task Savepoints_are_disabled_with_MARS(bool async) + { + await using var context = CreateContext(); + + context.Database.SetDbConnection( + new SqlConnection(TestStore.ConnectionString + ";MultipleActiveResultSets=True")); + + var transaction = await context.Database.BeginTransactionAsync(); + + var orderId = 300; + foreach (var _ in context.Set()) + { + context.Add(new TransactionOrder { Id = orderId++, Name = "Order " + orderId }); + if (async) + { + await context.SaveChangesAsync(); + } + else + { + context.SaveChanges(); + } + } + + await transaction.CommitAsync(); + + Assert.Contains(Fixture.ListLoggerFactory.Log, t => t.Id == SqlServerEventId.SavepointsDisabledBecauseOfMARS); + } + protected override bool SnapshotSupported => true; @@ -60,6 +97,7 @@ public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder build new SqlServerDbContextOptionsBuilder( base.AddOptions(builder)) .ExecutionStrategy(c => new SqlServerExecutionStrategy(c)); + builder.ConfigureWarnings(b => b.Log(SqlServerEventId.SavepointsDisabledBecauseOfMARS)); return builder; } }