Don't skip creating cascade delete on SQL Server when types are base-linking across tables (#28965)

ajcvickers · web-flow · commit bf62eaf95aeb · 2022-09-03T00:18:41.000+01:00
diff --git a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOnDeleteConvention.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOnDeleteConvention.cs
@@ -57,11 +57,6 @@ protected override DeleteBehavior GetTargetDeleteBehavior(IConventionForeignKey
             return deleteBehavior;
         }
 
-        if (foreignKey.IsBaseLinking())
-        {
-            return DeleteBehavior.ClientCascade;
-        }
-
         return ProcessSkipNavigations(foreignKey.GetReferencingSkipNavigations()) ?? deleteBehavior;
     }
 
@@ -133,6 +128,15 @@ public virtual void ProcessEntityTypeAnnotationChanged(
             || name == RelationalAnnotationNames.Schema)
         {
             ProcessSkipNavigations(entityTypeBuilder.Metadata.GetDeclaredSkipNavigations());
+
+            foreach (var foreignKey in entityTypeBuilder.Metadata.GetDeclaredForeignKeys())
+            {
+                var deleteBehavior = GetTargetDeleteBehavior(foreignKey);
+                if (foreignKey.DeleteBehavior != deleteBehavior)
+                {
+                    foreignKey.Builder.OnDelete(deleteBehavior);
+                }
+            }
         }
     }
 }
diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs
@@ -538,7 +538,7 @@ public virtual void Entities_are_stored_in_model_snapshot_for_TPT()
                     b.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+BaseEntity"", null)
                         .WithOne()
                         .HasForeignKey(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+DerivedEntity"", ""Id"")
-                        .OnDelete(DeleteBehavior.ClientCascade)
+                        .OnDelete(DeleteBehavior.Cascade)
                         .IsRequired();
                 });"),
             model =>
@@ -606,7 +606,7 @@ public virtual void Entities_are_stored_in_model_snapshot_for_TPT_with_one_exclu
                     b.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+BaseEntity"", null)
                         .WithOne()
                         .HasForeignKey(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+DerivedEntity"", ""Id"")
-                        .OnDelete(DeleteBehavior.ClientCascade)
+                        .OnDelete(DeleteBehavior.Cascade)
                         .IsRequired();
                 });"),
             o =>
@@ -832,7 +832,7 @@ public virtual void Entity_splitting_is_stored_in_snapshot_with_tables()
                     b.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order"", null)
                         .WithOne()
                         .HasForeignKey(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order"", ""Id"")
-                        .OnDelete(DeleteBehavior.ClientCascade)
+                        .OnDelete(DeleteBehavior.Cascade)
                         .IsRequired();
 
                     b.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderDetails"", ""OrderBillingDetails"", b1 =>
