Fix to #27072 - Suboptimal SQL generation for query with optional nav…

…igation, its collection navigation and lateral join

When expanding collection navigation we added a check to filter out rows for which parent value is null and only afterwards match inner and outer keys. This could happen when chaining collection navigation after optional reference. Problem was that during translation we try to convert the collection subquery to a join and if that succeeds we don't need the null check anymore - joins key comparison doesn't match nulls. However, we also remove the null check when we are not able to convert to JOIN and use APPLY instead.

Fix is to save the original predicate before we try to extract the join predicate and then for APPLY case re-apply the original predicate and only use the extracted join predicate for the JOIN case.

src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs

@@ -2104,6 +2104,10 @@ private void AddJoin(
                     innerSelectExpression.Limit = null;
                     innerSelectExpression.Offset = null;
 
+                    var originalInnerSelectPredicate = innerSelectExpression.GroupBy.Count > 0
+                        ? innerSelectExpression.Having
+                        : innerSelectExpression.Predicate;
+
                     joinPredicate = TryExtractJoinKey(this, innerSelectExpression, allowNonEquality: limit == null && offset == null);
                     if (joinPredicate != null)
                     {
@@ -2171,7 +2175,18 @@ private void AddJoin(
                             return;
                         }
 
-                        innerSelectExpression.ApplyPredicate(joinPredicate);
+                        if (!AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue27072", out var enabled27072) || !enabled27072)
+                        {
+                            if (originalInnerSelectPredicate != null)
+                            {
+                                innerSelectExpression.Predicate = originalInnerSelectPredicate;
+                            }
+                        }
+                        else
+                        {
+                            innerSelectExpression.ApplyPredicate(joinPredicate);
+                        }
+
                         joinPredicate = null;
                     }
 

test/EFCore.Specification.Tests/Query/ComplexNavigationsCollectionsQueryTestBase.cs

@@ -2584,5 +2584,33 @@ public virtual Task Complex_query_issue_21665(bool async)
                         });
                 });
         }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Projecting_collection_after_optional_reference_correlated_with_parent(bool async)
+            => AssertQuery(
+                async,
+                ss => ss.Set<Level1>().Select(l1 => new
+                {
+                    Id = l1.Id,
+                    Collection = l1.OneToOne_Optional_FK1.OneToMany_Optional2.Select(l3 => new { ChildId = l3.Id, ParentName = l1.OneToOne_Optional_FK1.Name })
+                }),
+
+                ss => ss.Set<Level1>().Select(l1 => new
+                {
+                    Id = l1.Id,
+                    Collection = l1.Maybe(x => x.OneToOne_Optional_FK1.Maybe(xx => xx.OneToMany_Optional2.Select(l3 => new { ChildId = (int)l3.MaybeScalar(xxx => xxx.Id), ParentName = l1.OneToOne_Optional_FK1.Maybe(xxx => xxx.Name) })))
+                }),
+
+                elementSorter: e => e.Id,
+                elementAsserter: (e, a) =>
+                {
+                    Assert.Equal(e.Id, a.Id);
+
+                    if (!(e.Collection == null && a.Collection != null && a.Collection.Count() == 0))
+                    {
+                        AssertCollection(e.Collection, a.Collection, elementSorter: ee => ee.ChildId);
+                    }
+                });
     }
 }

test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsCollectionsQuerySqlServerTest.cs

@@ -2242,6 +2242,22 @@ OFFSET 1 ROWS FETCH NEXT 5 ROWS ONLY
 ORDER BY [t].[Name], [t].[Id], [t].[Id0], [t0].[Name], [t0].[Id]");
         }
 
+        public override async Task Projecting_collection_after_optional_reference_correlated_with_parent(bool async)
+        {
+            await base.Projecting_collection_after_optional_reference_correlated_with_parent(async);
+
+            AssertSql(
+                @"SELECT [l].[Id], [l0].[Id], [t].[ChildId], [t].[ParentName]
+FROM [LevelOne] AS [l]
+LEFT JOIN [LevelTwo] AS [l0] ON [l].[Id] = [l0].[Level1_Optional_Id]
+OUTER APPLY (
+    SELECT [l1].[Id] AS [ChildId], [l0].[Name] AS [ParentName]
+    FROM [LevelThree] AS [l1]
+    WHERE ([l0].[Id] IS NOT NULL) AND ([l0].[Id] = [l1].[OneToMany_Optional_Inverse3Id])
+) AS [t]
+ORDER BY [l].[Id], [l0].[Id]");
+        }
+
         private void AssertSql(params string[] expected)
             => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
     }

