Prune also non-owned Includes for ExecuteUpdate/Include

Fixes dotnet#30572
roji · Mar 24, 2023 · 8bd1e8a · 8bd1e8a
1 parent 945be53
commit 8bd1e8a
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 7 deletions.
diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs
@@ -1044,7 +1044,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
     {
         if (source.ShaperExpression is IncludeExpression includeExpression)
         {
-            source = source.UpdateShaperExpression(PruneOwnedIncludes(includeExpression));
+            source = source.UpdateShaperExpression(PruneIncludes(includeExpression));
         }
 
         if (source.ShaperExpression is not EntityShaperExpression entityShaperExpression)
@@ -1168,7 +1168,7 @@ static bool AreOtherNonOwnedEntityTypesInTheTable(IEntityType rootType, ITableBa
         // ExecuteUpdate's lambdas. Note that we don't currently support updates across tables.
         if (source.ShaperExpression is IncludeExpression includeExpression)
         {
-            source = source.UpdateShaperExpression(PruneOwnedIncludes(includeExpression));
+            source = source.UpdateShaperExpression(PruneIncludes(includeExpression));
         }
 
         var propertyValueLambdaExpressions = new List<(LambdaExpression, Expression)>();
@@ -1614,17 +1614,15 @@ private Expression RemapLambdaBody(ShapedQueryExpression shapedQueryExpression,
     private Expression ExpandSharedTypeEntities(SelectExpression selectExpression, Expression lambdaBody)
         => _sharedTypeEntityExpandingExpressionVisitor.Expand(selectExpression, lambdaBody);
 
-    private static Expression PruneOwnedIncludes(IncludeExpression includeExpression)
+    private static Expression PruneIncludes(IncludeExpression includeExpression)
     {
-        if (includeExpression.Navigation is ISkipNavigation
-            || includeExpression.Navigation is not INavigation navigation
-            || !navigation.ForeignKey.IsOwnership)
+        if (includeExpression.Navigation is ISkipNavigation or not INavigation)
         {
             return includeExpression;
         }
 
         return includeExpression.EntityExpression is IncludeExpression innerIncludeExpression
-            ? PruneOwnedIncludes(innerIncludeExpression)
+            ? PruneIncludes(innerIncludeExpression)
             : includeExpression.EntityExpression;
     }
 

diff --git a/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs
@@ -127,6 +127,40 @@ await AssertUpdate(
             rowsAffectedCount: 0);
     }
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual async Task Delete_entity_with_auto_include(bool async)
+    {
+        var contextFactory = await InitializeAsync<Context30572>();
+        await AssertDelete(async, contextFactory.CreateContext, ss => ss.Set<Context30572_Principal>(), rowsAffectedCount: 0);
+    }
+
+    protected class Context30572 : DbContext
+    {
+        public Context30572(DbContextOptions options)
+            : base(options)
+        {
+        }
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+            => modelBuilder.Entity<Context30572_Principal>().Navigation(o => o.Dependent).AutoInclude();
+    }
+
+    public class Context30572_Principal
+    {
+        public int Id { get; set; }
+        public string? Title { get; set; }
+
+        public Context30572_Dependent? Dependent { get; set; }
+    }
+
+    public class Context30572_Dependent
+    {
+        public int Id { get; set; }
+
+        public int Number { get; set; }
+    }
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual async Task Delete_predicate_based_on_optional_navigation(bool async)

diff --git a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs
@@ -65,6 +65,18 @@ FROM [Owner] AS [o]
 """);
     }
 
+    public override async Task Delete_entity_with_auto_include(bool async)
+    {
+        await base.Delete_entity_with_auto_include(async);
+
+        AssertSql(
+"""
+DELETE FROM [c]
+FROM [Context30572_Principal] AS [c]
+LEFT JOIN [Context30572_Dependent] AS [c0] ON [c].[DependentId] = [c0].[Id]
+""");
+    }
+
     public override async Task Delete_predicate_based_on_optional_navigation(bool async)
     {
         await base.Delete_predicate_based_on_optional_navigation(async);

diff --git a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs
@@ -61,6 +61,9 @@ public override async Task Update_non_owned_property_on_entity_with_owned2(bool
 """);
     }
 
+    public override Task Delete_entity_with_auto_include(bool async)
+        => Assert.ThrowsAsync<ArgumentOutOfRangeException>(() => base.Delete_entity_with_auto_include(async));
+
     public override async Task Delete_predicate_based_on_optional_navigation(bool async)
     {
         await base.Delete_predicate_based_on_optional_navigation(async);