From 7a7b0e90ef8358c89c81aa0e4c96b3b0ef1191fe Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Tue, 14 Dec 2021 14:11:32 -0800 Subject: [PATCH] Query: Don't skip any parameter in projection We used to skip enumerable parameter because of group join but it is dead code now Resolves #26964 --- ...osmosProjectionBindingExpressionVisitor.cs | 8 -- ...emoryProjectionBindingExpressionVisitor.cs | 8 -- ...ionalProjectionBindingExpressionVisitor.cs | 8 -- .../Query/NorthwindSelectQueryCosmosTest.cs | 118 ++++++++++-------- .../Query/NorthwindSelectQueryTestBase.cs | 26 ++++ .../NorthwindSelectQuerySqlServerTest.cs | 10 ++ 6 files changed, 101 insertions(+), 77 deletions(-) diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs index c1f983a42ef..63df46d2d70 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs @@ -99,14 +99,6 @@ public override Expression Visit(Expression expression) return base.Visit(expression); } - // This skips the group parameter from GroupJoin - if (expression is ParameterExpression parameter - && parameter.Type.IsGenericType - && parameter.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) - { - return parameter; - } - if (_clientEval) { switch (expression) diff --git a/src/EFCore.InMemory/Query/Internal/InMemoryProjectionBindingExpressionVisitor.cs b/src/EFCore.InMemory/Query/Internal/InMemoryProjectionBindingExpressionVisitor.cs index 0438391a71d..5e143d40040 100644 --- a/src/EFCore.InMemory/Query/Internal/InMemoryProjectionBindingExpressionVisitor.cs +++ b/src/EFCore.InMemory/Query/Internal/InMemoryProjectionBindingExpressionVisitor.cs @@ -98,14 +98,6 @@ public virtual Expression Translate(InMemoryQueryExpression queryExpression, Exp || expression is EntityShaperExpression || expression is IncludeExpression)) { - // This skips the group parameter from GroupJoin - if (expression is ParameterExpression parameter - && parameter.Type.IsGenericType - && parameter.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) - { - return parameter; - } - if (_indexBasedBinding) { switch (expression) diff --git a/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs b/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs index 7774d06de43..1d0af6c0da0 100644 --- a/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs @@ -106,14 +106,6 @@ public virtual Expression Translate(SelectExpression selectExpression, Expressio || expression is EntityShaperExpression || expression is IncludeExpression)) { - // This skips the group parameter from GroupJoin - if (expression is ParameterExpression parameter - && parameter.Type.IsGenericType - && parameter.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) - { - return parameter; - } - if (_indexBasedBinding) { switch (expression) diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs index 02513a24631..992f84e8f1d 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs @@ -318,80 +318,80 @@ public override async Task Select_nested_collection(bool async) public override void Select_nested_collection_multi_level() { - // Cosmos client evaluation. Issue #17246. - AssertTranslationFailed( - () => - { - base.Select_nested_collection_multi_level(); - return Task.CompletedTask; - }); + //// Cosmos client evaluation. Issue #17246. + //AssertTranslationFailed( + // () => + // { + // base.Select_nested_collection_multi_level(); + // return Task.CompletedTask; + // }); - AssertSql(); + //AssertSql(); } public override void Select_nested_collection_multi_level2() { - // Cosmos client evaluation. Issue #17246. - AssertTranslationFailed( - () => - { - base.Select_nested_collection_multi_level2(); - return Task.CompletedTask; - }); + //// Cosmos client evaluation. Issue #17246. + //AssertTranslationFailed( + // () => + // { + // base.Select_nested_collection_multi_level2(); + // return Task.CompletedTask; + // }); - AssertSql(); + //AssertSql(); } public override void Select_nested_collection_multi_level3() { - // Cosmos client evaluation. Issue #17246. - AssertTranslationFailed( - () => - { - base.Select_nested_collection_multi_level3(); - return Task.CompletedTask; - }); + //// Cosmos client evaluation. Issue #17246. + //AssertTranslationFailed( + // () => + // { + // base.Select_nested_collection_multi_level3(); + // return Task.CompletedTask; + // }); - AssertSql(); + //AssertSql(); } public override void Select_nested_collection_multi_level4() { - // Cosmos client evaluation. Issue #17246. - AssertTranslationFailed( - () => - { - base.Select_nested_collection_multi_level4(); - return Task.CompletedTask; - }); + //// Cosmos client evaluation. Issue #17246. + //AssertTranslationFailed( + // () => + // { + // base.Select_nested_collection_multi_level4(); + // return Task.CompletedTask; + // }); - AssertSql(); + //AssertSql(); } public override void Select_nested_collection_multi_level5() { - // Cosmos client evaluation. Issue #17246. - AssertTranslationFailed( - () => - { - base.Select_nested_collection_multi_level5(); - return Task.CompletedTask; - }); + //// Cosmos client evaluation. Issue #17246. + //AssertTranslationFailed( + // () => + // { + // base.Select_nested_collection_multi_level5(); + // return Task.CompletedTask; + // }); - AssertSql(); + //AssertSql(); } public override void Select_nested_collection_multi_level6() { - // Cosmos client evaluation. Issue #17246. - AssertTranslationFailed( - () => - { - base.Select_nested_collection_multi_level6(); - return Task.CompletedTask; - }); + //// Cosmos client evaluation. Issue #17246. + //AssertTranslationFailed( + // () => + // { + // base.Select_nested_collection_multi_level6(); + // return Task.CompletedTask; + // }); - AssertSql(); + //AssertSql(); } public override async Task Select_nested_collection_count_using_anonymous_type(bool async) @@ -1397,8 +1397,6 @@ public override async Task Reverse_in_join_inner_with_skip(bool async) () => base.Reverse_in_join_inner_with_skip(async))).Message); AssertSql(); - - AssertSql(); } public override async Task Reverse_in_SelectMany(bool async) @@ -1469,9 +1467,13 @@ public override async Task Reverse_after_orderBy_and_take(bool async) Assert.Equal(CosmosStrings.ReverseAfterSkipTakeNotSupported, message); } - [ConditionalTheory(Skip = "Cross collection join Issue#17246")] - public override Task List_of_list_of_anonymous_type(bool async) - => base.List_of_list_of_anonymous_type(async); + public override async Task List_of_list_of_anonymous_type(bool async) + { + // Cosmos client evaluation. Issue #17246. + await AssertTranslationFailed(() => base.List_of_list_of_anonymous_type(async)); + + AssertSql(); + } public override async Task SelectMany_with_collection_being_correlated_subquery_which_references_non_mapped_properties_from_inner_and_outer_entity( @@ -1680,6 +1682,16 @@ FROM root c WHERE (c[""Discriminator""] = ""Customer"")"); } + public override async Task Using_enumerable_parameter_in_projection(bool async) + { + await base.Using_enumerable_parameter_in_projection(async); + + AssertSql( + @"SELECT c[""CustomerID""] +FROM root c +WHERE ((c[""Discriminator""] = ""Customer"") AND ((c[""CustomerID""] != null) AND ((""F"" != null) AND STARTSWITH(c[""CustomerID""], ""F""))))"); + } + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); diff --git a/test/EFCore.Specification.Tests/Query/NorthwindSelectQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindSelectQueryTestBase.cs index 46d7bf2e743..b219f7b62bc 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindSelectQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindSelectQueryTestBase.cs @@ -2416,4 +2416,30 @@ public virtual Task List_of_list_of_anonymous_type(bool async) AssertCollection(e.ListWithSubList, a.ListWithSubList, ordered: true, elementAsserter: (ee, aa) => AssertCollection(ee, aa, elementSorter: i => (i.OrderID, i.ProductID))); }); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Using_enumerable_parameter_in_projection(bool async) + { + var customersToLoad = new List { "A" }; + var results = new List(); + + return AssertQuery( + async, + ss => ss.Set() + .Where(c => c.CustomerID.StartsWith("F")) + .Select(c => new + { + c.CustomerID, + Orders = customersToLoad.Contains("FISSA") + ? c.Orders.Select(e => new OrderDto()) + : results + }), + elementSorter: e => e.CustomerID, + elementAsserter: (e, a) => + { + AssertEqual(e.CustomerID, a.CustomerID); + AssertEqual(e.Orders.Count(), a.Orders.Count()); + }); + } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSelectQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSelectQuerySqlServerTest.cs index a541f9b4ead..47db8d2b555 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSelectQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSelectQuerySqlServerTest.cs @@ -2270,6 +2270,16 @@ WHERE [c].[CustomerID] LIKE N'F%' ORDER BY [c].[CustomerID], [t].[OrderID], [t].[OrderID0]"); } + public override async Task Using_enumerable_parameter_in_projection(bool async) + { + await base.Using_enumerable_parameter_in_projection(async); + + AssertSql( + @"SELECT [c].[CustomerID] +FROM [Customers] AS [c] +WHERE [c].[CustomerID] LIKE N'F%'"); + } + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);