test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs

@@ -4121,7 +4121,7 @@ FROM [Gears] AS [g]
 OUTER APPLY (
     SELECT [g0].[FullName] AS [ReportName], [g0].[Nickname], [g0].[SquadId]
     FROM [Gears] AS [g0]
-    WHERE ([g].[FullName] <> N'Foo') AND (([g].[Nickname] = [g0].[LeaderNickname]) AND ([g].[SquadId] = [g0].[LeaderSquadId]))
+    WHERE (([g].[Nickname] = [g0].[LeaderNickname]) AND ([g].[SquadId] = [g0].[LeaderSquadId])) AND ([g].[FullName] <> N'Foo')
 ) AS [t]
 WHERE [g].[Discriminator] = N'Officer'
 ORDER BY [g].[Nickname], [g].[SquadId], [t].[Nickname]");
@@ -4140,7 +4140,7 @@ FROM [Gears] AS [g0]
     OUTER APPLY (
         SELECT [w].[Name], [g0].[Nickname], [w].[Id]
         FROM [Weapons] AS [w]
-        WHERE (([w].[Name] <> N'Bar') OR ([w].[Name] IS NULL)) AND ([g0].[FullName] = [w].[OwnerFullName])
+        WHERE ([g0].[FullName] = [w].[OwnerFullName]) AND (([w].[Name] <> N'Bar') OR ([w].[Name] IS NULL))
     ) AS [t]
     WHERE [g0].[FullName] <> N'Foo'
 ) AS [t0] ON ([g].[Nickname] = [t0].[LeaderNickname]) AND ([g].[SquadId] = [t0].[LeaderSquadId])
@@ -4163,7 +4163,7 @@ LEFT JOIN (
         FROM [Weapons] AS [w]
         WHERE ([w].[Name] <> N'Bar') OR ([w].[Name] IS NULL)
     ) AS [t] ON [g0].[FullName] = [t].[OwnerFullName]
-    WHERE ([g0].[FullName] <> N'Foo') AND (([g].[Nickname] = [g0].[LeaderNickname]) AND ([g].[SquadId] = [g0].[LeaderSquadId]))
+    WHERE (([g].[Nickname] = [g0].[LeaderNickname]) AND ([g].[SquadId] = [g0].[LeaderSquadId])) AND ([g0].[FullName] <> N'Foo')
 ) AS [t0]
 WHERE [g].[Discriminator] = N'Officer'
 ORDER BY [g].[Nickname], [g].[SquadId], [t0].[Nickname], [t0].[SquadId]");
@@ -7939,7 +7939,7 @@ FROM [Gears] AS [g]
     OUTER APPLY (
         SELECT [t].[Id], [t].[Name], [t0].[Nickname], [t0].[FullName], [t0].[HasSoulPatch], [w].[Id] AS [Id0]
         FROM [Weapons] AS [w]
-        WHERE [t0].[FullName] = [w].[OwnerFullName]
+        WHERE [w].[OwnerFullName] = [t0].[FullName]
     ) AS [t2]
 ) AS [t1]
 ORDER BY [t].[Id], [t1].[Nickname], [t1].[FullName], [t1].[HasSoulPatch]");

test/EFCore.SqlServer.FunctionalTests/Query/NorthwindIncludeQuerySqlServerTest.cs

@@ -537,7 +537,7 @@ FROM [Customers] AS [c]
 CROSS APPLY (
     SELECT TOP(5) [o].[OrderID]
     FROM [Orders] AS [o]
-    WHERE [c].[CustomerID] = [o].[CustomerID]
+    WHERE [o].[CustomerID] = [c].[CustomerID]
     ORDER BY [c].[CustomerID]
 ) AS [t]
 LEFT JOIN [Orders] AS [o0] ON [c].[CustomerID] = [o0].[CustomerID]
@@ -555,7 +555,7 @@ FROM [Customers] AS [c]
 OUTER APPLY (
     SELECT TOP(5) [o].[OrderID]
     FROM [Orders] AS [o]
-    WHERE [c].[CustomerID] = [o].[CustomerID]
+    WHERE [o].[CustomerID] = [c].[CustomerID]
     ORDER BY [c].[CustomerID]
 ) AS [t]
 LEFT JOIN [Orders] AS [o0] ON [c].[CustomerID] = [o0].[CustomerID]

