Skip to content

Commit

Permalink
Fix to #32217 - Nav expansion visitor does not visit the Contains ite…
Browse files Browse the repository at this point in the history
…m argument (and possibly other non-lambda arguments)

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
  • Loading branch information
maumar committed Nov 15, 2023
1 parent bbb0a35 commit 74ee9e5
Show file tree
Hide file tree
Showing 6 changed files with 248 additions and 0 deletions.
14 changes: 14 additions & 0 deletions src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ 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;

private static readonly PropertyInfo QueryContextContextPropertyInfo
= typeof(QueryContext).GetTypeInfo().GetDeclaredProperty(nameof(QueryContext.Context))!;

Expand Down Expand Up @@ -921,6 +930,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);
}

Expand Down
22 changes: 22 additions & 0 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8286,6 +8286,28 @@ 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)));
}

protected GearsOfWarContext CreateContext()
=> Fixture.CreateContext();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10151,6 +10151,54 @@ 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))
""");
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
}
Original file line number Diff line number Diff line change
Expand Up @@ -13365,6 +13365,66 @@ 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 [t].[Nickname], [t].[SquadId], [t].[AssignedCityName], [t].[CityOfBirthName], [t].[FullName], [t].[HasSoulPatch], [t].[LeaderNickname], [t].[LeaderSquadId], [t].[Rank], [t].[Discriminator]
FROM (
SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank], N'Gear' AS [Discriminator]
FROM [Gears] AS [g]
UNION ALL
SELECT [o].[Nickname], [o].[SquadId], [o].[AssignedCityName], [o].[CityOfBirthName], [o].[FullName], [o].[HasSoulPatch], [o].[LeaderNickname], [o].[LeaderSquadId], [o].[Rank], N'Officer' AS [Discriminator]
FROM [Officers] AS [o]
) AS [t]
WHERE CASE
WHEN EXISTS (
SELECT 1
FROM [Weapons] AS [w]
WHERE [t].[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 [t].[Nickname], [t].[SquadId], [t].[AssignedCityName], [t].[CityOfBirthName], [t].[FullName], [t].[HasSoulPatch], [t].[LeaderNickname], [t].[LeaderSquadId], [t].[Rank], [t].[Discriminator]
FROM (
SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank], N'Gear' AS [Discriminator]
FROM [Gears] AS [g]
UNION ALL
SELECT [o].[Nickname], [o].[SquadId], [o].[AssignedCityName], [o].[CityOfBirthName], [o].[FullName], [o].[HasSoulPatch], [o].[LeaderNickname], [o].[LeaderSquadId], [o].[Rank], N'Officer' AS [Discriminator]
FROM [Officers] AS [o]
) AS [t]
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 [t].[FullName] = [w].[OwnerFullName]
ORDER BY [w].[Id]) OR ([w0].[value] IS NULL AND (
SELECT TOP(1) [w].[Name]
FROM [Weapons] AS [w]
WHERE [t].[FullName] = [w].[OwnerFullName]
ORDER BY [w].[Id]) IS NULL))
""");
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11508,6 +11508,60 @@ 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].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank], CASE
WHEN [o].[Nickname] IS NOT NULL THEN N'Officer'
END AS [Discriminator]
FROM [Gears] AS [g]
LEFT JOIN [Officers] AS [o] ON [g].[Nickname] = [o].[Nickname] AND [g].[SquadId] = [o].[SquadId]
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].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank], CASE
WHEN [o].[Nickname] IS NOT NULL THEN N'Officer'
END AS [Discriminator]
FROM [Gears] AS [g]
LEFT JOIN [Officers] AS [o] ON [g].[Nickname] = [o].[Nickname] AND [g].[SquadId] = [o].[SquadId]
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))
""");
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9520,6 +9520,56 @@ 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 = 6)

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 json_each(@__numbers_0) 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 = 44)

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 json_each(@__weapons_0) AS "w0"
WHERE "w0"."value" = (
SELECT "w"."Name"
FROM "Weapons" AS "w"
WHERE "g"."FullName" = "w"."OwnerFullName"
ORDER BY "w"."Id"
LIMIT 1) OR ("w0"."value" IS NULL AND (
SELECT "w"."Name"
FROM "Weapons" AS "w"
WHERE "g"."FullName" = "w"."OwnerFullName"
ORDER BY "w"."Id"
LIMIT 1) IS NULL))
""");
}

public override Task DateTimeOffset_to_unix_time_milliseconds(bool async)
=> AssertTranslationFailed(() => base.DateTimeOffset_to_unix_time_milliseconds(async));

Expand Down

0 comments on commit 74ee9e5

Please sign in to comment.