From ace49d3340ecf75335f0ff8704b04eece79cdc3d Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Sat, 22 Jun 2024 09:47:27 +0200 Subject: [PATCH] Report cross-document join exception on Cosmos And some test cleanup Closes #33969 --- .../Properties/CosmosStrings.Designer.cs | 6 + .../Properties/CosmosStrings.resx | 3 + ...yableMethodTranslatingExpressionVisitor.cs | 10 +- .../Query/OwnedQueryCosmosTest.cs | 150 +++++++++++++----- .../Query/OwnedQueryTestBase.cs | 21 +++ .../Query/OwnedQuerySqlServerTest.cs | 36 +++++ 6 files changed, 182 insertions(+), 44 deletions(-) diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs index 67c08640c24..75f7b0ff92e 100644 --- a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs +++ b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs @@ -63,6 +63,12 @@ public static string ContainerContainingPropertyConflict(object? entityType, obj public static string CosmosNotInUse => GetString("CosmosNotInUse"); + /// + /// Joins across documents aren't supported in Cosmos; consider modeling your data differently so that related data is in the same document. Alternatively, perform two separate queries to query the two documents. + /// + public static string CrossDocumentJoinNotSupported + => GetString("CrossDocumentJoinNotSupported"); + /// /// The default time to live was configured to '{ttl1}' on '{entityType1}', but on '{entityType2}' it was configured to '{ttl2}'. All entity types mapped to the same container '{container}' must be configured with the same default time to live. /// diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.resx b/src/EFCore.Cosmos/Properties/CosmosStrings.resx index 10552b282e4..573504b97c6 100644 --- a/src/EFCore.Cosmos/Properties/CosmosStrings.resx +++ b/src/EFCore.Cosmos/Properties/CosmosStrings.resx @@ -135,6 +135,9 @@ Cosmos-specific methods can only be used when the context is using the Cosmos provider. + + Joins across documents aren't supported in Cosmos; consider modeling your data differently so that related data is in the same document. Alternatively, perform two separate queries to query the two documents. + The default time to live was configured to '{ttl1}' on '{entityType1}', but on '{entityType2}' it was configured to '{ttl2}'. All entity types mapped to the same container '{container}' must be configured with the same default time to live. diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs index 1aba18a409f..108f4762ad1 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs @@ -762,7 +762,10 @@ protected override ShapedQueryExpression TranslateDistinct(ShapedQueryExpression LambdaExpression outerKeySelector, LambdaExpression innerKeySelector, LambdaExpression resultSelector) - => null; + { + AddTranslationErrorDetails(CosmosStrings.CrossDocumentJoinNotSupported); + return null; + } /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -807,7 +810,10 @@ protected override ShapedQueryExpression TranslateDistinct(ShapedQueryExpression LambdaExpression outerKeySelector, LambdaExpression innerKeySelector, LambdaExpression resultSelector) - => null; + { + AddTranslationErrorDetails(CosmosStrings.CrossDocumentJoinNotSupported); + return null; + } /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs index d65433ac39b..0e372e50990 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs @@ -17,9 +17,11 @@ public OwnedQueryCosmosTest(OwnedQueryCosmosFixture fixture, ITestOutputHelper t Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper); } - // TODO: Fake LeftJoin, #33969 + // Fink.Barton is a non-owned navigation, cross-document join public override Task Query_loads_reference_nav_automatically_in_projection(bool async) - => AssertTranslationFailed(() => base.Query_loads_reference_nav_automatically_in_projection(async)); + => AssertTranslationFailedWithDetails( + () => base.Query_loads_reference_nav_automatically_in_projection(async), + CosmosStrings.CrossDocumentJoinNotSupported); // Non-correlated queries not supported by Cosmos public override Task Query_with_owned_entity_equality_operator(bool async) @@ -161,10 +163,11 @@ FROM root c """); }); - // TODO: Fake LeftJoin, #33969 + // Address.Planet is a non-owned navigation, cross-document join public override Task Filter_owned_entity_chained_with_regular_entity_followed_by_projecting_owned_collection(bool async) - => AssertTranslationFailed( - () => base.Filter_owned_entity_chained_with_regular_entity_followed_by_projecting_owned_collection(async)); + => AssertTranslationFailedWithDetails( + () => base.Filter_owned_entity_chained_with_regular_entity_followed_by_projecting_owned_collection(async), + CosmosStrings.CrossDocumentJoinNotSupported); public override async Task Set_throws_for_owned_type(bool async) { @@ -173,55 +176,65 @@ public override async Task Set_throws_for_owned_type(bool async) AssertSql(); } - // TODO: Fake LeftJoin, #33969 + // Address.Planet is a non-owned navigation, cross-document join public override Task Navigation_rewrite_on_owned_reference_followed_by_regular_entity(bool async) - => AssertTranslationFailed( - () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity(async)); + => AssertTranslationFailedWithDetails( + () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity(async), + CosmosStrings.CrossDocumentJoinNotSupported); - // TODO: Fake LeftJoin, #33969 + // Address.Planet is a non-owned navigation, cross-document join public override Task Navigation_rewrite_on_owned_reference_followed_by_regular_entity_filter(bool async) - => AssertTranslationFailed( - () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_filter(async)); + => AssertTranslationFailedWithDetails( + () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity(async), + CosmosStrings.CrossDocumentJoinNotSupported); - // TODO: Fake LeftJoin, #33969 + // Address.Planet is a non-owned navigation, cross-document join public override Task Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_another_reference(bool async) - => AssertTranslationFailed( - () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_another_reference(async)); + => AssertTranslationFailedWithDetails( + () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_another_reference(async), + CosmosStrings.CrossDocumentJoinNotSupported); - // TODO: Fake LeftJoin, #33969 + // Address.Planet is a non-owned navigation, cross-document join public override Task Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_another_reference_and_scalar(bool async) - => AssertTranslationFailed( - () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_another_reference_and_scalar(async)); + => AssertTranslationFailedWithDetails( + () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_another_reference_and_scalar(async), + CosmosStrings.CrossDocumentJoinNotSupported); - // TODO: Fake LeftJoin, #33969 + // Address.Planet is a non-owned navigation, cross-document join public override Task Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_collection(bool async) - => AssertTranslationFailed( - () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_collection(async)); + => AssertTranslationFailedWithDetails( + () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_collection(async), + CosmosStrings.CrossDocumentJoinNotSupported); - // TODO: Fake LeftJoin, #33969 + // Address.Planet is a non-owned navigation, cross-document join public override Task Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_collection_count(bool async) - => AssertTranslationFailed( - () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_collection(async)); + => AssertTranslationFailedWithDetails( + () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_collection_count(async), + CosmosStrings.CrossDocumentJoinNotSupported); - // TODO: Fake LeftJoin, #33969 + // Address.Planet is a non-owned navigation, cross-document join public override Task Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_property(bool async) - => AssertTranslationFailed( - () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_property(async)); + => AssertTranslationFailedWithDetails( + () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_property(async), + CosmosStrings.CrossDocumentJoinNotSupported); - // TODO: Fake LeftJoin, #33969 + // Address.Planet is a non-owned navigation, cross-document join public override Task Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_another_reference_in_predicate_and_projection(bool async) - => AssertTranslationFailed( - () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_another_reference_in_predicate_and_projection(async)); + => AssertTranslationFailedWithDetails( + () => base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity_and_another_reference_in_predicate_and_projection(async), + CosmosStrings.CrossDocumentJoinNotSupported); - // TODO: Fake LeftJoin, #33969 + // Address.Planet is a non-owned navigation, cross-document join public override Task Project_multiple_owned_navigations(bool async) - => AssertTranslationFailed( - () => base.Project_multiple_owned_navigations(async)); + => AssertTranslationFailedWithDetails( + () => base.Project_multiple_owned_navigations(async), + CosmosStrings.CrossDocumentJoinNotSupported); - // TODO: Fake LeftJoin, #33969 + // Address.Planet is a non-owned navigation, cross-document join public override Task Project_multiple_owned_navigations_with_expansion_on_owned_collections(bool async) - => AssertTranslationFailed( - () => base.Project_multiple_owned_navigations_with_expansion_on_owned_collections(async)); + => AssertTranslationFailedWithDetails( + () => base.Project_multiple_owned_navigations_with_expansion_on_owned_collections(async), + CosmosStrings.CrossDocumentJoinNotSupported); [ConditionalTheory] [MemberData(nameof(IsAsyncData))] @@ -240,13 +253,38 @@ JOIN a IN c["Orders"] """); }); - // TODO: Fake LeftJoin, #33969 + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public override Task SelectMany_with_result_selector(bool async) + => CosmosTestHelpers.Instance.NoSyncTest( + async, async a => + { + await base.SelectMany_with_result_selector(a); + + AssertSql( + """ +SELECT VALUE +{ + "PersonId" : c["Id"], + "OrderId" : a["Id"] +} +FROM root c +JOIN a IN c["Orders"] +WHERE c["Discriminator"] IN ("OwnedPerson", "Branch", "LeafB", "LeafA") +"""); + }); + + // Address.Planet is a non-owned navigation, cross-document join public override Task SelectMany_on_owned_reference_followed_by_regular_entity_and_collection(bool async) - => AssertTranslationFailed(() => base.SelectMany_on_owned_reference_followed_by_regular_entity_and_collection(async)); + => AssertTranslationFailedWithDetails( + () => base.SelectMany_on_owned_reference_followed_by_regular_entity_and_collection(async), + CosmosStrings.CrossDocumentJoinNotSupported); - // TODO: Fake LeftJoin, #33969 + // Address.Planet is a non-owned navigation, cross-document join public override Task SelectMany_on_owned_reference_with_entity_in_between_ending_in_owned_collection(bool async) - => AssertTranslationFailed(() => base.SelectMany_on_owned_reference_with_entity_in_between_ending_in_owned_collection(async)); + => AssertTranslationFailedWithDetails( + () => base.SelectMany_on_owned_reference_with_entity_in_between_ending_in_owned_collection(async), + CosmosStrings.CrossDocumentJoinNotSupported); // Non-correlated queries not supported by Cosmos public override Task Query_with_owned_entity_equality_method(bool async) @@ -274,6 +312,32 @@ FROM root c public override Task Query_when_subquery(bool async) => AssertTranslationFailed(() => base.Query_when_subquery(async)); + public override Task Project_owned_reference_navigation_which_owns_additional(bool async) + => CosmosTestHelpers.Instance.NoSyncTest( + async, async a => + { + await base.Project_owned_reference_navigation_which_owns_additional(a); + + // TODO: The following should project out c["PersonAddress"], not c: #34067 + AssertSql( + """ +SELECT c +FROM root c +WHERE c["Discriminator"] IN ("OwnedPerson", "Branch", "LeafB", "LeafA") +ORDER BY c["Id"] +"""); + }); + + // TODO: #34068 + public override async Task Project_owned_reference_navigation_which_does_not_own_additional(bool async) + { + // Always throws for sync. + if (async) + { + await Assert.ThrowsAsync(() => base.Project_owned_reference_navigation_which_does_not_own_additional(async)); + } + } + public override Task No_ignored_include_warning_when_implicit_load(bool async) => CosmosTestHelpers.Instance.NoSyncTest( async, async a => @@ -320,6 +384,7 @@ await AssertQuery( assertOrder: true, elementAsserter: (e, a) => AssertCollection(e, a)); + // TODO: The following should project out a["Details"], not a: #34067 AssertSql( """ SELECT a @@ -687,12 +752,13 @@ public override async Task NoTracking_Include_with_cycles_throws(bool async) AssertSql(); } - // TODO: Fake LeftJoin, #33969 + // Order.Client is a non-owned navigation, cross-document join public override Task NoTracking_Include_with_cycles_does_not_throw_when_performing_identity_resolution( bool async, bool useAsTracking) - => AssertTranslationFailed( - () => base.NoTracking_Include_with_cycles_does_not_throw_when_performing_identity_resolution(async, useAsTracking)); + => AssertTranslationFailedWithDetails( + () => base.NoTracking_Include_with_cycles_does_not_throw_when_performing_identity_resolution(async, useAsTracking), + CosmosStrings.CrossDocumentJoinNotSupported); public override async Task Trying_to_access_non_existent_indexer_property_throws_meaningful_exception(bool async) { diff --git a/test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs index 7708a68f809..bf8d2404c7e 100644 --- a/test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs @@ -130,6 +130,20 @@ public virtual Task Query_when_subquery(bool async) assertOrder: true, elementAsserter: (e, a) => AssertEqual(e.op, a.op)); + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Project_owned_reference_navigation_which_owns_additional(bool async) + => AssertQuery( + async, + ss => ss.Set().OrderBy(o => o.Id).Select(p => p.PersonAddress)); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Project_owned_reference_navigation_which_does_not_own_additional(bool async) + => AssertQuery( + async, + ss => ss.Set().OrderBy(o => o.Id).Select(p => p.PersonAddress.Country)); + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Navigation_rewrite_on_owned_reference_projecting_scalar(bool async) @@ -177,6 +191,13 @@ public virtual Task SelectMany_on_owned_collection(bool async) async, ss => ss.Set().SelectMany(p => p.Orders)); + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task SelectMany_with_result_selector(bool async) + => AssertQuery( + async, + ss => ss.Set().SelectMany(o => o.Orders, (p, o) => new { PersonId = p.Id, OrderId = o.Id })); + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual async Task Set_throws_for_owned_type(bool async) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs index e3f6baa47cb..6b28475aee8 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs @@ -191,6 +191,30 @@ FROM [Order] AS [o1] """); } + public override async Task Project_owned_reference_navigation_which_owns_additional(bool async) + { + await base.Project_owned_reference_navigation_which_owns_additional(async); + + AssertSql( + """ +SELECT [o].[Id], [o].[PersonAddress_AddressLine], [o].[PersonAddress_PlaceType], [o].[PersonAddress_ZipCode], [o].[PersonAddress_Country_Name], [o].[PersonAddress_Country_PlanetId] +FROM [OwnedPerson] AS [o] +ORDER BY [o].[Id] +"""); + } + + public override async Task Project_owned_reference_navigation_which_does_not_own_additional(bool async) + { + await base.Project_owned_reference_navigation_which_does_not_own_additional(async); + + AssertSql( + """ +SELECT [o].[Id], [o].[PersonAddress_Country_Name], [o].[PersonAddress_Country_PlanetId] +FROM [OwnedPerson] AS [o] +ORDER BY [o].[Id] +"""); + } + public override async Task Navigation_rewrite_on_owned_reference_projecting_scalar(bool async) { await base.Navigation_rewrite_on_owned_reference_projecting_scalar(async); @@ -291,6 +315,18 @@ FROM [OwnedPerson] AS [o] """); } + public override async Task SelectMany_with_result_selector(bool async) + { + await base.SelectMany_with_result_selector(async); + + AssertSql( + """ +SELECT [o].[Id] AS [PersonId], [o0].[Id] AS [OrderId] +FROM [OwnedPerson] AS [o] +INNER JOIN [Order] AS [o0] ON [o].[Id] = [o0].[ClientId] +"""); + } + public override async Task Navigation_rewrite_on_owned_reference_followed_by_regular_entity(bool async) { await base.Navigation_rewrite_on_owned_reference_followed_by_regular_entity(async);