Fix to #19825 - Query: incorrectly generating JOIN rather than APPLY …

…for subqueries with outside references to a joined table During translation, when we decide whether we can apply JOIN or APPLY we visit inner SelectExpression looking for references to tables in the outer SelectExpression. Problem was that we were using Contains method, which would not match cases where the outer table was in the form of JoinExpressionBase. Fix is to use ContainsTableReference method which matches different types of tables.
dotnet · Feb 19, 2020 · fd07ee9 · fd07ee9
1 parent 5ae009c
commit fd07ee9
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 6 deletions.
diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
@@ -942,7 +942,7 @@ public Expression ApplyCollectionJoin(
             }
 
             var joinPredicate = TryExtractJoinKey(innerSelectExpression);
-            var containsOuterReference = new SelectExpressionCorrelationFindingExpressionVisitor(Tables)
+            var containsOuterReference = new SelectExpressionCorrelationFindingExpressionVisitor(this)
                 .ContainsOuterReference(innerSelectExpression);
             if (containsOuterReference && joinPredicate != null)
             {
@@ -1230,12 +1230,12 @@ private bool ContainsTableReference(TableExpressionBase table)
 
         private sealed class SelectExpressionCorrelationFindingExpressionVisitor : ExpressionVisitor
         {
-            private readonly IReadOnlyList<TableExpressionBase> _tables;
+            private readonly SelectExpression _outerSelectExpression;
             private bool _containsOuterReference;
 
-            public SelectExpressionCorrelationFindingExpressionVisitor(IReadOnlyList<TableExpressionBase> tables)
+            public SelectExpressionCorrelationFindingExpressionVisitor(SelectExpression outerSelectExpression)
             {
-                _tables = tables;
+                _outerSelectExpression = outerSelectExpression;
             }
 
             public bool ContainsOuterReference(SelectExpression selectExpression)
@@ -1255,7 +1255,7 @@ public override Expression Visit(Expression expression)
                 }
 
                 if (expression is ColumnExpression columnExpression
-                    && _tables.Contains(columnExpression.Table))
+                    && _outerSelectExpression.ContainsTableReference(columnExpression.Table))
                 {
                     _containsOuterReference = true;
 
@@ -1308,7 +1308,7 @@ private void AddJoin(
                 joinPredicate = TryExtractJoinKey(innerSelectExpression);
                 if (joinPredicate != null)
                 {
-                    var containsOuterReference = new SelectExpressionCorrelationFindingExpressionVisitor(Tables)
+                    var containsOuterReference = new SelectExpressionCorrelationFindingExpressionVisitor(this)
                         .ContainsOuterReference(innerSelectExpression);
                     if (containsOuterReference)
                     {

diff --git a/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsWeakQueryInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsWeakQueryInMemoryTest.cs
@@ -71,5 +71,17 @@ public override Task Select_subquery_single_nested_subquery2(bool async)
         {
             return base.Select_subquery_single_nested_subquery2(async);
         }
+
+        [ConditionalTheory(Skip = "issue #19967")]
+        public override Task SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(bool async)
+        {
+            return base.SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(async);
+        }
+
+        [ConditionalTheory(Skip = "issue #19967")]
+        public override Task Nested_SelectMany_correlated_with_join_table_correctly_translated_to_apply(bool async)
+        {
+            return base.Nested_SelectMany_correlated_with_join_table_correctly_translated_to_apply(async);
+        }
     }
 }
diff --git a/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs
@@ -5164,6 +5164,7 @@ public virtual Task Select_subquery_single_nested_subquery(bool async)
                 });
 
         }
+
         [ConditionalTheory]
         [MemberData(nameof(IsAsyncData))]
         public virtual Task Select_subquery_single_nested_subquery2(bool async)
@@ -5197,5 +5198,31 @@ public virtual Task Select_subquery_single_nested_subquery2(bool async)
                         });
                 });
         }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(bool async)