test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSelectQuerySqlServerTest.cs

@@ -1,11 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System;
 using System.Threading.Tasks;
-using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.TestUtilities;
-using Xunit;
 using Xunit.Abstractions;
 
 namespace Microsoft.EntityFrameworkCore.Query
@@ -1604,7 +1601,7 @@ FROM [Orders] AS [o]
 OUTER APPLY (
     SELECT [t].[CustomerID], [o0].[OrderID], [o0].[OrderDate]
     FROM [Orders] AS [o0]
-    WHERE [o0].[OrderID] IN (10248, 10249, 10250) AND (([t].[CustomerID] = [o0].[CustomerID]) OR (([t].[CustomerID] IS NULL) AND ([o0].[CustomerID] IS NULL)))
+    WHERE (([t].[CustomerID] IS NOT NULL) AND ([t].[CustomerID] = [o0].[CustomerID])) AND [o0].[OrderID] IN (10248, 10249, 10250)
 ) AS [t0]
 ORDER BY [t].[CustomerID], [t0].[OrderID]");
         }
@@ -1622,7 +1619,7 @@ FROM [Orders] AS [o]
 OUTER APPLY (
     SELECT [t].[OrderID] AS [Outer], [o0].[OrderID] AS [Inner], [o0].[OrderDate]
     FROM [Orders] AS [o0]
-    WHERE [o0].[OrderID] IN (10248, 10249, 10250) AND ([t].[OrderID] = [o0].[OrderID])
+    WHERE ([o0].[OrderID] = [t].[OrderID]) AND [o0].[OrderID] IN (10248, 10249, 10250)
 ) AS [t0]
 ORDER BY [t].[OrderID]");
         }
@@ -1640,7 +1637,7 @@ FROM [Orders] AS [o]
 OUTER APPLY (
     SELECT [t].[OrderDate] AS [Outer1], [t].[CustomerID] AS [Outer2], [o0].[OrderID] AS [Inner], [o0].[OrderDate]
     FROM [Orders] AS [o0]
-    WHERE [o0].[OrderID] IN (10248, 10249, 10250) AND (([t].[CustomerID] = [o0].[CustomerID]) OR (([t].[CustomerID] IS NULL) AND ([o0].[CustomerID] IS NULL)))
+    WHERE (([o0].[CustomerID] = [t].[CustomerID]) OR (([o0].[CustomerID] IS NULL) AND ([t].[CustomerID] IS NULL))) AND [o0].[OrderID] IN (10248, 10249, 10250)
 ) AS [t0]
 ORDER BY [t].[OrderDate], [t].[CustomerID]");
         }
@@ -1677,7 +1674,7 @@ FROM [Orders] AS [o]
 OUTER APPLY (
     SELECT [t].[OrderID] AS [Outer], [o0].[OrderID] AS [Inner], [o0].[OrderDate]
     FROM [Orders] AS [o0]
-    WHERE [o0].[OrderID] IN (10248, 10249, 10250) AND ([t].[OrderID] = [o0].[OrderID])
+    WHERE ([o0].[OrderID] = [t].[OrderID]) AND [o0].[OrderID] IN (10248, 10249, 10250)
 ) AS [t0]
 ORDER BY [t].[OrderID]");
         }
@@ -1697,7 +1694,7 @@ OUTER APPLY (
         FROM [Orders] AS [o0]
         WHERE [o].[CustomerID] = [c].[CustomerID]
     ) AS [t]
-    WHERE (DATEPART(year, [o].[OrderDate]) = 1997) AND ([c].[CustomerID] = [o].[CustomerID])
+    WHERE ([o].[CustomerID] = [c].[CustomerID]) AND (DATEPART(year, [o].[OrderDate]) = 1997)
 ) AS [t0]
 WHERE [c].[City] = N'London'
 ORDER BY [c].[CustomerID], [t0].[OrderID], [t0].[OrderID00]");

test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSplitIncludeQuerySqlServerTest.cs

@@ -686,7 +686,7 @@ FROM [Customers] AS [c]
 CROSS APPLY (
     SELECT TOP(5) [o].[OrderID]
     FROM [Orders] AS [o]
-    WHERE [c].[CustomerID] = [o].[CustomerID]
+    WHERE [o].[CustomerID] = [c].[CustomerID]
     ORDER BY [c].[CustomerID]
 ) AS [t]
 WHERE [c].[CustomerID] LIKE N'F%'
