Don't skip creating cascade delete on SQL Server when types are base-…

…linking across tables

E.g. for TPT.

Fixes #28532 (Cascade delete behavior for FK between PKs in TPT)

Note this is a breaking change--creating a cascade delete for the TPT relationship could cause a cycle with another existing cascade relationship. This is what happened with the GearsOfWar model.

src/EFCore.SqlServer/Metadata/Conventions/SqlServerOnDeleteConvention.cs

@@ -57,7 +57,8 @@ protected override DeleteBehavior GetTargetDeleteBehavior(IConventionForeignKey
             return deleteBehavior;
         }
 
-        if (foreignKey.IsBaseLinking())
+        if (foreignKey.IsBaseLinking()
+            && IsMappedToSameTable(foreignKey.DeclaringEntityType, foreignKey.PrincipalEntityType))
         {
             return DeleteBehavior.ClientCascade;
         }
@@ -104,23 +105,23 @@ protected override DeleteBehavior GetTargetDeleteBehavior(IConventionForeignKey
         DeleteBehavior DefaultDeleteBehavior(IConventionSkipNavigation conventionSkipNavigation)
             => conventionSkipNavigation.ForeignKey!.IsRequired ? DeleteBehavior.Cascade : DeleteBehavior.ClientSetNull;
 
-        bool IsMappedToSameTable(IConventionEntityType entityType1, IConventionEntityType entityType2)
-        {
-            var tableName1 = entityType1.GetTableName();
-            var tableName2 = entityType2.GetTableName();
-
-            return tableName1 != null
-                && tableName2 != null
-                && tableName1 == tableName2
-                && entityType1.GetSchema() == entityType2.GetSchema();
-        }
-
         bool IsFirstSkipNavigation(IConventionSkipNavigation navigation)
             => navigation.DeclaringEntityType != navigation.TargetEntityType
                 ? string.Compare(navigation.DeclaringEntityType.Name, navigation.TargetEntityType.Name, StringComparison.Ordinal) < 0
                 : string.Compare(navigation.Name, navigation.Inverse!.Name, StringComparison.Ordinal) < 0;
     }
 
+    private bool IsMappedToSameTable(IConventionEntityType entityType1, IConventionEntityType entityType2)
+    {
+        var tableName1 = entityType1.GetTableName();
+        var tableName2 = entityType2.GetTableName();
+
+        return tableName1 != null
+            && tableName2 != null
+            && tableName1 == tableName2
+            && entityType1.GetSchema() == entityType2.GetSchema();
+    }
+
     /// <inheritdoc />
     public virtual void ProcessEntityTypeAnnotationChanged(
         IConventionEntityTypeBuilder entityTypeBuilder,
@@ -133,6 +134,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);
+                }
+            }
         }
     }
 }

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 =>

test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs

@@ -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);
 

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;
 

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);
     }
 }

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);
 

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)

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();
                     });
             },

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)