+        {
+            return AssertQuery(
+                async,
+                ss => from l1 in ss.Set<Level1>() 
+                      join l2 in ss.Set<Level2>() on l1.Id equals l2.Level1_Required_Id
+                      join l3 in ss.Set<Level3>() on l2.Id equals l3.Level2_Required_Id
+                      join l4 in ss.Set<Level4>() on l3.Id equals l4.Level3_Required_Id
+                      from other in ss.Set<Level1>().Where(x => x.Id <= l2.Id && x.Name == l4.Name).DefaultIfEmpty()
+                      select l1);
+        }
+
+        [ConditionalTheory(Skip = "issue #19095")]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Nested_SelectMany_correlated_with_join_table_correctly_translated_to_apply(bool async)
+        {
+            return AssertQuery(
+                async,
+                ss => ss.Set<Level1>().SelectMany(
+                l1 => l1.OneToMany_Optional1.DefaultIfEmpty().SelectMany(
+                    l2 => l2.OneToOne_Required_PK2.OneToMany_Optional3.DefaultIfEmpty()
+                    .Select(l4 => new { l1Name = l1.Name, l2Name = l2.OneToOne_Required_PK2.Name, l3Name = l4.OneToOne_Optional_PK_Inverse4.Name }))));
+        }
     }
 }
diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs
@@ -4398,6 +4398,31 @@ FROM [LevelThree] AS [l1]
 ORDER BY [l].[Id], [t1].[Id], [t1].[Id0]");
         }
 
+        public override async Task SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(bool async)
+        {
+            await base.SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(async);
+
+            AssertSql(
+                @"SELECT [l].[Id], [l].[Date], [l].[Name], [l].[OneToMany_Optional_Self_Inverse1Id], [l].[OneToMany_Required_Self_Inverse1Id], [l].[OneToOne_Optional_Self1Id]
+FROM [LevelOne] AS [l]
+INNER JOIN [LevelTwo] AS [l0] ON [l].[Id] = [l0].[Level1_Required_Id]
+INNER JOIN [LevelThree] AS [l1] ON [l0].[Id] = [l1].[Level2_Required_Id]
+INNER JOIN [LevelFour] AS [l2] ON [l1].[Id] = [l2].[Level3_Required_Id]
+OUTER APPLY (
+    SELECT [l3].[Id], [l3].[Date], [l3].[Name], [l3].[OneToMany_Optional_Self_Inverse1Id], [l3].[OneToMany_Required_Self_Inverse1Id], [l3].[OneToOne_Optional_Self1Id]
+    FROM [LevelOne] AS [l3]
+    WHERE ([l3].[Id] <= [l0].[Id]) AND (([l2].[Name] = [l3].[Name]) OR ([l2].[Name] IS NULL AND [l3].[Name] IS NULL))
+) AS [t]");
+        }
+
+        public override async Task Nested_SelectMany_correlated_with_join_table_correctly_translated_to_apply(bool async)
+        {
+            await base.Nested_SelectMany_correlated_with_join_table_correctly_translated_to_apply(async);
+
+            AssertSql(
+                @"");
+        }
+
         private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
     }
 }
diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/ComplexNavigationsQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/ComplexNavigationsQuerySqliteTest.cs
@@ -30,5 +30,9 @@ public override Task Include_inside_subquery(bool async)
         {
             return base.Include_inside_subquery(async);
         }
+
+        // Sqlite does not support cross/outer apply
+        public override Task SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(bool async) => null;
+        public override Task Nested_SelectMany_correlated_with_join_table_correctly_translated_to_apply(bool async) => null;
     }
 }
diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/ComplexNavigationsWeakQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/ComplexNavigationsWeakQuerySqliteTest.cs
@@ -25,5 +25,9 @@ public override Task Project_collection_navigation_nested_with_take(bool async)
         {
             return base.Project_collection_navigation_nested_with_take(async);
         }
+
+        // Sqlite does not support cross/outer apply
+        public override Task SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(bool async) => null;
+        public override Task Nested_SelectMany_correlated_with_join_table_correctly_translated_to_apply(bool async) => null;
     }
 }