@@ -697,7 +697,7 @@ FROM [Customers] AS [c]
 CROSS APPLY (
     SELECT TOP(5) [o].[OrderID]
     FROM [Orders] AS [o]
-    WHERE [c].[CustomerID] = [o].[CustomerID]
+    WHERE [o].[CustomerID] = [c].[CustomerID]
     ORDER BY [c].[CustomerID]
 ) AS [t]
 INNER JOIN [Orders] AS [o0] ON [c].[CustomerID] = [o0].[CustomerID]
@@ -715,7 +715,7 @@ FROM [Customers] AS [c]
 OUTER APPLY (
     SELECT TOP(5) [o].[OrderID]
     FROM [Orders] AS [o]
-    WHERE [c].[CustomerID] = [o].[CustomerID]
+    WHERE [o].[CustomerID] = [c].[CustomerID]
     ORDER BY [c].[CustomerID]
 ) AS [t]
 WHERE [c].[CustomerID] LIKE N'F%'
@@ -726,7 +726,7 @@ FROM [Customers] AS [c]
 OUTER APPLY (
     SELECT TOP(5) [o].[OrderID]
     FROM [Orders] AS [o]
-    WHERE [c].[CustomerID] = [o].[CustomerID]
+    WHERE [o].[CustomerID] = [c].[CustomerID]
     ORDER BY [c].[CustomerID]
 ) AS [t]
 INNER JOIN [Orders] AS [o0] ON [c].[CustomerID] = [o0].[CustomerID]

test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs

@@ -5607,10 +5607,10 @@ OUTER APPLY (
     SELECT [a1].[Id], [a1].[ActivityTypeId], [a1].[CompetitionSeasonId], [a1].[Points], [c0].[Id] AS [Id0]
     FROM [ActivityTypePoints12456] AS [a1]
     INNER JOIN [CompetitionSeasons] AS [c0] ON [a1].[CompetitionSeasonId] = [c0].[Id]
-    WHERE ([c0].[Id] = (
+    WHERE ([a0].[Id] = [a1].[ActivityTypeId]) AND ([c0].[Id] = (
         SELECT TOP(1) [c1].[Id]
         FROM [CompetitionSeasons] AS [c1]
-        WHERE ([c1].[StartDate] <= [a].[DateTime]) AND ([a].[DateTime] < [c1].[EndDate]))) AND ([a0].[Id] = [a1].[ActivityTypeId])
+        WHERE ([c1].[StartDate] <= [a].[DateTime]) AND ([a].[DateTime] < [c1].[EndDate])))
 ) AS [t]
 ORDER BY [a].[Id], [a0].[Id], [t].[Id]");
             }

test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs

@@ -4920,7 +4920,7 @@ LEFT JOIN [Officers] AS [o] ON ([g].[Nickname] = [o].[Nickname]) AND ([g].[Squad
 OUTER APPLY (
     SELECT [g0].[FullName] AS [ReportName], [g0].[Nickname], [g0].[SquadId]
     FROM [Gears] AS [g0]
-    WHERE ([g].[FullName] <> N'Foo') AND (([g].[Nickname] = [g0].[LeaderNickname]) AND ([g].[SquadId] = [g0].[LeaderSquadId]))
+    WHERE (([g].[Nickname] = [g0].[LeaderNickname]) AND ([g].[SquadId] = [g0].[LeaderSquadId])) AND ([g].[FullName] <> N'Foo')
 ) AS [t]
 WHERE [o].[Nickname] IS NOT NULL
 ORDER BY [g].[Nickname], [g].[SquadId], [t].[Nickname]");
@@ -4940,7 +4940,7 @@ FROM [Gears] AS [g0]
     OUTER APPLY (
         SELECT [w].[Name], [g0].[Nickname], [w].[Id]
         FROM [Weapons] AS [w]
-        WHERE (([w].[Name] <> N'Bar') OR ([w].[Name] IS NULL)) AND ([g0].[FullName] = [w].[OwnerFullName])
+        WHERE ([g0].[FullName] = [w].[OwnerFullName]) AND (([w].[Name] <> N'Bar') OR ([w].[Name] IS NULL))
     ) AS [t]
     WHERE [g0].[FullName] <> N'Foo'
 ) AS [t0] ON ([g].[Nickname] = [t0].[LeaderNickname]) AND ([g].[SquadId] = [t0].[LeaderSquadId])
@@ -4964,7 +4964,7 @@ LEFT JOIN (
         FROM [Weapons] AS [w]
         WHERE ([w].[Name] <> N'Bar') OR ([w].[Name] IS NULL)
     ) AS [t] ON [g0].[FullName] = [t].[OwnerFullName]
-    WHERE ([g0].[FullName] <> N'Foo') AND (([g].[Nickname] = [g0].[LeaderNickname]) AND ([g].[SquadId] = [g0].[LeaderSquadId]))
+    WHERE (([g].[Nickname] = [g0].[LeaderNickname]) AND ([g].[SquadId] = [g0].[LeaderSquadId])) AND ([g0].[FullName] <> N'Foo')
 ) AS [t0]
 WHERE [o].[Nickname] IS NOT NULL
 ORDER BY [g].[Nickname], [g].[SquadId], [t0].[Nickname], [t0].[SquadId]");

