From a08970192501d5e37bc1b799681772abc8095ed0 Mon Sep 17 00:00:00 2001 From: Maurycy Markowski Date: Thu, 17 Nov 2016 17:50:36 -0800 Subject: [PATCH] Fix to #7003 - Wrong SQL generated for query with group join on a subquery that is not present in the final projection Problem was that for GroupJoin we need to mark both inputs for materialization, so we have all the elements necessary to produce joining calls. However, if one side is a subquery, we just mark that outer query source for materialization, without digging deep into the subquery itself. This results in the actual table (which is inside) not being materialized, and that in turn produces SQL which doesn't have necessary elements to produce the correct joins. Fix is to check if the groupjoin element in a subquery, and if so recursively mark it's elements for materialization. --- .../ComplexNavigationsQueryTestBase.cs | 47 +++++++++++++++---- .../Query/QueryCompilationContext.cs | 21 +++++++-- .../ComplexNavigationsQuerySqlServerTest.cs | 42 +++++++++++++++++ 3 files changed, 97 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.EntityFrameworkCore.Specification.Tests/ComplexNavigationsQueryTestBase.cs b/src/Microsoft.EntityFrameworkCore.Specification.Tests/ComplexNavigationsQueryTestBase.cs index 995cb5d472a..1478d7e59c5 100644 --- a/src/Microsoft.EntityFrameworkCore.Specification.Tests/ComplexNavigationsQueryTestBase.cs +++ b/src/Microsoft.EntityFrameworkCore.Specification.Tests/ComplexNavigationsQueryTestBase.cs @@ -3111,14 +3111,45 @@ public virtual void Contains_with_subquery_optional_navigation_and_constant_item public virtual void Complex_query_with_optional_navigations_and_client_side_evaluation() { AssertQuery( - l1s => l1s.Where(l1 => !l1.OneToMany_Optional.Select(l2 => l2.OneToOne_Optional_FK.OneToOne_Optional_FK.Id).All(l4 => ClientMethod(l4))), - l1s => l1s.Where(l1 => l1.OneToMany_Optional.Select(l2 => MaybeScalar( - l2.OneToOne_Optional_FK, - () => MaybeScalar( - l2.OneToOne_Optional_FK.OneToOne_Optional_FK, - () => l2.OneToOne_Optional_FK.OneToOne_Optional_FK.Id))).All(a => true)), - e => e.Id, - (e, a) => Assert.Equal(e.Id, a.Id)); + l1s => l1s.Where(l1 => !l1.OneToMany_Optional.Select(l2 => l2.OneToOne_Optional_FK.OneToOne_Optional_FK.Id).All(l4 => ClientMethod(l4))), + l1s => l1s.Where(l1 => l1.OneToMany_Optional.Select(l2 => MaybeScalar( + l2.OneToOne_Optional_FK, + () => MaybeScalar( + l2.OneToOne_Optional_FK.OneToOne_Optional_FK, + () => l2.OneToOne_Optional_FK.OneToOne_Optional_FK.Id))).All(a => true)), + e => e.Id, + (e, a) => Assert.Equal(e.Id, a.Id)); + } + + [ConditionalFact] + public virtual void GroupJoin_on_left_side_being_a_subquery() + { + AssertQuery( + l1s => l1s.OrderBy(l1 => l1.OneToOne_Optional_FK.Name) + .Take(2) + .Select(x => new { Id = x.Id, Brand = x.OneToOne_Optional_FK.Name }), + l1s => l1s.OrderBy(l1 => Maybe(l1.OneToOne_Optional_FK, () => l1.OneToOne_Optional_FK.Name)) + .Take(2) + .Select(x => new { Id = x.Id, Brand = Maybe(x.OneToOne_Optional_FK, () => x.OneToOne_Optional_FK.Name) }), + e => e.Id); + } + + [ConditionalFact] + public virtual void GroupJoin_on_right_side_being_a_subquery() + { + AssertQuery( + (l1s, l2s) => + from l2 in l2s + join l1 in l1s.OrderBy(x => x.OneToOne_Optional_FK.Name).Take(2) on l2.Level1_Optional_Id equals l1.Id into grouping + from l1 in grouping.DefaultIfEmpty() + select new { Id = l2.Id, Nane = l1 != null ? l1.Name : null }, + (l1s, l2s) => + from l2 in l2s + join l1 in l1s.OrderBy(x => Maybe(x.OneToOne_Optional_FK, () => x.OneToOne_Optional_FK.Name)).Take(2) + on l2.Level1_Optional_Id equals l1.Id into grouping + from l1 in grouping.DefaultIfEmpty() + select new { Id = l2.Id, Nane = l1 != null ? l1.Name : null }, + e => e.Id); } private bool ClientMethod(int? id) diff --git a/src/Microsoft.EntityFrameworkCore/Query/QueryCompilationContext.cs b/src/Microsoft.EntityFrameworkCore/Query/QueryCompilationContext.cs index 574c3b7dea7..8f5e5c2e587 100644 --- a/src/Microsoft.EntityFrameworkCore/Query/QueryCompilationContext.cs +++ b/src/Microsoft.EntityFrameworkCore/Query/QueryCompilationContext.cs @@ -33,7 +33,7 @@ public class QueryCompilationContext private IReadOnlyCollection _queryAnnotations; private IDictionary>> _trackableIncludes; - private ISet _querySourcesRequiringMaterialization; + private ISet _querySourcesRequiringMaterialization = new HashSet(); /// /// This API supports the Entity Framework Core infrastructure and is not intended to be used @@ -340,15 +340,26 @@ public virtual void FindQuerySourcesRequiringMaterialization( Check.NotNull(queryModelVisitor, nameof(queryModelVisitor)); Check.NotNull(queryModel, nameof(queryModel)); - _querySourcesRequiringMaterialization - = _requiresMaterializationExpressionVisitorFactory - .Create(queryModelVisitor) - .FindQuerySourcesRequiringMaterialization(queryModel); + var querySourcesRequiringMaterialization = _requiresMaterializationExpressionVisitorFactory + .Create(queryModelVisitor) + .FindQuerySourcesRequiringMaterialization(queryModel); + + foreach (var querySourceRequiringMaterialization in querySourcesRequiringMaterialization) + { + _querySourcesRequiringMaterialization.Add(querySourceRequiringMaterialization); + } var groupJoinClauses = queryModel.BodyClauses.OfType().ToList(); if (groupJoinClauses.Any()) { _querySourcesRequiringMaterialization.Add(queryModel.MainFromClause); + + var mainFromClauseSubquery = queryModel.MainFromClause.FromExpression as SubQueryExpression; + if (mainFromClauseSubquery != null) + { + FindQuerySourcesRequiringMaterialization(queryModelVisitor, mainFromClauseSubquery.QueryModel); + } + foreach (var groupJoinClause in groupJoinClauses) { _querySourcesRequiringMaterialization.Add(groupJoinClause.JoinClause); diff --git a/test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/ComplexNavigationsQuerySqlServerTest.cs b/test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/ComplexNavigationsQuerySqlServerTest.cs index 243c503813d..89685ef065d 100644 --- a/test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/ComplexNavigationsQuerySqlServerTest.cs +++ b/test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/ComplexNavigationsQuerySqlServerTest.cs @@ -2414,6 +2414,48 @@ public override void Complex_query_with_optional_navigations_and_client_side_eva Sql); } + public override void GroupJoin_on_left_side_being_a_subquery() + { + base.GroupJoin_on_left_side_being_a_subquery(); + + Assert.Equal( + @"SELECT [x.OneToOne_Optional_FK].[Id], [x.OneToOne_Optional_FK].[Date], [x.OneToOne_Optional_FK].[Level1_Optional_Id], [x.OneToOne_Optional_FK].[Level1_Required_Id], [x.OneToOne_Optional_FK].[Name], [x.OneToOne_Optional_FK].[OneToMany_Optional_InverseId], [x.OneToOne_Optional_FK].[OneToMany_Optional_Self_InverseId], [x.OneToOne_Optional_FK].[OneToMany_Required_InverseId], [x.OneToOne_Optional_FK].[OneToMany_Required_Self_InverseId], [x.OneToOne_Optional_FK].[OneToOne_Optional_PK_InverseId], [x.OneToOne_Optional_FK].[OneToOne_Optional_SelfId] +FROM [Level2] AS [x.OneToOne_Optional_FK] + +@__p_0: 2 + +SELECT [t].[Id], [t].[Date], [t].[Name], [t].[OneToMany_Optional_Self_InverseId], [t].[OneToMany_Required_Self_InverseId], [t].[OneToOne_Optional_SelfId], [t].[c0], [t].[c1], [t].[Level1_Optional_Id], [t].[Level1_Required_Id], [t].[c2], [t].[OneToMany_Optional_InverseId], [t].[c3], [t].[OneToMany_Required_InverseId], [t].[c4], [t].[OneToOne_Optional_PK_InverseId], [t].[c5] +FROM ( + SELECT TOP(@__p_0) [l10].[Id], [l10].[Date], [l10].[Name], [l10].[OneToMany_Optional_Self_InverseId], [l10].[OneToMany_Required_Self_InverseId], [l10].[OneToOne_Optional_SelfId], [l1.OneToOne_Optional_FK1].[Id] AS [c0], [l1.OneToOne_Optional_FK1].[Date] AS [c1], [l1.OneToOne_Optional_FK1].[Level1_Optional_Id], [l1.OneToOne_Optional_FK1].[Level1_Required_Id], [l1.OneToOne_Optional_FK1].[Name] AS [c2], [l1.OneToOne_Optional_FK1].[OneToMany_Optional_InverseId], [l1.OneToOne_Optional_FK1].[OneToMany_Optional_Self_InverseId] AS [c3], [l1.OneToOne_Optional_FK1].[OneToMany_Required_InverseId], [l1.OneToOne_Optional_FK1].[OneToMany_Required_Self_InverseId] AS [c4], [l1.OneToOne_Optional_FK1].[OneToOne_Optional_PK_InverseId], [l1.OneToOne_Optional_FK1].[OneToOne_Optional_SelfId] AS [c5] + FROM [Level1] AS [l10] + LEFT JOIN [Level2] AS [l1.OneToOne_Optional_FK1] ON [l10].[Id] = [l1.OneToOne_Optional_FK1].[Level1_Optional_Id] + ORDER BY [l1.OneToOne_Optional_FK1].[Name], [l10].[Id] +) AS [t] +ORDER BY [t].[Name], [t].[Id]", + Sql); + } + + public override void GroupJoin_on_right_side_being_a_subquery() + { + base.GroupJoin_on_right_side_being_a_subquery(); + + Assert.Equal( + @"@__p_0: 2 + +SELECT [t].[Id], [t].[Date], [t].[Name], [t].[OneToMany_Optional_Self_InverseId], [t].[OneToMany_Required_Self_InverseId], [t].[OneToOne_Optional_SelfId], [t].[c0], [t].[c1], [t].[Level1_Optional_Id], [t].[Level1_Required_Id], [t].[c2], [t].[OneToMany_Optional_InverseId], [t].[c3], [t].[OneToMany_Required_InverseId], [t].[c4], [t].[OneToOne_Optional_PK_InverseId], [t].[c5] +FROM ( + SELECT TOP(@__p_0) [x0].[Id], [x0].[Date], [x0].[Name], [x0].[OneToMany_Optional_Self_InverseId], [x0].[OneToMany_Required_Self_InverseId], [x0].[OneToOne_Optional_SelfId], [x.OneToOne_Optional_FK1].[Id] AS [c0], [x.OneToOne_Optional_FK1].[Date] AS [c1], [x.OneToOne_Optional_FK1].[Level1_Optional_Id], [x.OneToOne_Optional_FK1].[Level1_Required_Id], [x.OneToOne_Optional_FK1].[Name] AS [c2], [x.OneToOne_Optional_FK1].[OneToMany_Optional_InverseId], [x.OneToOne_Optional_FK1].[OneToMany_Optional_Self_InverseId] AS [c3], [x.OneToOne_Optional_FK1].[OneToMany_Required_InverseId], [x.OneToOne_Optional_FK1].[OneToMany_Required_Self_InverseId] AS [c4], [x.OneToOne_Optional_FK1].[OneToOne_Optional_PK_InverseId], [x.OneToOne_Optional_FK1].[OneToOne_Optional_SelfId] AS [c5] + FROM [Level1] AS [x0] + LEFT JOIN [Level2] AS [x.OneToOne_Optional_FK1] ON [x0].[Id] = [x.OneToOne_Optional_FK1].[Level1_Optional_Id] + ORDER BY [x.OneToOne_Optional_FK1].[Name], [x0].[Id] +) AS [t] +ORDER BY [t].[Name], [t].[Id] + +SELECT [l2].[Id], [l2].[Date], [l2].[Level1_Optional_Id], [l2].[Level1_Required_Id], [l2].[Name], [l2].[OneToMany_Optional_InverseId], [l2].[OneToMany_Optional_Self_InverseId], [l2].[OneToMany_Required_InverseId], [l2].[OneToMany_Required_Self_InverseId], [l2].[OneToOne_Optional_PK_InverseId], [l2].[OneToOne_Optional_SelfId] +FROM [Level2] AS [l2]", + Sql); + } + private const string FileLineEnding = @" ";