From 5e666dead4e4cd1d4d48daba3e3801f6e59db328 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Fri, 31 May 2019 12:32:54 -0700 Subject: [PATCH] Query: Apply unique projection names in the subquery Fixes #15833 --- ...yableMethodTranslatingExpressionVisitor.cs | 81 ++++++++++--------- .../Query/ComplexNavigationsQueryTestBase.cs | 48 +++++------ .../Query/GearsOfWarQueryTestBase.cs | 6 +- .../Query/QueryNavigationsTestBase.cs | 4 +- 4 files changed, 71 insertions(+), 68 deletions(-) diff --git a/src/EFCore.Relational/Query/Pipeline/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/Pipeline/RelationalQueryableMethodTranslatingExpressionVisitor.cs index a0d282a3f55..80135682ea5 100644 --- a/src/EFCore.Relational/Query/Pipeline/RelationalQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Pipeline/RelationalQueryableMethodTranslatingExpressionVisitor.cs @@ -254,44 +254,45 @@ protected override ShapedQueryExpression TranslateFirstOrDefault(ShapedQueryExpr protected override ShapedQueryExpression TranslateGroupJoin(ShapedQueryExpression outer, ShapedQueryExpression inner, LambdaExpression outerKeySelector, LambdaExpression innerKeySelector, LambdaExpression resultSelector) { - var outerSelectExpression = (SelectExpression)outer.QueryExpression; - if (outerSelectExpression.Limit != null - || outerSelectExpression.Offset != null - || outerSelectExpression.IsDistinct) - { - outerSelectExpression.PushdownIntoSubQuery(); - } - - var innerSelectExpression = (SelectExpression)inner.QueryExpression; - if (innerSelectExpression.Orderings.Any() - || innerSelectExpression.Limit != null - || innerSelectExpression.Offset != null - || innerSelectExpression.IsDistinct - || innerSelectExpression.Predicate != null) - { - innerSelectExpression.PushdownIntoSubQuery(); - } - - var joinPredicate = CreateJoinPredicate(outer, outerKeySelector, inner, innerKeySelector); - if (joinPredicate != null) - { - outer = TranslateThenBy(outer, outerKeySelector, true); - - var innerTransparentIdentifierType = CreateTransparentIdentifierType( - resultSelector.Parameters[0].Type, - resultSelector.Parameters[1].Type.TryGetSequenceType()); - - outerSelectExpression.AddLeftJoin( - innerSelectExpression, joinPredicate, innerTransparentIdentifierType); - - return TranslateResultSelectorForGroupJoin( - outer, - inner.ShaperExpression, - outerKeySelector, - innerKeySelector, - resultSelector, - innerTransparentIdentifierType); - } + //var outerSelectExpression = (SelectExpression)outer.QueryExpression; + //if (outerSelectExpression.Limit != null + // || outerSelectExpression.Offset != null + // || outerSelectExpression.IsDistinct) + //{ + // outerSelectExpression.PushdownIntoSubQuery(); + //} + + //var innerSelectExpression = (SelectExpression)inner.QueryExpression; + //if (innerSelectExpression.Orderings.Any() + // || innerSelectExpression.Limit != null + // || innerSelectExpression.Offset != null + // || innerSelectExpression.IsDistinct + // || innerSelectExpression.Predicate != null + // || innerSelectExpression.Tables.Count > 1) + //{ + // innerSelectExpression.PushdownIntoSubQuery(); + //} + + //var joinPredicate = CreateJoinPredicate(outer, outerKeySelector, inner, innerKeySelector); + //if (joinPredicate != null) + //{ + // outer = TranslateThenBy(outer, outerKeySelector, true); + + // var innerTransparentIdentifierType = CreateTransparentIdentifierType( + // resultSelector.Parameters[0].Type, + // resultSelector.Parameters[1].Type.TryGetSequenceType()); + + // outerSelectExpression.AddLeftJoin( + // innerSelectExpression, joinPredicate, innerTransparentIdentifierType); + + // return TranslateResultSelectorForGroupJoin( + // outer, + // inner.ShaperExpression, + // outerKeySelector, + // innerKeySelector, + // resultSelector, + // innerTransparentIdentifierType); + //} throw new NotImplementedException(); } @@ -305,6 +306,7 @@ protected override ShapedQueryExpression TranslateJoin( LambdaExpression innerKeySelector, LambdaExpression resultSelector) { + // TODO: write a test which has distinct on outer so that we can verify pushdown var innerSelectExpression = (SelectExpression)inner.QueryExpression; if (innerSelectExpression.Orderings.Any() || innerSelectExpression.Limit != null @@ -353,7 +355,8 @@ protected override ShapedQueryExpression TranslateLeftJoin(ShapedQueryExpression || innerSelectExpression.Limit != null || innerSelectExpression.Offset != null || innerSelectExpression.IsDistinct - || innerSelectExpression.Predicate != null) + || innerSelectExpression.Predicate != null + || innerSelectExpression.Tables.Count > 1) { innerSelectExpression.PushdownIntoSubQuery(); } diff --git a/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs index 9d614794c55..10d1c4b213c 100644 --- a/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs @@ -348,7 +348,7 @@ public virtual void Multi_level_include_with_short_circuiting() } } - [ConditionalTheory(Skip = "Issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Join_navigation_key_access_optional(bool isAsync) { @@ -375,7 +375,7 @@ join e2 in l2s on e1.Id equals MaybeScalar( e => e.Id1 + " " + e.Id2); } - [ConditionalTheory(Skip = "Issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Join_navigation_key_access_required(bool isAsync) { @@ -759,7 +759,7 @@ join e1 in l1s on MaybeScalar( e => e.Id1 + " " + e.Id3); } - [ConditionalTheory(Skip = "Issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Join_navigation_in_inner_selector_translated_to_subquery(bool isAsync) { @@ -784,7 +784,7 @@ join e1 in l1s on e2.Id equals MaybeScalar(e1.OneToOne_Optional_FK1, () => e => e.Id2 + " " + e.Id1); } - [ConditionalTheory(Skip = "Issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Join_navigations_in_inner_selector_translated_to_multiple_subquery_without_collision(bool isAsync) { @@ -814,7 +814,7 @@ join e3 in l3s on e2.Id equals MaybeScalar( e => e.Id2 + " " + e.Id1 + " " + e.Id3); } - [ConditionalTheory(Skip = "issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Join_navigation_translated_to_subquery_non_key_join(bool isAsync) { @@ -874,7 +874,7 @@ join e1 in l1s.OrderBy(l1 => l1.Id) on e2.Name equals Maybe( e => e.Id2 + " " + e.Name2 + " " + e.Id1 + " " + e.Name1); } - [ConditionalTheory(Skip = "Issue# 15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Join_navigation_translated_to_subquery_self_ref(bool isAsync) { @@ -900,7 +900,7 @@ join e2 in l1s on e1.Id equals MaybeScalar( e => e.Id1 + " " + e.Id2); } - [ConditionalTheory(Skip = "Issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Join_navigation_translated_to_subquery_nested(bool isAsync) { @@ -959,7 +959,7 @@ join e1 in l1s.OrderBy(ll => ll.Id) on e3.Id equals MaybeScalar( e => e.Id3 + " " + e.Id1); } - [ConditionalTheory(Skip = "Issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Join_navigation_translated_to_subquery_deeply_nested_non_key_join(bool isAsync) { @@ -994,7 +994,7 @@ join e1 in l1s on e4.Name equals Maybe( e => e.Id4 + " " + e.Name4 + " " + e.Id1 + " " + e.Name1); } - [ConditionalTheory(Skip = "Issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Join_navigation_translated_to_subquery_deeply_nested_required(bool isAsync) { @@ -2407,7 +2407,7 @@ public virtual Task Null_protection_logic_work_for_inner_key_access_of_manually_ (e, a) => Assert.Equal(e.Id, a.Id)); } - [ConditionalTheory(Skip = "issue #15833")] + [ConditionalTheory(Skip = "Issue#15872")] [MemberData(nameof(IsAsyncData))] public virtual Task Null_protection_logic_work_for_inner_key_access_of_manually_created_GroupJoin2(bool isAsync) { @@ -3680,7 +3680,7 @@ from l1 in grouping.DefaultIfEmpty() elementSorter: e => e.Id1); } - [ConditionalTheory(Skip = "issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Optional_navigation_propagates_nullability_to_manually_created_left_join2(bool isAsync) { @@ -3708,7 +3708,7 @@ from l2_nav in grouping.DefaultIfEmpty() elementSorter: e => e.Name1 + e.Name2); } - [ConditionalTheory(Skip = "issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Null_reference_protection_complex(bool isAsync) { @@ -3736,7 +3736,7 @@ from l2_outer in grouping_outer.DefaultIfEmpty() select Maybe(l2_outer, () => l2_outer.Name)); } - [ConditionalTheory(Skip = "issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Null_reference_protection_complex_materialization(bool isAsync) { @@ -3783,7 +3783,7 @@ private static TResult ClientMethodReturnSelf(TResult element) return element; } - [ConditionalTheory(Skip = "issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Null_reference_protection_complex_client_eval(bool isAsync) { @@ -3811,7 +3811,7 @@ from l2_outer in grouping_outer.DefaultIfEmpty() select ClientMethodReturnSelf(Maybe(l2_outer, () => l2_outer.Name))); } - [ConditionalTheory(Skip = "issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task GroupJoin_with_complex_subquery_with_joins_does_not_get_flattened(bool isAsync) { @@ -3841,7 +3841,7 @@ from subquery in grouping.DefaultIfEmpty() select MaybeScalar(subquery, () => subquery.Id)); } - [ConditionalTheory(Skip = "issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task GroupJoin_with_complex_subquery_with_joins_does_not_get_flattened2(bool isAsync) { @@ -3871,7 +3871,7 @@ from subquery in grouping.DefaultIfEmpty() select MaybeScalar(subquery, () => subquery.Id)); } - [ConditionalTheory(Skip = "issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task GroupJoin_with_complex_subquery_with_joins_does_not_get_flattened3(bool isAsync) { @@ -5851,7 +5851,7 @@ public virtual void Include16() } } - [ConditionalFact(Skip = "issue #15833")] + [ConditionalFact] public virtual void Include17() { using (var ctx = CreateContext()) @@ -5909,7 +5909,7 @@ public virtual Task Include18_2(bool isAsync) expectedIncludes); } - [ConditionalFact(Skip = "issue #15833")] + [ConditionalFact] public virtual void Include18_3() { using (var ctx = CreateContext()) @@ -5920,7 +5920,7 @@ public virtual void Include18_3() } } - [ConditionalFact(Skip = "issue #15833")] + [ConditionalFact] public virtual void Include18_3_1() { using (var ctx = CreateContext()) @@ -5931,7 +5931,7 @@ public virtual void Include18_3_1() } } - [ConditionalFact(Skip = "issue #15833")] + [ConditionalFact] public virtual void Include18_3_2() { using (var ctx = CreateContext()) @@ -5957,7 +5957,7 @@ public virtual Task Include18_3_3(bool isAsync) expectedIncludes); } - [ConditionalFact(Skip = "issue #15833")] + [ConditionalFact] public virtual void Include18_4() { using (var ctx = CreateContext()) @@ -5968,7 +5968,7 @@ public virtual void Include18_4() } } - [ConditionalFact(Skip = "issue #15833")] + [ConditionalFact] public virtual void Include18() { using (var ctx = CreateContext()) @@ -5979,7 +5979,7 @@ public virtual void Include18() } } - [ConditionalFact(Skip = "issue #15833")] + [ConditionalFact] public virtual void Include19() { using (var ctx = CreateContext()) diff --git a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs index aa59cfd5cc9..022497d6f3c 100644 --- a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs @@ -1954,7 +1954,7 @@ from g in grouping select g); } - [ConditionalTheory(Skip = "Issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Join_navigation_translated_to_subquery_composite_key(bool isAsync) { @@ -6137,7 +6137,7 @@ join w2 in ws on w1.SynergyWith equals w2 elementSorter: e => e.Name1 + " " + e.Name2); } - [ConditionalTheory(Skip = "Issue #15833")] + [ConditionalTheory(Skip = "Issue#15588")] [MemberData(nameof(IsAsyncData))] public virtual Task Join_on_entity_qsre_keys_inner_key_is_navigation(bool isAsync) { @@ -6188,7 +6188,7 @@ join w in ws.Where(ww => ww.IsAutomatic) on s equals w.Owner.Squad elementSorter: e => e.SquadName + " " + e.WeaponName); } - [ConditionalTheory(Skip = "Issue #15833")] + [ConditionalTheory(Skip = "Issue#15588")] [MemberData(nameof(IsAsyncData))] public virtual Task GroupJoin_on_entity_qsre_keys_inner_key_is_nested_navigation(bool isAsync) { diff --git a/test/EFCore.Specification.Tests/Query/QueryNavigationsTestBase.cs b/test/EFCore.Specification.Tests/Query/QueryNavigationsTestBase.cs index 8872e4956b0..f0bf24ecfa3 100644 --- a/test/EFCore.Specification.Tests/Query/QueryNavigationsTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/QueryNavigationsTestBase.cs @@ -1318,7 +1318,7 @@ orderby o.OrderID elementSorter: e => e.OrderID); } - [ConditionalTheory(Skip = "issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task GroupJoin_with_complex_subquery_and_LOJ_gets_flattened(bool isAsync) { @@ -1339,7 +1339,7 @@ from subquery in result.DefaultIfEmpty() entryCount: 91); } - [ConditionalTheory(Skip = "issue #15833")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task GroupJoin_with_complex_subquery_and_LOJ_gets_flattened2(bool isAsync) {