[release/8.0] Fix to #32217 - Nav expansion visitor does not visit the Contains item argument (and possibly other non-lambda arguments)

src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs

@@ -15,6 +15,24 @@ namespace Microsoft.EntityFrameworkCore.Query.Internal;
 /// </summary>
 public partial class NavigationExpandingExpressionVisitor : ExpressionVisitor
 {
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public static readonly bool UseOldBehavior32217 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32217", out var enabled32217) && enabled32217;
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public static readonly bool UseOldBehavior32312 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32312", out var enabled32312) && enabled32312;
+
     private static readonly PropertyInfo QueryContextContextPropertyInfo
         = typeof(QueryContext).GetTypeInfo().GetDeclaredProperty(nameof(QueryContext.Context))!;
 
@@ -921,6 +939,11 @@ private Expression ProcessContains(NavigationExpansionExpression source, Express
         source = (NavigationExpansionExpression)_pendingSelectorExpandingExpressionVisitor.Visit(source);
         var queryable = Reduce(source);
 
+        if (!UseOldBehavior32217)
+        {
+            item = Visit(item);
+        }
+
         return Expression.Call(QueryableMethods.Contains.MakeGenericMethod(queryable.Type.GetSequenceType()), queryable, item);
     }
 
@@ -959,11 +982,16 @@ private NavigationExpansionExpression ProcessDistinct(NavigationExpansionExpress
         return new NavigationExpansionExpression(result, navigationTree, navigationTree, parameterName);
     }
 
-    private static NavigationExpansionExpression ProcessSkipTake(
+    private NavigationExpansionExpression ProcessSkipTake(
         NavigationExpansionExpression source,
         MethodInfo genericMethod,
         Expression count)
     {
+        if (!UseOldBehavior32312)
+        {
+            count = Visit(count);
+        }
+
         source.UpdateSource(Expression.Call(genericMethod.MakeGenericMethod(source.SourceElementType), source.Source, count));
 
         return source;
@@ -1002,6 +1030,11 @@ private NavigationExpansionExpression ProcessElementAt(
             source.ApplySelector(Expression.Convert(source.PendingSelector, returnType));
         }
 
+        if (!UseOldBehavior32312)
+        {
+            index = Visit(index);
+        }
+
         source.ConvertToSingleResult(genericMethod, index);
 
         return source;
@@ -1560,11 +1593,16 @@ private GroupByNavigationExpansionExpression ProcessOrderByThenBy(
         return new NavigationExpansionExpression(newSource, navigationTree, navigationTree, parameterName);
     }
 
-    private static GroupByNavigationExpansionExpression ProcessSkipTake(
+    private GroupByNavigationExpansionExpression ProcessSkipTake(
         GroupByNavigationExpansionExpression groupBySource,
         MethodInfo genericMethod,
         Expression count)
     {
+        if (!UseOldBehavior32312)
+        {
+            count = Visit(count);
+        }
+
         groupBySource.UpdateSource(
             Expression.Call(genericMethod.MakeGenericMethod(groupBySource.SourceElementType), groupBySource.Source, count));
 

test/EFCore.InMemory.FunctionalTests/Query/GearsOfWarQueryInMemoryTest.cs

@@ -136,4 +136,7 @@ public override Task Where_subquery_with_ElementAt_using_column_as_index(bool as
 
     public override Task Where_compare_anonymous_types(bool async)
         => Task.CompletedTask;
+
+    public override Task Subquery_inside_Take_argument(bool async)
+        => Task.CompletedTask;
 }

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

@@ -8286,6 +8286,87 @@ public virtual Task Set_operator_with_navigation_in_projection_groupby_aggregate
                 .GroupBy(x => new { x.Name })
                 .Select(x => new { x.Key.Name, SumOfLengths = x.Sum(xx => xx.Location.Length) }));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Nav_expansion_inside_Contains_argument(bool async)
+    {
+        var numbers = new[] { 1, -1 };
+
+        return AssertQuery(
+            async,
+            ss => ss.Set<Gear>().Where(x => numbers.Contains(x.Weapons.Any() ? 1 : 0)));
+    }
+
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Nav_expansion_with_member_pushdown_inside_Contains_argument(bool async)
+    {
+        var weapons = new[] { "Marcus' Lancer", "Dom's Gnasher" };
+
+        return AssertQuery(
+            async,
+            ss => ss.Set<Gear>().Where(x => weapons.Contains(x.Weapons.OrderBy(w => w.Id).FirstOrDefault().Name)));
+    }
+
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Subquery_inside_Take_argument(bool async)
+    {
+        var numbers = new[] { 0, 1, 2 };
+
+        return AssertQuery(
+            async,
+            ss => ss.Set<Gear>().OrderBy(x => x.Nickname).Select(
+                x => x.Weapons.OrderBy(g => g.Id).Take(numbers.OrderBy(xx => xx).Skip(1).FirstOrDefault())),
+            assertOrder: true,
+            elementAsserter: (e, a) => AssertCollection(e, a, ordered: true));
+    }
+
+    [ConditionalTheory(Skip = "issue #32303")]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Nav_expansion_inside_Skip_correlated_to_source(bool async)
+    {
+        return AssertQuery(
+            async,
+            ss => ss.Set<City>().OrderBy(x => x.Name).Select(
+                x => x.BornGears.OrderBy(g => g.FullName).Skip(x.StationedGears.Any() ? 1 : 0)));
+    }
+
+    [ConditionalTheory(Skip = "issue #32303")]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Nav_expansion_inside_Take_correlated_to_source(bool async)
+    {
+        return AssertQuery(
+            async,
+            ss => ss.Set<Gear>().OrderBy(x => x.Nickname).Select(
+                x => x.Weapons.OrderBy(g => g.Id).Take(x.AssignedCity.Name.Length)));
+    }
+
+    [ConditionalTheory(Skip = "issue #32303")]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Nav_expansion_with_member_pushdown_inside_Take_correlated_to_source(bool async)
+    {
+        var numbers = new[] { 0, 1, 2 };
+
+        return AssertQuery(
+            async,
+            ss => ss.Set<Gear>().OrderBy(x => x.Nickname).Select(
+                x => x.Weapons.OrderBy(g => g.Id).Take(
+                    ss.Set<Gear>().OrderBy(xx => xx.Nickname).FirstOrDefault().AssignedCity.Name.Length)),
+            assertOrder: true,
+            elementAsserter: (e, a) => AssertCollection(e, a, ordered: true));
+    }
+
+    [ConditionalTheory(Skip = "issue #32303")]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Nav_expansion_inside_ElementAt_correlated_to_source(bool async)
+    {
+        return AssertQuery(
+            async,
+            ss => ss.Set<Gear>().OrderBy(x => x.Nickname).Select(
+                x => x.Weapons.OrderBy(g => g.Id).ElementAt(x.AssignedCity != null ? 1 : 0)));
+    }
+
     protected GearsOfWarContext CreateContext()
         => Fixture.CreateContext();
 

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

@@ -10151,6 +10151,108 @@ GROUP BY [s].[Name]
 """);
     }
 
+    public override async Task Nav_expansion_inside_Contains_argument(bool async)
+    {
+        await base.Nav_expansion_inside_Contains_argument(async);
+
+        AssertSql(
+"""
+@__numbers_0='[1,-1]' (Size = 4000)
+
+SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank]
+FROM [Gears] AS [g]
+WHERE CASE
+    WHEN EXISTS (
+        SELECT 1
+        FROM [Weapons] AS [w]
+        WHERE [g].[FullName] = [w].[OwnerFullName]) THEN 1
+    ELSE 0
+END IN (
+    SELECT [n].[value]
+    FROM OPENJSON(@__numbers_0) WITH ([value] int '$') AS [n]
+)
+""");
+    }
+
+    public override async Task Nav_expansion_with_member_pushdown_inside_Contains_argument(bool async)
+    {
+        await base.Nav_expansion_with_member_pushdown_inside_Contains_argument(async);
+
+        AssertSql(
+"""
+@__weapons_0='["Marcus\u0027 Lancer","Dom\u0027s Gnasher"]' (Size = 4000)
+
+SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank]
+FROM [Gears] AS [g]
+WHERE EXISTS (
+    SELECT 1
+    FROM OPENJSON(@__weapons_0) WITH ([value] nvarchar(max) '$') AS [w0]
+    WHERE [w0].[value] = (
+        SELECT TOP(1) [w].[Name]
+        FROM [Weapons] AS [w]
+        WHERE [g].[FullName] = [w].[OwnerFullName]
+        ORDER BY [w].[Id]) OR ([w0].[value] IS NULL AND (
+        SELECT TOP(1) [w].[Name]
+        FROM [Weapons] AS [w]
+        WHERE [g].[FullName] = [w].[OwnerFullName]
+        ORDER BY [w].[Id]) IS NULL))
+""");
+    }
+
+    public override async Task Subquery_inside_Take_argument(bool async)
+    {
+        await base.Subquery_inside_Take_argument(async);
+
+        AssertSql(
+"""
+@__numbers_0='[0,1,2]' (Size = 4000)
+
+SELECT [g].[Nickname], [g].[SquadId], [t0].[Id], [t0].[AmmunitionType], [t0].[IsAutomatic], [t0].[Name], [t0].[OwnerFullName], [t0].[SynergyWithId]
+FROM [Gears] AS [g]
+LEFT JOIN (
+    SELECT [t].[Id], [t].[AmmunitionType], [t].[IsAutomatic], [t].[Name], [t].[OwnerFullName], [t].[SynergyWithId]
+    FROM (
+        SELECT [w].[Id], [w].[AmmunitionType], [w].[IsAutomatic], [w].[Name], [w].[OwnerFullName], [w].[SynergyWithId], ROW_NUMBER() OVER(PARTITION BY [w].[OwnerFullName] ORDER BY [w].[Id]) AS [row]
+        FROM [Weapons] AS [w]
+    ) AS [t]
+    WHERE [t].[row] <= COALESCE((
+        SELECT [n].[value]
+        FROM OPENJSON(@__numbers_0) WITH ([value] int '$') AS [n]
+        ORDER BY [n].[value]
+        OFFSET 1 ROWS FETCH NEXT 1 ROWS ONLY), 0)
+) AS [t0] ON [g].[FullName] = [t0].[OwnerFullName]
+ORDER BY [g].[Nickname], [g].[SquadId], [t0].[OwnerFullName], [t0].[Id]
+""");
+    }
+
+    public override async Task Nav_expansion_inside_Skip_correlated_to_source(bool async)
+    {
+        await base.Nav_expansion_inside_Skip_correlated_to_source(async);
+
+        AssertSql();
+    }
+
+    public override async Task Nav_expansion_inside_Take_correlated_to_source(bool async)
+    {
+        await base.Nav_expansion_inside_Take_correlated_to_source(async);
+
+        AssertSql();
+    }
+
+    public override async Task Nav_expansion_with_member_pushdown_inside_Take_correlated_to_source(bool async)
+    {
+        await base.Nav_expansion_with_member_pushdown_inside_Take_correlated_to_source(async);
+
+        AssertSql();
+    }
+
+    public override async Task Nav_expansion_inside_ElementAt_correlated_to_source(bool async)
+    {
+        await base.Nav_expansion_inside_ElementAt_correlated_to_source(async);
+
+        AssertSql();
+    }
+
     private void AssertSql(params string[] expected)
         => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
 }