From 748c596a83a19e0ce6e32c3de1b5971ccfe78afe Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Thu, 6 Oct 2022 09:47:18 -0700 Subject: [PATCH] Validate that if any fragments use table sharing then the main fragments are mapped to the same table Fixes #29104 --- .../RelationalModelValidator.cs | 21 ++- .../Properties/RelationalStrings.Designer.cs | 8 +- .../Properties/RelationalStrings.resx | 2 +- .../Query/EntitySplittingQueryTestBase.cs | 167 +----------------- .../RelationalModelValidatorTest.cs | 40 ++++- .../EntitySplittingQuerySqlServerTest.cs | 30 +--- .../Query/EntitySplittingQuerySqliteTest.cs | 6 +- 7 files changed, 68 insertions(+), 206 deletions(-) diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index 1b60b793e6c..c114e0553e7 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -2143,16 +2143,19 @@ protected virtual void ValidateMappingFragments( entityType.DisplayName(), fragment.StoreObject.DisplayName())); } - var unmatchedLeafRowInternalFk = entityType.FindRowInternalForeignKeys(fragment.StoreObject) - .FirstOrDefault( - fk => entityType.FindRowInternalForeignKeys(mainStoreObject.Value) - .All(mainFk => mainFk.PrincipalEntityType != fk.PrincipalEntityType)); - if (unmatchedLeafRowInternalFk != null) + foreach (var foreignKey in entityType.FindRowInternalForeignKeys(fragment.StoreObject)) { - throw new InvalidOperationException( - RelationalStrings.EntitySplittingUnmatchedMainTableSplitting( - entityType.DisplayName(), fragment.StoreObject.DisplayName(), - unmatchedLeafRowInternalFk.PrincipalEntityType.DisplayName())); + var principalMainFragment = StoreObjectIdentifier.Create( + foreignKey.PrincipalEntityType, fragment.StoreObject.StoreObjectType)!.Value; + if (principalMainFragment != mainStoreObject) + { + throw new InvalidOperationException( + RelationalStrings.EntitySplittingUnmatchedMainTableSplitting( + entityType.DisplayName(), + fragment.StoreObject.DisplayName(), + foreignKey.PrincipalEntityType.DisplayName(), + principalMainFragment.DisplayName())); + } } var propertiesFound = false; diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index b288e497733..c412f98f98d 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -670,12 +670,12 @@ public static string EntitySplittingUnmappedMainFragment(object? entityType, obj entityType, storeObject, storeObjectType); /// - /// Entity type '{entityType}' has a split mapping for '{storeObject}' that shares the table with '{principalEntityType}', but the main mappings of these types do not share a table. Map the split fragments of '{entityType}' to non-shared tables or map the main fragment to a table that '{principalEntityType}' is also mapped to. + /// Entity type '{entityType}' has a split mapping for '{storeObject}' that is shared with the entity type '{principalEntityType}', but the main mappings of these types do not share a table. Map the split fragments of '{entityType}' to non-shared tables or map the main fragment to '{principalStoreObject}'. /// - public static string EntitySplittingUnmatchedMainTableSplitting(object? entityType, object? storeObject, object? principalEntityType) + public static string EntitySplittingUnmatchedMainTableSplitting(object? entityType, object? storeObject, object? principalEntityType, object? principalStoreObject) => string.Format( - GetString("EntitySplittingUnmatchedMainTableSplitting", nameof(entityType), nameof(storeObject), nameof(principalEntityType)), - entityType, storeObject, principalEntityType); + GetString("EntitySplittingUnmatchedMainTableSplitting", nameof(entityType), nameof(storeObject), nameof(principalEntityType), nameof(principalStoreObject)), + entityType, storeObject, principalEntityType, principalStoreObject); /// /// An error occurred while reading a database value for property '{entityType}.{property}'. See the inner exception for more information. diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index b1df32164cf..2b06d916915 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -365,7 +365,7 @@ Entity type '{entityType}' has a split mapping for '{storeObject}', but it doesn't have a main mapping of the same type. Map '{entityType}' to '{storeObjectType}'. - Entity type '{entityType}' has a split mapping for '{storeObject}' that shares the table with '{principalEntityType}', but the main mappings of these types do not share a table. Map the split fragments of '{entityType}' to non-shared tables or map the main fragment to a table that '{principalEntityType}' is also mapped to. + Entity type '{entityType}' has a split mapping for '{storeObject}' that is shared with the entity type '{principalEntityType}', but the main mappings of these types do not share a table. Map the split fragments of '{entityType}' to non-shared tables or map the main fragment to '{principalStoreObject}'. An error occurred while reading a database value for property '{entityType}.{property}'. See the inner exception for more information. diff --git a/test/EFCore.Relational.Specification.Tests/Query/EntitySplittingQueryTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/EntitySplittingQueryTestBase.cs index 9fff6ca10b2..ae15cb0a320 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/EntitySplittingQueryTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/EntitySplittingQueryTestBase.cs @@ -904,116 +904,6 @@ await AssertQuery( entryCount: 10); } - [ConditionalTheory(Skip = "Issue#29104")] - [MemberData(nameof(IsAsyncData))] - public virtual async Task Split_entity_owning_a_split_reference_with_table_sharing_2(bool async) - { - await InitializeContextFactoryAsync( - mb => - { - mb.Entity( - b => - { - b.ToTable("SplitEntityOnePart1"); - - b.SplitToTable("SplitEntityOnePart2", - tb => - { - tb.Property(e => e.IntValue3); - tb.Property(e => e.StringValue3); - }); - - b.SplitToTable("SplitEntityOnePart3", - tb => - { - tb.Property(e => e.IntValue4); - tb.Property(e => e.StringValue4); - }); - - b.OwnsOne(e => e.OwnedReference, - o => - { - o.ToTable("SplitEntityOnePart2"); - - o.SplitToTable("SplitEntityOnePart3", - t => - { - t.Property(e => e.OwnedIntValue3); - t.Property(e => e.OwnedStringValue3); - }); - - o.SplitToTable("SplitEntityOnePart1", - t => - { - t.Property(e => e.OwnedIntValue4); - t.Property(e => e.OwnedStringValue4); - }); - }); - }); - }); - - await AssertQuery( - async, - ss => ss.Set(), - elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude(i => i.OwnedReference)), - entryCount: 10); - } - - [ConditionalTheory(Skip = "Issue#29104")] - [MemberData(nameof(IsAsyncData))] - public virtual async Task Split_entity_owning_a_split_reference_with_table_sharing_3(bool async) - { - await InitializeContextFactoryAsync( - mb => - { - mb.Entity( - b => - { - b.ToTable("SplitEntityOnePart1"); - - b.SplitToTable("SplitEntityOnePart2", - tb => - { - tb.Property(e => e.IntValue3); - tb.Property(e => e.StringValue3); - }); - - b.SplitToTable("SplitEntityOnePart3", - tb => - { - tb.Property(e => e.IntValue4); - tb.Property(e => e.StringValue4); - }); - - b.OwnsOne(e => e.OwnedReference, - o => - { - o.ToTable("SplitEntityOnePart3"); - - o.SplitToTable("SplitEntityOnePart2", - t => - { - t.Property(e => e.OwnedIntValue3); - t.Property(e => e.OwnedStringValue3); - }); - - o.SplitToTable("SplitEntityOnePart1", - t => - { - t.Property(e => e.OwnedIntValue4); - t.Property(e => e.OwnedStringValue4); - }); - }); - }); - }); - - await AssertQuery( - async, - ss => ss.Set(), - elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude(i => i.OwnedReference)), - entryCount: 10); - } - [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual async Task Split_entity_owning_a_split_reference_with_table_sharing_4(bool async) @@ -1069,61 +959,6 @@ await AssertQuery( entryCount: 10); } - [ConditionalTheory(Skip = "Issue#29104")] - [MemberData(nameof(IsAsyncData))] - public virtual async Task Split_entity_owning_a_split_reference_with_table_sharing_5(bool async) - { - await InitializeContextFactoryAsync( - mb => - { - mb.Entity( - b => - { - b.ToTable("SplitEntityOnePart1"); - - b.SplitToTable("SplitEntityOnePart2", - tb => - { - tb.Property(e => e.IntValue3); - tb.Property(e => e.StringValue3); - }); - - b.SplitToTable("SplitEntityOnePart3", - tb => - { - tb.Property(e => e.IntValue4); - tb.Property(e => e.StringValue4); - }); - - b.OwnsOne(e => e.OwnedReference, - o => - { - o.ToTable("SplitEntityOnePart2"); - - o.SplitToTable("SplitEntityOnePart1", - t => - { - t.Property(e => e.OwnedIntValue3); - t.Property(e => e.OwnedStringValue3); - }); - - o.SplitToTable("OwnedReferencePart3", - t => - { - t.Property(e => e.OwnedIntValue4); - t.Property(e => e.OwnedStringValue4); - }); - }); - }); - }); - - await AssertQuery( - async, - ss => ss.Set(), - elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude(i => i.OwnedReference)), - entryCount: 10); - } - [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual async Task Split_entity_owning_a_split_reference_with_table_sharing_6(bool async) @@ -1155,7 +990,7 @@ await InitializeContextFactoryAsync( { o.ToTable("SplitEntityOnePart2"); - o.SplitToTable("SplitEntityOnePart3", + o.SplitToTable("OwnedReferencePart2", t => { t.Property(e => e.OwnedIntValue3); diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index a8c03f04b56..265143cc7e6 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -838,7 +838,45 @@ public void Detects_entity_splitting_with_partial_table_splitting() }); VerifyError( - RelationalStrings.EntitySplittingUnmatchedMainTableSplitting(nameof(OrderDetails), "Order", nameof(Order)), + RelationalStrings.EntitySplittingUnmatchedMainTableSplitting(nameof(OrderDetails), "Order", nameof(Order), "Order"), + modelBuilder); + } + + [ConditionalFact] + public void Detects_entity_splitting_with_reverse_table_splitting() + { + var modelBuilder = CreateConventionModelBuilder(); + + modelBuilder.Entity( + cb => + { + cb.Ignore(c => c.Customer); + + cb.ToTable("Order"); + + cb.SplitToTable( + "OrderDetails", tb => + { + tb.Property(c => c.PartitionId); + }); + + cb.OwnsOne( + c => c.OrderDetails, db => + { + db.ToTable("OrderDetails"); + + db.Property("OtherAddress"); + db.SplitToTable( + "Order", tb => + { + tb.Property("OtherAddress"); + }); + }); + cb.Navigation(c => c.OrderDetails).IsRequired(); + }); + + VerifyError( + RelationalStrings.EntitySplittingUnmatchedMainTableSplitting(nameof(OrderDetails), "Order", nameof(Order), "Order"), modelBuilder); } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/EntitySplittingQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/EntitySplittingQuerySqlServerTest.cs index 76157708975..468b32416f2 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/EntitySplittingQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/EntitySplittingQuerySqlServerTest.cs @@ -279,20 +279,6 @@ FROM [SplitEntityOnePart1] AS [s] INNER JOIN [SplitEntityOnePart2] AS [s1] ON [s].[Id] = [s1].[Id]"); } - public override async Task Split_entity_owning_a_split_reference_with_table_sharing_2(bool async) - { - await base.Split_entity_owning_a_split_reference_with_table_sharing_2(async); - - AssertSql(); - } - - public override async Task Split_entity_owning_a_split_reference_with_table_sharing_3(bool async) - { - await base.Split_entity_owning_a_split_reference_with_table_sharing_3(async); - - AssertSql(); - } - public override async Task Split_entity_owning_a_split_reference_with_table_sharing_4(bool async) { await base.Split_entity_owning_a_split_reference_with_table_sharing_4(async); @@ -305,19 +291,17 @@ FROM [SplitEntityOnePart1] AS [s] LEFT JOIN [OwnedReferencePart3] AS [o] ON [s].[Id] = [o].[EntityOneId]"); } - public override async Task Split_entity_owning_a_split_reference_with_table_sharing_5(bool async) - { - await base.Split_entity_owning_a_split_reference_with_table_sharing_5(async); - - AssertSql(); - } - - [ConditionalTheory(Skip = "Issue29075")] public override async Task Split_entity_owning_a_split_reference_with_table_sharing_6(bool async) { await base.Split_entity_owning_a_split_reference_with_table_sharing_6(async); - AssertSql(); + AssertSql( + @"SELECT [s].[Id], [s].[EntityThreeId], [s].[IntValue1], [s].[IntValue2], [s1].[IntValue3], [s0].[IntValue4], [s].[StringValue1], [s].[StringValue2], [s1].[StringValue3], [s0].[StringValue4], [s1].[Id], [s1].[OwnedReference_Id], [s1].[OwnedReference_OwnedIntValue1], [s1].[OwnedReference_OwnedIntValue2], [o0].[OwnedIntValue3], [o].[OwnedIntValue4], [s1].[OwnedReference_OwnedStringValue1], [s1].[OwnedReference_OwnedStringValue2], [o0].[OwnedStringValue3], [o].[OwnedStringValue4] +FROM [SplitEntityOnePart1] AS [s] +INNER JOIN [SplitEntityOnePart3] AS [s0] ON [s].[Id] = [s0].[Id] +INNER JOIN [SplitEntityOnePart2] AS [s1] ON [s].[Id] = [s1].[Id] +LEFT JOIN [OwnedReferencePart3] AS [o] ON [s1].[Id] = [o].[EntityOneId] +LEFT JOIN [OwnedReferencePart2] AS [o0] ON [s1].[Id] = [o0].[EntityOneId]"); } public override async Task Tph_entity_owning_a_split_reference_on_base_with_table_sharing(bool async) diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/EntitySplittingQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/EntitySplittingQuerySqliteTest.cs index 0ffd39b6493..9f8dcaad9a9 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/EntitySplittingQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/EntitySplittingQuerySqliteTest.cs @@ -81,16 +81,18 @@ LEFT JOIN ( ) AS ""t"" ON ""e"".""Id"" = ""t"".""EntityOneId"" ORDER BY ""e"".""Id"", ""t"".""EntityOneId"""); } + public override async Task Split_entity_owning_a_split_reference_with_table_sharing_6(bool async) { await base.Split_entity_owning_a_split_reference_with_table_sharing_6(async); AssertSql( - @"SELECT ""s"".""Id"", ""s"".""EntityThreeId"", ""s"".""IntValue1"", ""s"".""IntValue2"", ""s1"".""IntValue3"", ""s0"".""IntValue4"", ""s"".""StringValue1"", ""s"".""StringValue2"", ""s1"".""StringValue3"", ""s0"".""StringValue4"", ""s1"".""Id"", ""s1"".""OwnedReference_Id"", ""s1"".""OwnedReference_OwnedIntValue1"", ""s1"".""OwnedReference_OwnedIntValue2"", ""s0"".""OwnedReference_OwnedIntValue3"", ""o"".""OwnedIntValue4"", ""s1"".""OwnedReference_OwnedStringValue1"", ""s1"".""OwnedReference_OwnedStringValue2"", ""s0"".""OwnedReference_OwnedStringValue3"", ""o"".""OwnedStringValue4"" + @"SELECT ""s"".""Id"", ""s"".""EntityThreeId"", ""s"".""IntValue1"", ""s"".""IntValue2"", ""s1"".""IntValue3"", ""s0"".""IntValue4"", ""s"".""StringValue1"", ""s"".""StringValue2"", ""s1"".""StringValue3"", ""s0"".""StringValue4"", ""s1"".""Id"", ""s1"".""OwnedReference_Id"", ""s1"".""OwnedReference_OwnedIntValue1"", ""s1"".""OwnedReference_OwnedIntValue2"", ""o0"".""OwnedIntValue3"", ""o"".""OwnedIntValue4"", ""s1"".""OwnedReference_OwnedStringValue1"", ""s1"".""OwnedReference_OwnedStringValue2"", ""o0"".""OwnedStringValue3"", ""o"".""OwnedStringValue4"" FROM ""SplitEntityOnePart1"" AS ""s"" INNER JOIN ""SplitEntityOnePart3"" AS ""s0"" ON ""s"".""Id"" = ""s0"".""Id"" INNER JOIN ""SplitEntityOnePart2"" AS ""s1"" ON ""s"".""Id"" = ""s1"".""Id"" -LEFT JOIN ""OwnedReferencePart3"" AS ""o"" ON ""s1"".""Id"" = ""o"".""EntityOneId"""); +LEFT JOIN ""OwnedReferencePart3"" AS ""o"" ON ""s1"".""Id"" = ""o"".""EntityOneId"" +LEFT JOIN ""OwnedReferencePart2"" AS ""o0"" ON ""s1"".""Id"" = ""o0"".""EntityOneId"""); } public override async Task Tph_entity_owning_a_split_reference_on_base_without_table_sharing(bool async)