test/EFCore.Sqlite.FunctionalTests/Query/ComplexNavigationsCollectionsQuerySqliteTest.cs

@@ -141,5 +141,11 @@ public override async Task Complex_query_issue_21665(bool async)
                 SqliteStrings.ApplyNotSupported,
                 (await Assert.ThrowsAsync<InvalidOperationException>(
                     () => base.Complex_query_issue_21665(async))).Message);
+
+        public override async Task Projecting_collection_after_optional_reference_correlated_with_parent(bool async)
+            => Assert.Equal(
+                SqliteStrings.ApplyNotSupported,
+                (await Assert.ThrowsAsync<InvalidOperationException>(
+                    () => base.Projecting_collection_after_optional_reference_correlated_with_parent(async))).Message);
     }
 }

test/EFCore.Sqlite.FunctionalTests/Query/ComplexNavigationsCollectionsSharedTypeQuerySqliteTest.cs

@@ -125,5 +125,11 @@ public override async Task SelectMany_with_predicate_and_DefaultIfEmpty_projecti
                 SqliteStrings.ApplyNotSupported,
                 (await Assert.ThrowsAsync<InvalidOperationException>(
                     () => base.SelectMany_with_predicate_and_DefaultIfEmpty_projecting_root_collection_element_and_another_collection(async))).Message);
+
+        public override async Task Projecting_collection_after_optional_reference_correlated_with_parent(bool async)
+            => Assert.Equal(
+                SqliteStrings.ApplyNotSupported,
+                (await Assert.ThrowsAsync<InvalidOperationException>(
+                    () => base.Projecting_collection_after_optional_reference_correlated_with_parent(async))).Message);
     }
 }

test/EFCore.Sqlite.FunctionalTests/Query/ComplexNavigationsCollectionsSplitQuerySqliteTest.cs

@@ -117,5 +117,11 @@ public override async Task Complex_query_issue_21665(bool async)
                 SqliteStrings.ApplyNotSupported,
                 (await Assert.ThrowsAsync<InvalidOperationException>(
                     () => base.Complex_query_issue_21665(async))).Message);
+
+        public override async Task Projecting_collection_after_optional_reference_correlated_with_parent(bool async)
+            => Assert.Equal(
+                SqliteStrings.ApplyNotSupported,
+                (await Assert.ThrowsAsync<InvalidOperationException>(
+                    () => base.Projecting_collection_after_optional_reference_correlated_with_parent(async))).Message);
     }
 }

test/EFCore.Sqlite.FunctionalTests/Query/ComplexNavigationsCollectionsSplitSharedTypeQuerySqliteTest.cs

@@ -101,5 +101,11 @@ public override async Task SelectMany_with_predicate_and_DefaultIfEmpty_projecti
                 SqliteStrings.ApplyNotSupported,
                 (await Assert.ThrowsAsync<InvalidOperationException>(
                     () => base.SelectMany_with_predicate_and_DefaultIfEmpty_projecting_root_collection_element_and_another_collection(async))).Message);
+
+        public override async Task Projecting_collection_after_optional_reference_correlated_with_parent(bool async)
+            => Assert.Equal(
+                SqliteStrings.ApplyNotSupported,
+                (await Assert.ThrowsAsync<InvalidOperationException>(
+                    () => base.Projecting_collection_after_optional_reference_correlated_with_parent(async))).Message);
     }
 }