@@ -863,7 +863,7 @@ public virtual void Entity_splitting_is_stored_in_snapshot_with_tables()
                             b1.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order.OrderBillingDetails#Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderDetails"", null)
                                 .WithOne()
                                 .HasForeignKey(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order.OrderBillingDetails#Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderDetails"", ""OrderId"")
-                                .OnDelete(DeleteBehavior.ClientCascade)
+                                .OnDelete(DeleteBehavior.Cascade)
                                 .IsRequired();
 
                             b1.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+StreetAddress"", ""StreetAddress"", b2 =>
@@ -912,7 +912,7 @@ public virtual void Entity_splitting_is_stored_in_snapshot_with_tables()
                             b1.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order.OrderShippingDetails#Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderDetails"", null)
                                 .WithOne()
                                 .HasForeignKey(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order.OrderShippingDetails#Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderDetails"", ""OrderId"")
-                                .OnDelete(DeleteBehavior.ClientCascade)
+                                .OnDelete(DeleteBehavior.Cascade)
                                 .IsRequired();
 
                             b1.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+StreetAddress"", ""StreetAddress"", b2 =>
diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs
@@ -1340,7 +1340,7 @@ public static RuntimeForeignKey CreateForeignKey2(RuntimeEntityType declaringEnt
             var runtimeForeignKey = declaringEntityType.AddForeignKey(new[] { declaringEntityType.FindProperty(""PrincipalBaseId"")!, declaringEntityType.FindProperty(""PrincipalBaseAlternateId"")! },
                 principalEntityType.FindKey(new[] { principalEntityType.FindProperty(""PrincipalBaseId"")!, principalEntityType.FindProperty(""PrincipalBaseAlternateId"")! })!,
                 principalEntityType,
-                deleteBehavior: DeleteBehavior.ClientCascade,
+                deleteBehavior: DeleteBehavior.Cascade,
                 unique: true,
                 required: true,
                 requiredDependent: true);
@@ -1681,7 +1681,7 @@ public static RuntimeForeignKey CreateForeignKey1(RuntimeEntityType declaringEnt
             var runtimeForeignKey = declaringEntityType.AddForeignKey(new[] { declaringEntityType.FindProperty(""Id"")!, declaringEntityType.FindProperty(""AlternateId"")! },
                 principalEntityType.FindKey(new[] { principalEntityType.FindProperty(""Id"")!, principalEntityType.FindProperty(""AlternateId"")! })!,
                 principalEntityType,
-                deleteBehavior: DeleteBehavior.ClientCascade,
+                deleteBehavior: DeleteBehavior.Cascade,
                 unique: true,
                 required: true);
 
@@ -1967,7 +1967,7 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType)
                     Assert.True(tptForeignKey.IsUnique);
                     Assert.Null(tptForeignKey.DependentToPrincipal);
                     Assert.Null(tptForeignKey.PrincipalToDependent);
-                    Assert.Equal(DeleteBehavior.ClientCascade, tptForeignKey.DeleteBehavior);
+                    Assert.Equal(DeleteBehavior.Cascade, tptForeignKey.DeleteBehavior);
                     Assert.Equal(principalKey.Properties, tptForeignKey.Properties);
                     Assert.Same(principalKey, tptForeignKey.PrincipalKey);
 
diff --git a/test/EFCore.Relational.Specification.Tests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesTestBase.cs
@@ -34,18 +34,6 @@ public override Task Delete_GroupBy_Where_Select_First_3(bool async)
             RelationalStrings.ExecuteOperationOnTPT("ExecuteDelete", "Animal"),
             () => base.Delete_GroupBy_Where_Select_First_3(async));
 
-    [ConditionalTheory(Skip = "Issue#28532")]
-    public override Task Delete_where_using_hierarchy(bool async)
-    {
-        return base.Delete_where_using_hierarchy(async);
-    }
-
-    [ConditionalTheory(Skip = "Issue#28532")]
-    public override Task Delete_where_using_hierarchy_derived(bool async)
-    {
-        return base.Delete_where_using_hierarchy_derived(async);
-    }
-
     // Keyless entities are mapped as TPH only
     public override Task Update_where_keyless_entity_mapped_to_sql_query(bool async) => Task.CompletedTask;
 
diff --git a/test/EFCore.Relational.Specification.Tests/Query/TPTGearsOfWarQueryRelationalFixture.cs b/test/EFCore.Relational.Specification.Tests/Query/TPTGearsOfWarQueryRelationalFixture.cs
@@ -28,5 +28,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
         modelBuilder.Entity<LocustHorde>().ToTable("LocustHordes");
 
         modelBuilder.Entity<LocustCommander>().ToTable("LocustCommanders");
+
+        modelBuilder.Entity<Squad>()
+            .HasMany(s => s.Members)
+            .WithOne(g => g.Squad)
+            .HasForeignKey(g => g.SquadId)
+            .OnDelete(DeleteBehavior.ClientCascade);
     }
 }
diff --git a/test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs b/test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs
@@ -857,11 +857,13 @@ private static void AssertTables(IRelationalModel model, Mapping mapping)
                 Assert.Equal("FK_SpecialCustomer_Customer_Id", specialCustomerTptFkConstraint.Name);
                 Assert.NotNull(specialCustomerTptFkConstraint.MappedForeignKeys.Single());
                 Assert.Same(customerTable, specialCustomerTptFkConstraint.PrincipalTable);
+                Assert.Equal(ReferentialAction.Cascade, specialCustomerTptFkConstraint.OnDeleteAction);
 
                 var anotherSpecialCustomerFkConstraint = foreignKeys[2];
                 Assert.Equal("FK_SpecialCustomer_SpecialCustomer_AnotherRelatedCustomerId", anotherSpecialCustomerFkConstraint.Name);
                 Assert.NotNull(anotherSpecialCustomerFkConstraint.MappedForeignKeys.Single());
                 Assert.Same(specialCustomerTable, anotherSpecialCustomerFkConstraint.PrincipalTable);
