Skip to content

Commit befb0f7

Browse files
smitpatelsmitpatel
committed
ExecuteDelete for aggregate root when only owned types are sharing the table
Resolves #28671
1 parent e67a2c9 commit befb0f7

File tree

4 files changed

+176
-3
lines changed

4 files changed

+176
-3
lines changed

‎src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,6 +1013,11 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
10131013
/// <returns>The non query after translation.</returns>
10141014
protected virtual NonQueryExpression? TranslateExecuteDelete(ShapedQueryExpression source)
10151015
{
1016+
if (source.ShaperExpression is IncludeExpression includeExpression)
1017+
{
1018+
source = source.UpdateShaperExpression(PruneOwnedIncludes(includeExpression));
1019+
}
1020+
10161021
if (source.ShaperExpression is not EntityShaperExpression entityShaperExpression)
10171022
{
10181023
AddTranslationErrorDetails(RelationalStrings.ExecuteDeleteOnNonEntityType);
@@ -1048,9 +1053,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
10481053
var selectExpression = (SelectExpression)source.QueryExpression;
10491054
if (IsValidSelectExpressionForExecuteDelete(selectExpression, entityShaperExpression, out var tableExpression))
10501055
{
1051-
if ((mappingStrategy == null && tableExpression.Table.EntityTypeMappings.Count() != 1)
1052-
|| (mappingStrategy == RelationalAnnotationNames.TphMappingStrategy
1053-
&& tableExpression.Table.EntityTypeMappings.Any(e => e.EntityType.GetRootType() != entityType.GetRootType())))
1056+
if (AreNonAggregateEntityTypesInTheTable(entityType.GetRootType(), tableExpression.Table))
10541057
{
10551058
AddTranslationErrorDetails(
10561059
RelationalStrings.ExecuteDeleteOnTableSplitting(tableExpression.Table.SchemaQualifiedName));
@@ -1062,6 +1065,24 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
10621065
selectExpression.ApplyProjection();
10631066

10641067
return new NonQueryExpression(new DeleteExpression(tableExpression, selectExpression));
1068+
1069+
static bool AreNonAggregateEntityTypesInTheTable(IEntityType rootType, ITableBase table)
1070+
{
1071+
foreach (var entityTypeMapping in table.EntityTypeMappings)
1072+
{
1073+
var entityType = entityTypeMapping.EntityType;
1074+
if (entityTypeMapping.IsSharedTablePrincipal == true
1075+
&& entityType != rootType
1076+
|| entityTypeMapping.IsSharedTablePrincipal == false
1077+
&& entityType.GetRootType() != rootType
1078+
&& !entityType.IsOwned())
1079+
{
1080+
return true;
1081+
}
1082+
}
1083+
1084+
return false;
1085+
}
10651086
}
10661087

10671088
// We need to convert to PK predicate
@@ -1092,6 +1113,20 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
10921113
Expression.Quote(Expression.Lambda(predicateBody, entityParameter)));
10931114

10941115
return TranslateExecuteDelete((ShapedQueryExpression)Visit(newSource));
1116+
1117+
static Expression PruneOwnedIncludes(IncludeExpression includeExpression)
1118+
{
1119+
if (includeExpression.Navigation is ISkipNavigation
1120+
|| includeExpression.Navigation is not INavigation navigation
1121+
|| !navigation.ForeignKey.IsOwnership)
1122+
{
1123+
return includeExpression;
1124+
}
1125+
1126+
return includeExpression.EntityExpression is IncludeExpression innerIncludeExpression
1127+
? PruneOwnedIncludes(innerIncludeExpression)
1128+
: includeExpression.EntityExpression;
1129+
}
10951130
}
10961131

10971132
/// <summary>

‎test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,87 @@ public abstract class NonSharedModelBulkUpdatesTestBase : NonSharedModelTestBase
77
{
88
protected override string StoreName => "NonSharedModelBulkUpdatesTests";
99

10+
[ConditionalTheory]
11+
[MemberData(nameof(IsAsyncData))]
12+
public virtual async Task Delete_aggregate_root_when_eager_loaded_owned_collection(bool async)
13+
{
14+
var contextFactory = await InitializeAsync<Context28671>(onModelCreating: mb => mb.Entity<Owner>().Ignore(e => e.OwnedReference));
15+
await AssertDelete(async, contextFactory.CreateContext,
16+
context => context.Set<Owner>(), rowsAffectedCount: 0);
17+
}
18+
19+
[ConditionalTheory]
20+
[MemberData(nameof(IsAsyncData))]
21+
public virtual async Task Delete_aggregate_root_when_table_sharing_with_owned(bool async)
22+
{
23+
var contextFactory = await InitializeAsync<Context28671>();
24+
await AssertDelete(async, contextFactory.CreateContext,
25+
context => context.Set<Owner>(), rowsAffectedCount: 0);
26+
}
27+
28+
[ConditionalTheory]
29+
[MemberData(nameof(IsAsyncData))]
30+
public virtual async Task Delete_aggregate_root_when_table_sharing_with_non_owned_throws(bool async)
31+
{
32+
var contextFactory = await InitializeAsync<Context28671>(
33+
onModelCreating: mb =>
34+
{
35+
mb.Entity<Owner>().HasOne<OtherReference>().WithOne().HasForeignKey<OtherReference>(e => e.Id);
36+
mb.Entity<OtherReference>().ToTable(nameof(Owner));
37+
});
38+
39+
40+
await AssertTranslationFailedWithDetails(
41+
() => AssertDelete(async, contextFactory.CreateContext,
42+
context => context.Set<Owner>(), rowsAffectedCount: 0),
43+
RelationalStrings.ExecuteDeleteOnTableSplitting(nameof(Owner)));
44+
}
45+
46+
protected class Context28671 : DbContext
47+
{
48+
public Context28671(DbContextOptions options)
49+
: base(options)
50+
{
51+
}
52+
53+
protected override void OnModelCreating(ModelBuilder modelBuilder)
54+
{
55+
modelBuilder.Entity<Owner>(
56+
b =>
57+
{
58+
b.OwnsOne(e => e.OwnedReference);
59+
b.OwnsMany(e => e.OwnedCollections);
60+
});
61+
}
62+
}
63+
64+
public class Owner
65+
{
66+
public int Id { get; set; }
67+
public string Title { get; set; }
68+
69+
public OwnedReference OwnedReference { get; set; }
70+
public List<OwnedCollection> OwnedCollections { get; set; }
71+
}
72+
73+
public class OwnedReference
74+
{
75+
public int Number { get; set; }
76+
public string Value { get; set; }
77+
}
78+
79+
public class OwnedCollection
80+
{
81+
public string Value { get; set; }
82+
}
83+
84+
public class OtherReference
85+
{
86+
public int Id { get; set; }
87+
public int Number { get; set; }
88+
public string Title { get; set; }
89+
}
90+
1091
#nullable enable
1192
[ConditionalTheory]
1293
[MemberData(nameof(IsAsyncData))]
@@ -131,6 +212,12 @@ await TestHelpers.ExecuteWithStrategyInTransactionAsync(
131212
}
132213
}
133214

215+
protected static async Task AssertTranslationFailedWithDetails(Func<Task> query, string details)
216+
=> Assert.Contains(
217+
RelationalStrings.NonQueryTranslationFailedWithDetails("", details)[21..],
218+
(await Assert.ThrowsAsync<InvalidOperationException>(query))
219+
.Message);
220+
134221
public void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
135222
=> facade.UseTransaction(transaction.GetDbTransaction());
136223

‎test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,31 @@ public class NonSharedModelBulkUpdatesSqlServerTest : NonSharedModelBulkUpdatesT
1111
public virtual void Check_all_tests_overridden()
1212
=> TestHelpers.AssertAllMethodsOverridden(GetType());
1313

14+
public override async Task Delete_aggregate_root_when_eager_loaded_owned_collection(bool async)
15+
{
16+
await base.Delete_aggregate_root_when_eager_loaded_owned_collection(async);
17+
18+
AssertSql(
19+
@"DELETE FROM [o]
20+
FROM [Owner] AS [o]");
21+
}
22+
23+
public override async Task Delete_aggregate_root_when_table_sharing_with_owned(bool async)
24+
{
25+
await base.Delete_aggregate_root_when_table_sharing_with_owned(async);
26+
27+
AssertSql(
28+
@"DELETE FROM [o]
29+
FROM [Owner] AS [o]");
30+
}
31+
32+
public override async Task Delete_aggregate_root_when_table_sharing_with_non_owned_throws(bool async)
33+
{
34+
await base.Delete_aggregate_root_when_table_sharing_with_non_owned_throws(async);
35+
36+
AssertSql();
37+
}
38+
1439
public override async Task Delete_predicate_based_on_optional_navigation(bool async)
1540
{
1641
await base.Delete_predicate_based_on_optional_navigation(async);

‎test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,29 @@ public class NonSharedModelBulkUpdatesSqliteTest : NonSharedModelBulkUpdatesTest
1111
public virtual void Check_all_tests_overridden()
1212
=> TestHelpers.AssertAllMethodsOverridden(GetType());
1313

14+
public override async Task Delete_aggregate_root_when_eager_loaded_owned_collection(bool async)
15+
{
16+
await base.Delete_aggregate_root_when_eager_loaded_owned_collection(async);
17+
18+
AssertSql(
19+
@"DELETE FROM ""Owner"" AS ""o""");
20+
}
21+
22+
public override async Task Delete_aggregate_root_when_table_sharing_with_owned(bool async)
23+
{
24+
await base.Delete_aggregate_root_when_table_sharing_with_owned(async);
25+
26+
AssertSql(
27+
@"DELETE FROM ""Owner"" AS ""o""");
28+
}
29+
30+
public override async Task Delete_aggregate_root_when_table_sharing_with_non_owned_throws(bool async)
31+
{
32+
await base.Delete_aggregate_root_when_table_sharing_with_non_owned_throws(async);
33+
34+
AssertSql();
35+
}
36+
1437
public override async Task Delete_predicate_based_on_optional_navigation(bool async)
1538
{
1639
await base.Delete_predicate_based_on_optional_navigation(async);
@@ -24,6 +47,9 @@ SELECT 1
2447
WHERE ""b"".""Title"" IS NOT NULL AND (""b"".""Title"" LIKE 'Arthur%') AND ""p0"".""Id"" = ""p"".""Id"")");
2548
}
2649

50+
protected override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
51+
=> base.AddOptions(builder).ConfigureWarnings(wcb => wcb.Log(SqliteEventId.CompositeKeyWithValueGeneration));
52+
2753
private void AssertSql(params string[] expected)
2854
=> TestSqlLoggerFactory.AssertBaseline(expected);
2955

0 commit comments

Comments
 (0)