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)