+                Assert.Equal(ReferentialAction.Cascade, specialCustomerTptFkConstraint.OnDeleteAction);
 
                 Assert.Equal(new[] { orderCustomerFkConstraint, specialCustomerTptFkConstraint }, customerTable.ReferencingForeignKeyConstraints);
 
diff --git a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesSqlServerTest.cs
@@ -39,6 +39,9 @@ FROM [Countries] AS [c]
 WHERE (
     SELECT COUNT(*)
     FROM [Animals] AS [a]
+    LEFT JOIN [Birds] AS [b] ON [a].[Id] = [b].[Id]
+    LEFT JOIN [Eagle] AS [e] ON [a].[Id] = [e].[Id]
+    LEFT JOIN [Kiwi] AS [k] ON [a].[Id] = [k].[Id]
     WHERE [a].[CountryId] = 1 AND [c].[Id] = [a].[CountryId] AND [a].[CountryId] > 0) > 0");
     }
 
@@ -52,7 +55,10 @@ FROM [Countries] AS [c]
 WHERE (
     SELECT COUNT(*)
     FROM [Animals] AS [a]
-    WHERE [a].[CountryId] = 1 AND [c].[Id] = [a].[CountryId] AND [a].[Discriminator] = N'Kiwi' AND [a].[CountryId] > 0) > 0");
+    LEFT JOIN [Birds] AS [b] ON [a].[Id] = [b].[Id]
+    LEFT JOIN [Eagle] AS [e] ON [a].[Id] = [e].[Id]
+    LEFT JOIN [Kiwi] AS [k] ON [a].[Id] = [k].[Id]
+    WHERE [a].[CountryId] = 1 AND [c].[Id] = [a].[CountryId] AND [k].[Id] IS NOT NULL AND [a].[CountryId] > 0) > 0");
     }
 
     public override async Task Delete_where_keyless_entity_mapped_to_sql_query(bool async)
diff --git a/test/EFCore.SqlServer.Tests/Migrations/SqlServerModelDifferTest.cs b/test/EFCore.SqlServer.Tests/Migrations/SqlServerModelDifferTest.cs
@@ -1024,7 +1024,7 @@ public void Noop_TPT_with_FKs_and_seed_data()
                         b.HasOne("Animal", null)
                             .WithOne()
                             .HasForeignKey("Cat", "Id")
-                            .OnDelete(DeleteBehavior.ClientCascade)
+                            .OnDelete(DeleteBehavior.Cascade)
                             .IsRequired();
 
                         b.HasOne("Animal", null)
@@ -1038,7 +1038,7 @@ public void Noop_TPT_with_FKs_and_seed_data()
                         b.HasOne("Animal", null)
                             .WithOne()
                             .HasForeignKey("Dog", "Id")
-                            .OnDelete(DeleteBehavior.ClientCascade)
+                            .OnDelete(DeleteBehavior.Cascade)
                             .IsRequired();
 
                         b.HasOne("Animal", null)
@@ -1052,7 +1052,7 @@ public void Noop_TPT_with_FKs_and_seed_data()
                         b.HasOne("Animal", null)
                             .WithOne()
                             .HasForeignKey("Mouse", "Id")
-                            .OnDelete(DeleteBehavior.ClientCascade)
+                            .OnDelete(DeleteBehavior.Cascade)
                             .IsRequired();
                     });
             },
