diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index 83891d3f1bc..70497e09b13 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -871,6 +871,7 @@ public Expression AddSingleProjection([NotNull] ShapedQueryExpression shapedQuer innerSelectExpression.ApplyProjection(); var projectionCount = innerSelectExpression.Projection.Count; + var pendingCollectionOffset = _pendingCollections.Count; AddOuterApply(innerSelectExpression, null); // Joined SelectExpression may different based on left join or outer apply @@ -885,7 +886,8 @@ public Expression AddSingleProjection([NotNull] ShapedQueryExpression shapedQuer AddToProjection(MakeNullable(addedSelectExperssion.GenerateOuterColumn(projection.Expression))); } - return new ShaperRemappingExpressionVisitor(this, innerSelectExpression, indexOffset) + // We move pendingCollectionOffset if one was lifted from inner. + return new ShaperRemappingExpressionVisitor(this, innerSelectExpression, indexOffset, pendingCollectionOffset) .Visit(shaperExpression); static Expression RemoveConvert(Expression expression) @@ -980,7 +982,9 @@ public Expression ApplyCollectionJoin( AppendOrdering(new OrderingExpression(updatedColumn, ascending: true)); } - var shaperRemapper = new ShaperRemappingExpressionVisitor(this, innerSelectExpression, indexOffset); + // Inner should not have pendingCollection since we apply them first. + // Shaper should not have CollectionShaperExpression as any collection would get converted to RelationalCollectionShaperExpression. + var shaperRemapper = new ShaperRemappingExpressionVisitor(this, innerSelectExpression, indexOffset, pendingCollectionOffset: 0); innerShaper = shaperRemapper.Visit(innerShaper); selfIdentifier = shaperRemapper.Visit(selfIdentifier); @@ -1014,43 +1018,54 @@ private sealed class ShaperRemappingExpressionVisitor : ExpressionVisitor { private readonly SelectExpression _queryExpression; private readonly SelectExpression _innerSelectExpression; - private readonly int _offset; + private readonly int _projectionOffset; + private readonly int _pendingCollectionOffset; - public ShaperRemappingExpressionVisitor(SelectExpression queryExpression, SelectExpression innerSelectExpression, int offset) + public ShaperRemappingExpressionVisitor( + SelectExpression queryExpression, SelectExpression innerSelectExpression, int projectionOffset, int pendingCollectionOffset) { _queryExpression = queryExpression; _innerSelectExpression = innerSelectExpression; - _offset = offset; + _projectionOffset = projectionOffset; + _pendingCollectionOffset = pendingCollectionOffset; } protected override Expression VisitExtension(Expression extensionExpression) { Check.NotNull(extensionExpression, nameof(extensionExpression)); - if (extensionExpression is ProjectionBindingExpression projectionBindingExpression) + switch (extensionExpression) { - var oldIndex = (int)GetProjectionIndex(projectionBindingExpression); + case ProjectionBindingExpression projectionBindingExpression: + var oldIndex = (int)GetProjectionIndex(projectionBindingExpression); - return new ProjectionBindingExpression(_queryExpression, oldIndex + _offset, projectionBindingExpression.Type); - } + return new ProjectionBindingExpression( + _queryExpression, oldIndex + _projectionOffset, projectionBindingExpression.Type); - if (extensionExpression is EntityShaperExpression entityShaper) - { - var oldIndexMap = (IDictionary)GetProjectionIndex( - (ProjectionBindingExpression)entityShaper.ValueBufferExpression); - var indexMap = new Dictionary(); - foreach (var keyValuePair in oldIndexMap) - { - indexMap[keyValuePair.Key] = keyValuePair.Value + _offset; - } + case EntityShaperExpression entityShaperExpression: + var oldIndexMap = (IDictionary)GetProjectionIndex( + (ProjectionBindingExpression)entityShaperExpression.ValueBufferExpression); + var indexMap = new Dictionary(); + foreach (var keyValuePair in oldIndexMap) + { + indexMap[keyValuePair.Key] = keyValuePair.Value + _projectionOffset; + } - return new EntityShaperExpression( - entityShaper.EntityType, - new ProjectionBindingExpression(_queryExpression, indexMap), - nullable: true); - } + return new EntityShaperExpression( + entityShaperExpression.EntityType, + new ProjectionBindingExpression(_queryExpression, indexMap), + nullable: true); + + case CollectionShaperExpression collectionShaperExpression: + var collectionIndex = (int)GetProjectionIndex((ProjectionBindingExpression)collectionShaperExpression.Projection); + + return collectionShaperExpression.Update( + new ProjectionBindingExpression(_queryExpression, collectionIndex + _pendingCollectionOffset, typeof(object)), + collectionShaperExpression.InnerShaper); - return base.VisitExtension(extensionExpression); + default: + return base.VisitExtension(extensionExpression); + } } private object GetProjectionIndex(ProjectionBindingExpression projectionBindingExpression) @@ -1363,6 +1378,9 @@ private void AddJoin( } var innerTable = innerSelectExpression.Tables.Single(); + // Copy over pending collection if in join else that info would be lost. + _pendingCollections.AddRange(innerSelectExpression._pendingCollections); + var joinTable = joinType switch { JoinType.InnerJoin => new InnerJoinExpression(innerTable, joinPredicate), diff --git a/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsQueryInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsQueryInMemoryTest.cs index c2acdbb3e86..eca74b677fe 100644 --- a/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsQueryInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsQueryInMemoryTest.cs @@ -32,5 +32,17 @@ public override Task Lift_projection_mapping_when_pushing_down_subquery(bool asy { return base.Lift_projection_mapping_when_pushing_down_subquery(async); } + + [ConditionalTheory(Skip = "issue #19344")] + public override Task Select_subquery_single_nested_subquery(bool async) + { + return base.Select_subquery_single_nested_subquery(async); + } + + [ConditionalTheory(Skip = "issue #19344")] + public override Task Select_subquery_single_nested_subquery2(bool async) + { + return base.Select_subquery_single_nested_subquery2(async); + } } } diff --git a/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsWeakQueryInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsWeakQueryInMemoryTest.cs index d41594d9e82..6326c059a61 100644 --- a/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsWeakQueryInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsWeakQueryInMemoryTest.cs @@ -59,5 +59,17 @@ public override Task OrderBy_collection_count_ThenBy_reference_navigation(bool a { return base.OrderBy_collection_count_ThenBy_reference_navigation(async); } + + [ConditionalTheory(Skip = "issue #19344")] + public override Task Select_subquery_single_nested_subquery(bool async) + { + return base.Select_subquery_single_nested_subquery(async); + } + + [ConditionalTheory(Skip = "issue #19344")] + public override Task Select_subquery_single_nested_subquery2(bool async) + { + return base.Select_subquery_single_nested_subquery2(async); + } } } diff --git a/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs index 59705f205e3..8aec7e573ae 100644 --- a/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs @@ -5616,5 +5616,66 @@ public virtual Task Select_with_joined_where_clause_cast_using_as(bool async) ss => ss.Set() .Where(w => w.Id == (MaybeScalar(w.OneToOne_Optional_FK1, () => w.OneToOne_Optional_FK1.Id) as int?))); } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Select_subquery_single_nested_subquery(bool async) + { + return AssertQuery( + async, + ss => ss.Set().OrderBy(l1 => l1.Id).Select(l1 => new + { + Level2 = l1.OneToMany_Optional1.OrderBy(l2 => l2.Id).Select(l2 => new + { + Level3s = l2.OneToMany_Optional2.OrderBy(l3 => l3.Id).Select(l3 => new { l3.Id }).ToList() + }).FirstOrDefault() + }), + assertOrder: true, + elementAsserter: (e, a) => + { + if (e.Level2 == null) + { + Assert.Null(a.Level2); + } + else + { + AssertCollection(e.Level2.Level3s, a.Level2.Level3s, ordered: true); + } + }); + + } + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Select_subquery_single_nested_subquery2(bool async) + { + return AssertQuery( + async, + ss => ss.Set().OrderBy(l1 => l1.Id).Select(l1 => new + { + Level2s = l1.OneToMany_Optional1.OrderBy(l2 => l2.Id).Select(l2 => new + { + Level3 = l2.OneToMany_Optional2.OrderBy(l3 => l3.Id).Select(l3 => new + { + Level4s = l3.OneToMany_Optional3.OrderBy(l4 => l4.Id).Select(l4 => new { l4.Id }).ToList() + }).FirstOrDefault() + }) + }), + assertOrder: true, + elementAsserter: (e, a) => + { + AssertCollection(e.Level2s, a.Level2s, ordered: true, elementAsserter: + (e2, a2) => + { + if (e2.Level3 == null) + { + Assert.Null(a2.Level3); + } + else + { + AssertCollection(e2.Level3.Level4s, a2.Level3.Level4s, ordered: true); + } + }); + }); + } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs index 8dfc81f496f..525853ef2e3 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs @@ -4363,6 +4363,48 @@ FROM [LevelOne] AS [l] WHERE [l].[Id] = [l0].[Id]"); } + public override async Task Select_subquery_single_nested_subquery(bool async) + { + await base.Select_subquery_single_nested_subquery(async); + + AssertSql( + @"SELECT [t0].[c], [l].[Id], [l1].[Id] +FROM [LevelOne] AS [l] +LEFT JOIN ( + SELECT [t].[c], [t].[Id], [t].[OneToMany_Optional_Inverse2Id] + FROM ( + SELECT 1 AS [c], [l0].[Id], [l0].[OneToMany_Optional_Inverse2Id], ROW_NUMBER() OVER(PARTITION BY [l0].[OneToMany_Optional_Inverse2Id] ORDER BY [l0].[Id]) AS [row] + FROM [LevelTwo] AS [l0] + ) AS [t] + WHERE [t].[row] <= 1 +) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id] +LEFT JOIN [LevelThree] AS [l1] ON [t0].[Id] = [l1].[OneToMany_Optional_Inverse3Id] +ORDER BY [l].[Id], [l1].[Id]"); + } + + public override async Task Select_subquery_single_nested_subquery2(bool async) + { + await base.Select_subquery_single_nested_subquery2(async); + + AssertSql( + @"SELECT [l].[Id], [t1].[c], [t1].[Id], [t1].[Id0] +FROM [LevelOne] AS [l] +LEFT JOIN ( + SELECT [t0].[c], [l0].[Id], [l2].[Id] AS [Id0], [l0].[OneToMany_Optional_Inverse2Id] + FROM [LevelTwo] AS [l0] + LEFT JOIN ( + SELECT [t].[c], [t].[Id], [t].[OneToMany_Optional_Inverse3Id] + FROM ( + SELECT 1 AS [c], [l1].[Id], [l1].[OneToMany_Optional_Inverse3Id], ROW_NUMBER() OVER(PARTITION BY [l1].[OneToMany_Optional_Inverse3Id] ORDER BY [l1].[Id]) AS [row] + FROM [LevelThree] AS [l1] + ) AS [t] + WHERE [t].[row] <= 1 + ) AS [t0] ON [l0].[Id] = [t0].[OneToMany_Optional_Inverse3Id] + LEFT JOIN [LevelFour] AS [l2] ON [t0].[Id] = [l2].[OneToMany_Optional_Inverse4Id] +) AS [t1] ON [l].[Id] = [t1].[OneToMany_Optional_Inverse2Id] +ORDER BY [l].[Id], [t1].[Id], [t1].[Id0]"); + } + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); } }