Take into account store-generated values when ordering update commands

Fixes #33023
dotnet · Sep 6, 2024 · a1ecef5 · a1ecef5
1 parent f2eb7db
commit a1ecef5
Show file tree

Hide file tree

Showing 9 changed files with 83 additions and 13 deletions.
diff --git 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<IColumn> 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
@@ -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
@@ -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<ProductCategory> ProductCategories { get; set; } = null!;
 }
diff --git a/test/EFCore.Specification.Tests/TestModels/UpdatesModel/UpdatesContext.cs b/test/EFCore.Specification.Tests/TestModels/UpdatesModel/UpdatesContext.cs
@@ -29,15 +29,17 @@ public static Task SeedAsync(UpdatesContext context)
                 Id = productId1,
                 Name = "Apple Cider",
                 Price = 1.49M,
-                DependentId = 778
+                DependentId = 778,
+                IsPrimary = true
             });
         context.Add(
             new Product
             {
                 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
@@ -935,6 +935,10 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                 .HasForeignKey(e => e.DependentId)
                 .HasPrincipalKey(e => e.PrincipalId);
 
+            modelBuilder.Entity<Product>()
+                .HasIndex(e => new { e.Name, e.IsPrimaryNormalized })
+                .IsUnique();
+
             modelBuilder.Entity<Person>(
                 pb =>
                 {

diff --git 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
@@ -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
@@ -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
@@ -258,6 +258,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                 .Property(p => p.Id).HasDefaultValueSql("NEWID()");
 
             modelBuilder.Entity<Product>().HasIndex(p => new { p.Name, p.Price }).HasFilter("Name IS NOT NULL");
+
+            modelBuilder.Entity<Product>()
+                .HasIndex(e => new { e.Name, e.IsPrimaryNormalized })
+                .IsUnique()
+                .HasFilter(null);
+
+            modelBuilder.Entity<Product>()
+                .Property(e => e.IsPrimaryNormalized)
+                .HasComputedColumnSql($"IIF(IsPrimary = 1, CONVERT(bit, 1), NULL)", stored: true);
         }
 
         public virtual async Task ResetIdentity()