diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index dcd3b52aa47..0e31cee000a 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -13,20 +13,19 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions { public class SelectExpression : TableExpressionBase { - private IDictionary _projectionMapping = new Dictionary(); - private readonly List _projection = new List(); - private readonly IDictionary> _entityProjectionCache = new Dictionary>(); + private readonly List _projection = new List(); private readonly List _tables = new List(); private readonly List _groupBy = new List(); private readonly List _orderings = new List(); - private readonly List _identifier = new List(); private readonly List _childIdentifiers = new List(); private readonly List _pendingCollections = new List(); + private IDictionary _projectionMapping = new Dictionary(); + public IReadOnlyList Projection => _projection; public IReadOnlyList Tables => _tables; public IReadOnlyList GroupBy => _groupBy; @@ -137,7 +136,7 @@ public void ApplyProjection() _projectionMapping = result; } - private IEnumerable GetAllPropertiesInHierarchy(IEntityType entityType) + private static IEnumerable GetAllPropertiesInHierarchy(IEntityType entityType) => entityType.GetTypesInHierarchy().SelectMany(EntityTypeExtensions.GetDeclaredProperties); public void ReplaceProjectionMapping(IDictionary projectionMapping) @@ -660,35 +659,7 @@ public IDictionary PushdownIntoSubquery() var projectionMap = new Dictionary(); - EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpression entityProjection) - { - var propertyExpressions = new Dictionary(); - foreach (var property in GetAllPropertiesInHierarchy(entityProjection.EntityType)) - { - var innerColumn = entityProjection.BindProperty(property); - var outerColumn = subquery.GenerateOuterColumn(innerColumn); - projectionMap[innerColumn] = outerColumn; - propertyExpressions[property] = outerColumn; - } - - var newEntityProjection = new EntityProjectionExpression(entityProjection.EntityType, propertyExpressions); - // Also lift nested entity projections - foreach (var navigation in entityProjection.EntityType.GetTypesInHierarchy() - .SelectMany(EntityTypeExtensions.GetDeclaredNavigations)) - { - var boundEntityShaperExpression = entityProjection.BindNavigation(navigation); - if (boundEntityShaperExpression != null) - { - var innerEntityProjection = (EntityProjectionExpression)boundEntityShaperExpression.ValueBufferExpression; - var newInnerEntityProjection = liftEntityProjectionFromSubquery(innerEntityProjection); - boundEntityShaperExpression = boundEntityShaperExpression.Update(newInnerEntityProjection); - newEntityProjection.AddNavigationBinding(navigation, boundEntityShaperExpression); - } - } - - return newEntityProjection; - } - + // Projections may be present if added by lifting SingleResult/Enumerable in projection through join if (_projection.Any()) { var projections = _projection.Select(pe => pe.Expression).ToList(); @@ -700,21 +671,26 @@ EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpr projectionMap[projection] = outerColumn; } } - else + + foreach (var mapping in _projectionMapping.ToList()) { - foreach (var mapping in _projectionMapping.ToList()) + // If projectionMapping's value is ConstantExpression then projection has already been applied + // And captured in _projections above so we don't need to process this. + if (mapping.Value is ConstantExpression) { - if (mapping.Value is EntityProjectionExpression entityProjection) - { - _projectionMapping[mapping.Key] = liftEntityProjectionFromSubquery(entityProjection); - } - else - { - var innerColumn = (SqlExpression)mapping.Value; - var outerColumn = subquery.GenerateOuterColumn(innerColumn); - projectionMap[innerColumn] = outerColumn; - _projectionMapping[mapping.Key] = outerColumn; - } + break; + } + + if (mapping.Value is EntityProjectionExpression entityProjection) + { + _projectionMapping[mapping.Key] = LiftEntityProjectionFromSubquery(entityProjection); + } + else + { + var innerColumn = (SqlExpression)mapping.Value; + var outerColumn = subquery.GenerateOuterColumn(innerColumn); + projectionMap[innerColumn] = outerColumn; + _projectionMapping[mapping.Key] = outerColumn; } } @@ -788,6 +764,35 @@ EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpr _groupBy.Clear(); return projectionMap; + + EntityProjectionExpression LiftEntityProjectionFromSubquery(EntityProjectionExpression entityProjection) + { + var propertyExpressions = new Dictionary(); + foreach (var property in GetAllPropertiesInHierarchy(entityProjection.EntityType)) + { + var innerColumn = entityProjection.BindProperty(property); + var outerColumn = subquery.GenerateOuterColumn(innerColumn); + projectionMap[innerColumn] = outerColumn; + propertyExpressions[property] = outerColumn; + } + + var newEntityProjection = new EntityProjectionExpression(entityProjection.EntityType, propertyExpressions); + // Also lift nested entity projections + foreach (var navigation in entityProjection.EntityType.GetTypesInHierarchy() + .SelectMany(EntityTypeExtensions.GetDeclaredNavigations)) + { + var boundEntityShaperExpression = entityProjection.BindNavigation(navigation); + if (boundEntityShaperExpression != null) + { + var innerEntityProjection = (EntityProjectionExpression)boundEntityShaperExpression.ValueBufferExpression; + var newInnerEntityProjection = LiftEntityProjectionFromSubquery(innerEntityProjection); + boundEntityShaperExpression = boundEntityShaperExpression.Update(newInnerEntityProjection); + newEntityProjection.AddNavigationBinding(navigation, boundEntityShaperExpression); + } + } + + return newEntityProjection; + } } public Expression AddSingleProjection(ShapedQueryExpression shapedQueryExpression) diff --git a/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsQueryInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsQueryInMemoryTest.cs index 1a290d4432b..47b3314e76e 100644 --- a/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsQueryInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsQueryInMemoryTest.cs @@ -26,5 +26,11 @@ public override void Member_pushdown_chain_3_levels_deep_entity() { base.Member_pushdown_chain_3_levels_deep_entity(); } + + [ConditionalTheory(Skip = "issue #17620")] + public override Task Lift_projection_mapping_when_pushing_down_subquery(bool isAsync) + { + return base.Lift_projection_mapping_when_pushing_down_subquery(isAsync); + } } } diff --git a/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs index 5993c508f69..06610af5479 100644 --- a/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs @@ -9,18 +9,18 @@ using Microsoft.EntityFrameworkCore.TestUtilities; using Xunit; -// ReSharper disable ConvertClosureToMethodGroup -// ReSharper disable PossibleUnintendedReferenceComparison -// ReSharper disable ArgumentsStyleLiteral -// ReSharper disable PossibleMultipleEnumeration -// ReSharper disable UnusedVariable -// ReSharper disable EqualExpressionComparison -// ReSharper disable AccessToDisposedClosure -// ReSharper disable StringStartsWithIsCultureSpecific -// ReSharper disable InconsistentNaming -// ReSharper disable MergeConditionalExpression -// ReSharper disable ReplaceWithSingleCallToSingle -// ReSharper disable ReturnValueOfPureMethodIsNotUsed +// ReSharper disable ConvertClosureToMethodGroup +// ReSharper disable PossibleUnintendedReferenceComparison +// ReSharper disable ArgumentsStyleLiteral +// ReSharper disable PossibleMultipleEnumeration +// ReSharper disable UnusedVariable +// ReSharper disable EqualExpressionComparison +// ReSharper disable AccessToDisposedClosure +// ReSharper disable StringStartsWithIsCultureSpecific +// ReSharper disable InconsistentNaming +// ReSharper disable MergeConditionalExpression +// ReSharper disable ReplaceWithSingleCallToSingle +// ReSharper disable ReturnValueOfPureMethodIsNotUsed // ReSharper disable ConvertToExpressionBodyWhenPossible #pragma warning disable RCS1155 // Use StringComparison when comparing strings. namespace Microsoft.EntityFrameworkCore.Query @@ -5725,5 +5725,29 @@ public virtual void Union_over_entities_with_different_nullability() var result = query.ToList(); } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Lift_projection_mapping_when_pushing_down_subquery(bool isAsync) + { + return AssertQuery( + isAsync, + ss => ss.Set() + .Take(25) + .Select(l1 => new + { + l1.Id, + c1 = l1.OneToMany_Required1.Select(l2 => new { l2.Id }).FirstOrDefault(), + c2 = l1.OneToMany_Required1.Select(l2 => new { l2.Id }) + }), + elementSorter: t => t.Id, + elementAsserter: (e, a) => + { + Assert.Equal(e.Id, a.Id); + // See issue#17287 + Assert.Equal(e.c1?.Id ?? 0, a.c1?.Id); + AssertCollection(e.c2, a.c2, elementSorter: i => i.Id, elementAsserter: (ie, ia) => Assert.Equal(ie.Id, ia.Id)); + }); + } } } diff --git a/test/EFCore.Specification.Tests/Query/ComplexNavigationsWeakQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/ComplexNavigationsWeakQueryTestBase.cs index 4cb29354172..ff076cde1bc 100644 --- a/test/EFCore.Specification.Tests/Query/ComplexNavigationsWeakQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/ComplexNavigationsWeakQueryTestBase.cs @@ -178,5 +178,11 @@ public override Task Include_inside_subquery(bool isAsync) { return base.Include_inside_subquery(isAsync); } + + [ConditionalTheory(Skip = "Issue#18519")] + public override Task Lift_projection_mapping_when_pushing_down_subquery(bool isAsync) + { + return base.Lift_projection_mapping_when_pushing_down_subquery(isAsync); + } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs index cfd2179a3f7..632f24495f5 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs @@ -4375,6 +4375,30 @@ WHERE [l2].[Id] IS NULL ) AS [t]"); } + public override async Task Lift_projection_mapping_when_pushing_down_subquery(bool isAsync) + { + await base.Lift_projection_mapping_when_pushing_down_subquery(isAsync); + + AssertSql( + @"@__p_0='25' + +SELECT [t].[Id], [t1].[Id], [l1].[Id] +FROM ( + SELECT TOP(@__p_0) [l].[Id], [l].[Date], [l].[Name], [l].[OneToMany_Optional_Self_Inverse1Id], [l].[OneToMany_Required_Self_Inverse1Id], [l].[OneToOne_Optional_Self1Id] + FROM [LevelOne] AS [l] +) AS [t] +LEFT JOIN ( + SELECT [t0].[Id], [t0].[OneToMany_Required_Inverse2Id] + FROM ( + SELECT [l0].[Id], [l0].[OneToMany_Required_Inverse2Id], ROW_NUMBER() OVER(PARTITION BY [l0].[OneToMany_Required_Inverse2Id] ORDER BY [l0].[Id]) AS [row] + FROM [LevelTwo] AS [l0] + ) AS [t0] + WHERE [t0].[row] <= 1 +) AS [t1] ON [t].[Id] = [t1].[OneToMany_Required_Inverse2Id] +LEFT JOIN [LevelTwo] AS [l1] ON [t].[Id] = [l1].[OneToMany_Required_Inverse2Id] +ORDER BY [t].[Id], [l1].[Id]"); + } + private void AssertSql(params string[] expected) { Fixture.TestSqlLoggerFactory.AssertBaseline(expected);