Skip to content

Commit 6980d92

Browse files
maumarmaumar
authored
[release/8.0] Fix to #32217 - Nav expansion visitor does not visit the Contains item argument (and possibly other non-lambda arguments) (#32317)
This was always a problem but was uncovered by change in how we process Contains. We were going through the full process of nav expansion on the argument (only on the source), which could lead to untranslatable expression tree in some cases. Fixes #32217 Fixes #32312
1 parent 2be6f1d commit 6980d92

File tree

8 files changed

+660
-2
lines changed

8 files changed

+660
-2
lines changed

‎src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,24 @@ namespace Microsoft.EntityFrameworkCore.Query.Internal;
1515
/// </summary>
1616
public partial class NavigationExpandingExpressionVisitor : ExpressionVisitor
1717
{
18+
/// <summary>
19+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
20+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
21+
/// any release. You should only use it directly in your code with extreme caution and knowing that
22+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
23+
/// </summary>
24+
public static readonly bool UseOldBehavior32217 =
25+
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32217", out var enabled32217) && enabled32217;
26+
27+
/// <summary>
28+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
29+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
30+
/// any release. You should only use it directly in your code with extreme caution and knowing that
31+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
32+
/// </summary>
33+
public static readonly bool UseOldBehavior32312 =
34+
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32312", out var enabled32312) && enabled32312;
35+
1836
private static readonly PropertyInfo QueryContextContextPropertyInfo
1937
= typeof(QueryContext).GetTypeInfo().GetDeclaredProperty(nameof(QueryContext.Context))!;
2038

@@ -921,6 +939,11 @@ private Expression ProcessContains(NavigationExpansionExpression source, Express
921939
source = (NavigationExpansionExpression)_pendingSelectorExpandingExpressionVisitor.Visit(source);
922940
var queryable = Reduce(source);
923941

942+
if (!UseOldBehavior32217)
943+
{
944+
item = Visit(item);
945+
}
946+
924947
return Expression.Call(QueryableMethods.Contains.MakeGenericMethod(queryable.Type.GetSequenceType()), queryable, item);
925948
}
926949

@@ -959,11 +982,16 @@ private NavigationExpansionExpression ProcessDistinct(NavigationExpansionExpress
959982
return new NavigationExpansionExpression(result, navigationTree, navigationTree, parameterName);
960983
}
961984

962-
private static NavigationExpansionExpression ProcessSkipTake(
985+
private NavigationExpansionExpression ProcessSkipTake(
963986
NavigationExpansionExpression source,
964987
MethodInfo genericMethod,
965988
Expression count)
966989
{
990+
if (!UseOldBehavior32312)
991+
{
992+
count = Visit(count);
993+
}
994+
967995
source.UpdateSource(Expression.Call(genericMethod.MakeGenericMethod(source.SourceElementType), source.Source, count));
968996

969997
return source;
@@ -1002,6 +1030,11 @@ private NavigationExpansionExpression ProcessElementAt(
10021030
source.ApplySelector(Expression.Convert(source.PendingSelector, returnType));
10031031
}
10041032

1033+
if (!UseOldBehavior32312)
1034+
{
1035+
index = Visit(index);
1036+
}
1037+
10051038
source.ConvertToSingleResult(genericMethod, index);
10061039

10071040
return source;
@@ -1560,11 +1593,16 @@ private GroupByNavigationExpansionExpression ProcessOrderByThenBy(
15601593
return new NavigationExpansionExpression(newSource, navigationTree, navigationTree, parameterName);
15611594
}
15621595

1563-
private static GroupByNavigationExpansionExpression ProcessSkipTake(
1596+
private GroupByNavigationExpansionExpression ProcessSkipTake(
15641597
GroupByNavigationExpansionExpression groupBySource,
15651598
MethodInfo genericMethod,
15661599
Expression count)
15671600
{
1601+
if (!UseOldBehavior32312)
1602+
{
1603+
count = Visit(count);
1604+
}
1605+
15681606
groupBySource.UpdateSource(
15691607
Expression.Call(genericMethod.MakeGenericMethod(groupBySource.SourceElementType), groupBySource.Source, count));
15701608

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,4 +136,7 @@ public override Task Where_subquery_with_ElementAt_using_column_as_index(bool as
136136

137137
public override Task Where_compare_anonymous_types(bool async)
138138
=> Task.CompletedTask;
139+
140+
public override Task Subquery_inside_Take_argument(bool async)
141+
=> Task.CompletedTask;
139142
}

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8286,6 +8286,87 @@ public virtual Task Set_operator_with_navigation_in_projection_groupby_aggregate
82868286
.GroupBy(x => new { x.Name })
82878287
.Select(x => new { x.Key.Name, SumOfLengths = x.Sum(xx => xx.Location.Length) }));
82888288

8289+
[ConditionalTheory]
8290+
[MemberData(nameof(IsAsyncData))]
8291+
public virtual Task Nav_expansion_inside_Contains_argument(bool async)
8292+
{
8293+
var numbers = new[] { 1, -1 };
8294+
8295+
return AssertQuery(
8296+
async,
8297+
ss => ss.Set<Gear>().Where(x => numbers.Contains(x.Weapons.Any() ? 1 : 0)));
8298+
}
8299+
8300+
[ConditionalTheory]
8301+
[MemberData(nameof(IsAsyncData))]
8302+
public virtual Task Nav_expansion_with_member_pushdown_inside_Contains_argument(bool async)
8303+
{
8304+
var weapons = new[] { "Marcus' Lancer", "Dom's Gnasher" };
8305+
8306+
return AssertQuery(
8307+
async,
8308+
ss => ss.Set<Gear>().Where(x => weapons.Contains(x.Weapons.OrderBy(w => w.Id).FirstOrDefault().Name)));
8309+
}
8310+
8311+
[ConditionalTheory]
8312+
[MemberData(nameof(IsAsyncData))]
8313+
public virtual Task Subquery_inside_Take_argument(bool async)
8314+
{
8315+
var numbers = new[] { 0, 1, 2 };
8316+
8317+
return AssertQuery(
8318+
async,
8319+
ss => ss.Set<Gear>().OrderBy(x => x.Nickname).Select(
8320+
x => x.Weapons.OrderBy(g => g.Id).Take(numbers.OrderBy(xx => xx).Skip(1).FirstOrDefault())),
8321+
assertOrder: true,
8322+
elementAsserter: (e, a) => AssertCollection(e, a, ordered: true));
8323+
}
8324+
8325+
[ConditionalTheory(Skip = "issue #32303")]
8326+
[MemberData(nameof(IsAsyncData))]
8327+
public virtual Task Nav_expansion_inside_Skip_correlated_to_source(bool async)
8328+
{
8329+
return AssertQuery(
8330+
async,
8331+
ss => ss.Set<City>().OrderBy(x => x.Name).Select(
8332+
x => x.BornGears.OrderBy(g => g.FullName).Skip(x.StationedGears.Any() ? 1 : 0)));
8333+
}
8334+
8335+
[ConditionalTheory(Skip = "issue #32303")]
8336+
[MemberData(nameof(IsAsyncData))]
8337+
public virtual Task Nav_expansion_inside_Take_correlated_to_source(bool async)
8338+
{
8339+
return AssertQuery(
8340+
async,
8341+
ss => ss.Set<Gear>().OrderBy(x => x.Nickname).Select(
8342+
x => x.Weapons.OrderBy(g => g.Id).Take(x.AssignedCity.Name.Length)));
8343+
}
8344+
8345+
[ConditionalTheory(Skip = "issue #32303")]
8346+
[MemberData(nameof(IsAsyncData))]
8347+
public virtual Task Nav_expansion_with_member_pushdown_inside_Take_correlated_to_source(bool async)
8348+
{
8349+
var numbers = new[] { 0, 1, 2 };
8350+
8351+
return AssertQuery(
8352+
async,
8353+
ss => ss.Set<Gear>().OrderBy(x => x.Nickname).Select(
8354+
x => x.Weapons.OrderBy(g => g.Id).Take(
8355+
ss.Set<Gear>().OrderBy(xx => xx.Nickname).FirstOrDefault().AssignedCity.Name.Length)),
8356+
assertOrder: true,
8357+
elementAsserter: (e, a) => AssertCollection(e, a, ordered: true));
8358+
}
8359+
8360+
[ConditionalTheory(Skip = "issue #32303")]
8361+
[MemberData(nameof(IsAsyncData))]
8362+
public virtual Task Nav_expansion_inside_ElementAt_correlated_to_source(bool async)
8363+
{
8364+
return AssertQuery(
8365+
async,
8366+
ss => ss.Set<Gear>().OrderBy(x => x.Nickname).Select(
8367+
x => x.Weapons.OrderBy(g => g.Id).ElementAt(x.AssignedCity != null ? 1 : 0)));
8368+
}
8369+
82898370
protected GearsOfWarContext CreateContext()
82908371
=> Fixture.CreateContext();
82918372

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

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10151,6 +10151,108 @@ GROUP BY [s].[Name]
1015110151
""");
1015210152
}
1015310153

10154+
public override async Task Nav_expansion_inside_Contains_argument(bool async)
10155+
{
10156+
await base.Nav_expansion_inside_Contains_argument(async);
10157+
10158+
AssertSql(
10159+
"""
10160+
@__numbers_0='[1,-1]' (Size = 4000)
10161+
10162+
SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank]
10163+
FROM [Gears] AS [g]
10164+
WHERE CASE
10165+
WHEN EXISTS (
10166+
SELECT 1
10167+
FROM [Weapons] AS [w]
10168+
WHERE [g].[FullName] = [w].[OwnerFullName]) THEN 1
10169+
ELSE 0
10170+
END IN (
10171+
SELECT [n].[value]
10172+
FROM OPENJSON(@__numbers_0) WITH ([value] int '$') AS [n]
10173+
)
10174+
""");
10175+
}
10176+
10177+
public override async Task Nav_expansion_with_member_pushdown_inside_Contains_argument(bool async)
10178+
{
10179+
await base.Nav_expansion_with_member_pushdown_inside_Contains_argument(async);
10180+
10181+
AssertSql(
10182+
"""
10183+
@__weapons_0='["Marcus\u0027 Lancer","Dom\u0027s Gnasher"]' (Size = 4000)
10184+
10185+
SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank]
10186+
FROM [Gears] AS [g]
10187+
WHERE EXISTS (
10188+
SELECT 1
10189+
FROM OPENJSON(@__weapons_0) WITH ([value] nvarchar(max) '$') AS [w0]
10190+
WHERE [w0].[value] = (
10191+
SELECT TOP(1) [w].[Name]
10192+
FROM [Weapons] AS [w]
10193+
WHERE [g].[FullName] = [w].[OwnerFullName]
10194+
ORDER BY [w].[Id]) OR ([w0].[value] IS NULL AND (
10195+
SELECT TOP(1) [w].[Name]
10196+
FROM [Weapons] AS [w]
10197+
WHERE [g].[FullName] = [w].[OwnerFullName]
10198+
ORDER BY [w].[Id]) IS NULL))
10199+
""");
10200+
}
10201+
10202+
public override async Task Subquery_inside_Take_argument(bool async)
10203+
{
10204+
await base.Subquery_inside_Take_argument(async);
10205+
10206+
AssertSql(
10207+
"""
10208+
@__numbers_0='[0,1,2]' (Size = 4000)
10209+
10210+
SELECT [g].[Nickname], [g].[SquadId], [t0].[Id], [t0].[AmmunitionType], [t0].[IsAutomatic], [t0].[Name], [t0].[OwnerFullName], [t0].[SynergyWithId]
10211+
FROM [Gears] AS [g]
10212+
LEFT JOIN (
10213+
SELECT [t].[Id], [t].[AmmunitionType], [t].[IsAutomatic], [t].[Name], [t].[OwnerFullName], [t].[SynergyWithId]
10214+
FROM (
10215+
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]
10216+
FROM [Weapons] AS [w]
10217+
) AS [t]
10218+
WHERE [t].[row] <= COALESCE((
10219+
SELECT [n].[value]
10220+
FROM OPENJSON(@__numbers_0) WITH ([value] int '$') AS [n]
10221+
ORDER BY [n].[value]
10222+
OFFSET 1 ROWS FETCH NEXT 1 ROWS ONLY), 0)
10223+
) AS [t0] ON [g].[FullName] = [t0].[OwnerFullName]
10224+
ORDER BY [g].[Nickname], [g].[SquadId], [t0].[OwnerFullName], [t0].[Id]
10225+
""");
10226+
}
10227+
10228+
public override async Task Nav_expansion_inside_Skip_correlated_to_source(bool async)
10229+
{
10230+
await base.Nav_expansion_inside_Skip_correlated_to_source(async);
10231+
10232+
AssertSql();
10233+
}
10234+
10235+
public override async Task Nav_expansion_inside_Take_correlated_to_source(bool async)
10236+
{
10237+
await base.Nav_expansion_inside_Take_correlated_to_source(async);
10238+
10239+
AssertSql();
10240+
}
10241+
10242+
public override async Task Nav_expansion_with_member_pushdown_inside_Take_correlated_to_source(bool async)
10243+
{
10244+
await base.Nav_expansion_with_member_pushdown_inside_Take_correlated_to_source(async);
10245+
10246+
AssertSql();
10247+
}
10248+
10249+
public override async Task Nav_expansion_inside_ElementAt_correlated_to_source(bool async)
10250+
{
10251+
await base.Nav_expansion_inside_ElementAt_correlated_to_source(async);
10252+
10253+
AssertSql();
10254+
}
10255+
1015410256
private void AssertSql(params string[] expected)
1015510257
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
1015610258
}

0 commit comments

Comments
 (0)