From a1ecef515b49f1d1187fb2f88fd9fbbff037e49e Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Fri, 6 Sep 2024 12:09:00 -0700 Subject: [PATCH] Take into account store-generated values when ordering update commands Fixes #33023 --- .../Update/Internal/CommandBatchPreparer.cs | 18 ++++++-- .../Update/UpdatesRelationalTestBase.cs | 43 ++++++++++++++++++- .../TestModels/UpdatesModel/Product.cs | 4 ++ .../TestModels/UpdatesModel/UpdatesContext.cs | 6 ++- .../Update/UpdatesTestBase.cs | 4 ++ .../Update/UpdatesSqlServerTPCTest.cs | 4 +- .../Update/UpdatesSqlServerTPTTest.cs | 4 +- .../Update/UpdatesSqlServerTest.cs | 4 +- .../Update/UpdatesSqlServerTestBase.cs | 9 ++++ 9 files changed, 83 insertions(+), 13 deletions(-) diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index 23b83ce061e..fdf3c302b19 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -993,18 +993,30 @@ private static bool IsModified(IReadOnlyList columns, IReadOnlyModifica var entry = command.Entries[entryIndex]; var columnMapping = column.FindColumnMapping(entry.EntityType); var property = columnMapping?.Property; - if (property != null - && (property.GetAfterSaveBehavior() == PropertySaveBehavior.Save - || (!property.IsPrimaryKey() && entry.EntityState != EntityState.Modified))) + if (property != null) { switch (entry.EntityState) { case EntityState.Added: currentValue = entry.GetCurrentProviderValue(property); + if (entry.SharedIdentityEntry != null) + { + var sharedProperty = entry.SharedIdentityEntry.EntityType == entry.EntityType + ? property + : column.FindColumnMapping(entry.SharedIdentityEntry.EntityType)?.Property; + + if (sharedProperty != null) + { + originalValue ??= entry.SharedIdentityEntry.GetOriginalProviderValue(sharedProperty); + } + } + break; case EntityState.Deleted: case EntityState.Unchanged: originalValue ??= entry.GetOriginalProviderValue(property); + Check.DebugAssert(entry.SharedIdentityEntry == null, "entry.SharedIdentityEntry != null"); + break; case EntityState.Modified: if (entry.IsModified(property)) diff --git a/test/EFCore.Relational.Specification.Tests/Update/UpdatesRelationalTestBase.cs b/test/EFCore.Relational.Specification.Tests/Update/UpdatesRelationalTestBase.cs index 453ea766d0d..666e030a6a1 100644 --- a/test/EFCore.Relational.Specification.Tests/Update/UpdatesRelationalTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Update/UpdatesRelationalTestBase.cs @@ -175,6 +175,43 @@ public virtual Task Swap_filtered_unique_index_values() }); } + [ConditionalFact] // Issue #33023 + public virtual Task Swap_computed_unique_index_values() + { + var productId1 = new Guid("984ade3c-2f7b-4651-a351-642e92ab7146"); + var productId2 = new Guid("0edc9136-7eed-463b-9b97-bdb9648ab877"); + + return ExecuteWithStrategyInTransactionAsync( + async context => + { + var product1 = (await context.Products.FindAsync(productId1))!; + var product2 = (await context.Products.FindAsync(productId2))!; + + product1.IsPrimary = false; + product2.Name = product1.Name; + product2.IsPrimary = true; + + await context.SaveChangesAsync(); + }, async context => + { + var product1 = (await context.Products.FindAsync(productId1))!; + var product2 = (await context.Products.FindAsync(productId2))!; + + product1.Name = "Apple Cobler"; + product1.IsPrimary = true; + product2.IsPrimary = false; + + await context.SaveChangesAsync(); + }, async context => + { + var product1 = (await context.Products.FindAsync(productId1))!; + var product2 = (await context.Products.FindAsync(productId2))!; + + Assert.True(product1.IsPrimary); + Assert.False(product2.IsPrimary); + }); + } + [ConditionalFact] public virtual Task Update_non_indexed_values() { @@ -197,13 +234,15 @@ public virtual Task Update_non_indexed_values() { Id = productId1, Name = "", - Price = 1.49M + Price = 1.49M, + IsPrimary = true }; var product2 = new Product { Id = productId2, Name = "", - Price = 1.49M + Price = 1.49M, + IsPrimary = false }; context.Attach(product1).Property(p => p.DependentId).IsModified = true; diff --git a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/Product.cs b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/Product.cs index b3bd807684b..352432bee78 100644 --- a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/Product.cs +++ b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/Product.cs @@ -17,5 +17,9 @@ public class Product : ProductBase [ConcurrencyCheck] public decimal Price { get; set; } + public bool IsPrimary { get; set; } + + public bool? IsPrimaryNormalized { get => IsPrimary ? true : null; set { } } + public ICollection ProductCategories { get; set; } = null!; } diff --git a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/UpdatesContext.cs b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/UpdatesContext.cs index 53a54579e9b..7213ae7709c 100644 --- a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/UpdatesContext.cs +++ b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/UpdatesContext.cs @@ -29,7 +29,8 @@ public static Task SeedAsync(UpdatesContext context) Id = productId1, Name = "Apple Cider", Price = 1.49M, - DependentId = 778 + DependentId = 778, + IsPrimary = true }); context.Add( new Product @@ -37,7 +38,8 @@ public static Task SeedAsync(UpdatesContext context) Id = productId2, Name = "Apple Cobler", Price = 2.49M, - DependentId = 778 + DependentId = 778, + IsPrimary = false }); return context.SaveChangesAsync(); diff --git a/test/EFCore.Specification.Tests/Update/UpdatesTestBase.cs b/test/EFCore.Specification.Tests/Update/UpdatesTestBase.cs index 6108506e568..f0c0d83e3d7 100644 --- a/test/EFCore.Specification.Tests/Update/UpdatesTestBase.cs +++ b/test/EFCore.Specification.Tests/Update/UpdatesTestBase.cs @@ -935,6 +935,10 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con .HasForeignKey(e => e.DependentId) .HasPrincipalKey(e => e.PrincipalId); + modelBuilder.Entity() + .HasIndex(e => new { e.Name, e.IsPrimaryNormalized }) + .IsUnique(); + modelBuilder.Entity( pb => { diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTPCTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTPCTest.cs index 3f02486912d..b9be9e4c23a 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTPCTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTPCTest.cs @@ -51,7 +51,7 @@ FROM [SpecialCategory] AS [s] """ @__category_PrincipalId_0='778' (Nullable = true) -SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[Name], [p].[Price] +SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[IsPrimary], [p].[IsPrimaryNormalized], [p].[Name], [p].[Price] FROM [ProductBase] AS [p] WHERE [p].[Discriminator] = N'Product' AND [p].[DependentId] = @__category_PrincipalId_0 """, @@ -81,7 +81,7 @@ FROM [SpecialCategory] AS [s] """ @__category_PrincipalId_0='778' (Nullable = true) -SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[Name], [p].[Price] +SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[IsPrimary], [p].[IsPrimaryNormalized], [p].[Name], [p].[Price] FROM [ProductBase] AS [p] WHERE [p].[Discriminator] = N'Product' AND [p].[DependentId] = @__category_PrincipalId_0 """); diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTPTTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTPTTest.cs index 64f37a9b476..2ffeb1f8853 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTPTTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTPTTest.cs @@ -48,7 +48,7 @@ FROM [Categories] AS [c] """ @__category_PrincipalId_0='778' (Nullable = true) -SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[Name], [p].[Price] +SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[IsPrimary], [p].[IsPrimaryNormalized], [p].[Name], [p].[Price] FROM [ProductBase] AS [p] WHERE [p].[Discriminator] = N'Product' AND [p].[DependentId] = @__category_PrincipalId_0 """, @@ -75,7 +75,7 @@ FROM [Categories] AS [c] """ @__category_PrincipalId_0='778' (Nullable = true) -SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[Name], [p].[Price] +SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[IsPrimary], [p].[IsPrimaryNormalized], [p].[Name], [p].[Price] FROM [ProductBase] AS [p] WHERE [p].[Discriminator] = N'Product' AND [p].[DependentId] = @__category_PrincipalId_0 """); diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTest.cs index f98cae5f0b6..37cdb66be00 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTest.cs @@ -44,7 +44,7 @@ FROM [Categories] AS [c] """ @__category_PrincipalId_0='778' (Nullable = true) -SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[Name], [p].[Price] +SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[IsPrimary], [p].[IsPrimaryNormalized], [p].[Name], [p].[Price] FROM [ProductBase] AS [p] WHERE [p].[Discriminator] = N'Product' AND [p].[DependentId] = @__category_PrincipalId_0 """, @@ -68,7 +68,7 @@ FROM [Categories] AS [c] """ @__category_PrincipalId_0='778' (Nullable = true) -SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[Name], [p].[Price] +SELECT [p].[Id], [p].[Discriminator], [p].[DependentId], [p].[IsPrimary], [p].[IsPrimaryNormalized], [p].[Name], [p].[Price] FROM [ProductBase] AS [p] WHERE [p].[Discriminator] = N'Product' AND [p].[DependentId] = @__category_PrincipalId_0 """); diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTestBase.cs b/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTestBase.cs index 14ab500ba54..1232f975744 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTestBase.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/UpdatesSqlServerTestBase.cs @@ -258,6 +258,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con .Property(p => p.Id).HasDefaultValueSql("NEWID()"); modelBuilder.Entity().HasIndex(p => new { p.Name, p.Price }).HasFilter("Name IS NOT NULL"); + + modelBuilder.Entity() + .HasIndex(e => new { e.Name, e.IsPrimaryNormalized }) + .IsUnique() + .HasFilter(null); + + modelBuilder.Entity() + .Property(e => e.IsPrimaryNormalized) + .HasComputedColumnSql($"IIF(IsPrimary = 1, CONVERT(bit, 1), NULL)", stored: true); } public virtual async Task ResetIdentity()