Query: Lift pending collections property when applying single result …

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<IProperty, int>)GetProjectionIndex(
-                        (ProjectionBindingExpression)entityShaper.ValueBufferExpression);
-                    var indexMap = new Dictionary<IProperty, int>();
-                    foreach (var keyValuePair in oldIndexMap)
-                    {
-                        indexMap[keyValuePair.Key] = keyValuePair.Value + _offset;
-                    }
+                    case EntityShaperExpression entityShaperExpression:
+                        var oldIndexMap = (IDictionary<IProperty, int>)GetProjectionIndex(
+                            (ProjectionBindingExpression)entityShaperExpression.ValueBufferExpression);
+                        var indexMap = new Dictionary<IProperty, int>();
+                        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),

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);
+        }
     }
 }

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);
+        }
     }
 }

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<Level1>()
                     .Where(w => w.Id == (MaybeScalar<int>(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<Level1>().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<Level1>().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);
+                            }
+                        });
+                });
+        }
     }
 }

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);
     }
 }