diff --git a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesSqliteTest.cs
@@ -34,25 +34,29 @@ public override async Task Delete_where_using_hierarchy(bool async)
         await base.Delete_where_using_hierarchy(async);
 
         AssertSql(
-            @"DELETE FROM [c]
-FROM [Countries] AS [c]
+            @"DELETE FROM ""Countries"" AS ""c""
 WHERE (
     SELECT COUNT(*)
-    FROM [Animals] AS [a]
-    WHERE [a].[CountryId] = 1 AND [c].[Id] = [a].[CountryId] AND [a].[CountryId] > 0) > 0");
+    FROM ""Animals"" AS ""a""
+    LEFT JOIN ""Birds"" AS ""b"" ON ""a"".""Id"" = ""b"".""Id""
+    LEFT JOIN ""Eagle"" AS ""e"" ON ""a"".""Id"" = ""e"".""Id""
+    LEFT JOIN ""Kiwi"" AS ""k"" ON ""a"".""Id"" = ""k"".""Id""
+    WHERE ""a"".""CountryId"" = 1 AND ""c"".""Id"" = ""a"".""CountryId"" AND ""a"".""CountryId"" > 0) > 0");
     }
 
     public override async Task Delete_where_using_hierarchy_derived(bool async)
     {
         await base.Delete_where_using_hierarchy_derived(async);
 
         AssertSql(
-            @"DELETE FROM [c]
-FROM [Countries] AS [c]
+            @"DELETE FROM ""Countries"" AS ""c""
 WHERE (
     SELECT COUNT(*)
-    FROM [Animals] AS [a]
-    WHERE [a].[CountryId] = 1 AND [c].[Id] = [a].[CountryId] AND [a].[Discriminator] = N'Kiwi' AND [a].[CountryId] > 0) > 0");
+    FROM ""Animals"" AS ""a""
+    LEFT JOIN ""Birds"" AS ""b"" ON ""a"".""Id"" = ""b"".""Id""
+    LEFT JOIN ""Eagle"" AS ""e"" ON ""a"".""Id"" = ""e"".""Id""
+    LEFT JOIN ""Kiwi"" AS ""k"" ON ""a"".""Id"" = ""k"".""Id""
+    WHERE ""a"".""CountryId"" = 1 AND ""c"".""Id"" = ""a"".""CountryId"" AND ""k"".""Id"" IS NOT NULL AND ""a"".""CountryId"" > 0) > 0");
     }
 
     public override async Task Delete_where_keyless_entity_mapped_to_sql_query(bool async)

Original file line number	Diff line number	Diff line change
`@@ -57,11 +57,6 @@ protected override DeleteBehavior GetTargetDeleteBehavior(IConventionForeignKey`
`57`	`57`	`return deleteBehavior;`
`58`	`58`	`}`
`59`	`59`
`60`		`- if (foreignKey.IsBaseLinking())`
`61`		`- {`
`62`		`- return DeleteBehavior.ClientCascade;`
`63`		`- }`
`64`		`-`
`65`	`60`	`return ProcessSkipNavigations(foreignKey.GetReferencingSkipNavigations()) ?? deleteBehavior;`
`66`	`61`	`}`
`67`	`62`
`@@ -133,6 +128,15 @@ public virtual void ProcessEntityTypeAnnotationChanged(`
`133`	`128`	`\|\| name == RelationalAnnotationNames.Schema)`
`134`	`129`	`{`
`135`	`130`	`ProcessSkipNavigations(entityTypeBuilder.Metadata.GetDeclaredSkipNavigations());`
	`131`	`+`
	`132`	`+ foreach (var foreignKey in entityTypeBuilder.Metadata.GetDeclaredForeignKeys())`
	`133`	`+ {`
	`134`	`+ var deleteBehavior = GetTargetDeleteBehavior(foreignKey);`
	`135`	`+ if (foreignKey.DeleteBehavior != deleteBehavior)`
	`136`	`+ {`
	`137`	`+ foreignKey.Builder.OnDelete(deleteBehavior);`
	`138`	`+ }`
	`139`	`+ }`
`136`	`140`	`}`
`137`	`141`	`}`
`138`	`142`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,5 +28,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con`
`28`	`28`	`modelBuilder.Entity<LocustHorde>().ToTable("LocustHordes");`
`29`	`29`
`30`	`30`	`modelBuilder.Entity<LocustCommander>().ToTable("LocustCommanders");`
	`31`	`+`
	`32`	`+ modelBuilder.Entity<Squad>()`
	`33`	`+ .HasMany(s => s.Members)`
	`34`	`+ .WithOne(g => g.Squad)`
	`35`	`+ .HasForeignKey(g => g.SquadId)`
	`36`	`+ .OnDelete(DeleteBehavior.ClientCascade);`
`31`	`37`	`}`
`32`	`38`	`}`