Skip to content

Commit

Permalink
Fix to #35007 - EF Core fails to translate LINQ query to SQL if JOIN …
Browse files Browse the repository at this point in the history
…contains multiple columns

problem is in the GroupJoinConvertingExpressionVisitor where we convert GroupJoin key selectors into the correlation predicate for result selector subqueries. We blindly copy them over and in case of composite key selectors which fail to translate later in the pipeline (can't translate equals on anonymous object that is not a constant). Normally when processing regular joins we have logic that breaks up anonymous objects into constituent parts and compare them one by one instead (CreateJoinPredicate). However by the time we get to the translation we are dealing with a subquery with a Where predicate - we don't know at that point that it came from GroupJoin.

Fix is to tweak the logic in GroupJoinConvertingExpressionVisitor to break apart anonymous objects, like we do in CreateJoinPredicate

Fixes #35007
  • Loading branch information
maumar committed Nov 1, 2024
1 parent 17ed217 commit 0754221
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections;
using System.Collections.ObjectModel;
using Microsoft.EntityFrameworkCore.Internal;
using ExpressionExtensions = Microsoft.EntityFrameworkCore.Infrastructure.ExpressionExtensions;

Expand Down Expand Up @@ -753,17 +754,52 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
innerSource);
}

var correlationPredicate = ReplacingExpressionVisitor.Replace(
outerKeySelector.Parameters[0],
resultSelector.Parameters[0],
Expression.AndAlso(
ExpressionExtensions.CreateEqualsExpression(
outerKeySelector.Body,
Expression.Constant(null),
negated: true),
ExpressionExtensions.CreateEqualsExpression(
outerKeySelector.Body,
innerKeySelector.Body)));
Expression correlationPredicate;
if (outerKeySelector.Body is NewExpression { Arguments: ReadOnlyCollection<Expression> outerArguments }
&& innerKeySelector.Body is NewExpression { Arguments: ReadOnlyCollection<Expression> innerArguments }
&& outerArguments.Count == innerArguments.Count
&& outerArguments.Count > 0)
{
Expression? outerNotEqualsNull = null;
Expression? outerEqualsInner = null;
for (var i = 0; i < outerArguments.Count; i++)
{
var outerArgumentNotEqualsNull = ExpressionExtensions.CreateEqualsExpression(outerArguments[i], Expression.Constant(null), negated: true);
var outerArgumentEqualsInnerArgument = ExpressionExtensions.CreateEqualsExpression(outerArguments[i], innerArguments[i]);

if (i == 0)
{
outerNotEqualsNull = outerArgumentNotEqualsNull;
outerEqualsInner = outerArgumentEqualsInnerArgument;
}
else
{
outerNotEqualsNull = Expression.AndAlso(outerNotEqualsNull!, outerArgumentNotEqualsNull);
outerEqualsInner = Expression.AndAlso(outerEqualsInner!, outerArgumentEqualsInnerArgument);
}
}

correlationPredicate = ReplacingExpressionVisitor.Replace(
outerKeySelector.Parameters[0],
resultSelector.Parameters[0],
Expression.AndAlso(
outerNotEqualsNull!,
outerEqualsInner!));
}
else
{
correlationPredicate = ReplacingExpressionVisitor.Replace(
outerKeySelector.Parameters[0],
resultSelector.Parameters[0],
Expression.AndAlso(
ExpressionExtensions.CreateEqualsExpression(
outerKeySelector.Body,
Expression.Constant(null),
negated: true),
ExpressionExtensions.CreateEqualsExpression(
outerKeySelector.Body,
innerKeySelector.Body)));
}

innerSource = Expression.Call(
QueryableMethods.Where.MakeGenericMethod(genericArguments[1]),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +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 Microsoft.EntityFrameworkCore.TestModels.Northwind;

namespace Microsoft.EntityFrameworkCore.Query;

#nullable disable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,55 @@ join o in ss.Set<Order>().OrderBy(o => o.OrderID).Take(100) on c.CustomerID equa
from o in lo.Where(x => x.CustomerID.StartsWith("A"))
select new { c.CustomerID, o.OrderID });

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupJoin_aggregate_anonymous_key_selectors(bool async)
=> AssertQuery(
async,
ss => ss.Set<Customer>().GroupJoin(
ss.Set<Order>(),
x => new { x.CustomerID, x.City },
x => new { x.CustomerID, City = "London" },
(c, g) => new { c.CustomerID, Sum = g.Sum(x => x.CustomerID.Length) }),
elementSorter: e => e.CustomerID);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupJoin_aggregate_anonymous_key_selectors2(bool async)
=> AssertQuery(
async,
ss => ss.Set<Customer>().GroupJoin(
ss.Set<Order>(),
x => new { x.CustomerID, Year = 1996 },
x => new { x.CustomerID, x.OrderDate.Value.Year },
(c, g) => new { c.CustomerID, Sum = g.Sum(x => x.CustomerID.Length) }),
elementSorter: e => e.CustomerID);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupJoin_aggregate_anonymous_key_selectors_one_argument(bool async)
=> AssertQuery(
async,
ss => ss.Set<Customer>().GroupJoin(
ss.Set<Order>(),
x => new { x.CustomerID },
x => new { x.CustomerID },
(c, g) => new { c.CustomerID, Sum = g.Sum(x => x.CustomerID.Length) }),
elementSorter: e => e.CustomerID);

[ConditionalTheory(Skip = "issue 35028")]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupJoin_aggregate_nested_anonymous_key_selectors(bool async)
=> AssertTranslationFailed(
() => AssertQuery(
async,
ss => ss.Set<Customer>().GroupJoin(
ss.Set<Order>(),
x => new { x.CustomerID, Nested = new { x.City, Year = 1996 } },
x => new { x.CustomerID, Nested = new { City = "London", x.OrderDate.Value.Year } },
(c, g) => new { c.CustomerID, Sum = g.Sum(x => x.CustomerID.Length) }),
elementSorter: e => e.CustomerID));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Inner_join_with_tautology_predicate_converts_to_cross_join(bool async)
Expand Down Expand Up @@ -909,4 +958,21 @@ join o in ss.Set<Order>().Include(o => o.OrderDetails)
on c.CustomerID equals o.CustomerID into g
from o in g.DefaultIfEmpty()
select new { a = o != null ? o.OrderID : -1 });

[ConditionalTheory(Skip = "issue #35028")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Join_with_key_selectors_being_nested_anonymous_objects(bool async)
=> AssertQuery(
async,
ss => ss.Set<Customer>().Order().Take(10).Join(
ss.Set<Order>(),
x => new { x.CustomerID, Nested = new { x.City, Year = 1996 } },
x => new { x.CustomerID, Nested = new { City = "London", x.OrderDate.Value.Year } },
(c, o) => new { c, o }),
elementSorter: e => (e.c.CustomerID, e.o.OrderID ),
elementAsserter: (e, a) =>
{
AssertEqual(e.c, a.c);
AssertEqual(e.o, a.o);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,55 @@ WHERE [o0].[CustomerID] LIKE N'A%'
""");
}

public override async Task GroupJoin_aggregate_anonymous_key_selectors(bool async)
{
await base.GroupJoin_aggregate_anonymous_key_selectors(async);

AssertSql(
"""
SELECT [c].[CustomerID], (
SELECT COALESCE(SUM(CAST(LEN([o].[CustomerID]) AS int)), 0)
FROM [Orders] AS [o]
WHERE [c].[City] IS NOT NULL AND [c].[CustomerID] = [o].[CustomerID] AND [c].[City] = N'London') AS [Sum]
FROM [Customers] AS [c]
""");
}

public override async Task GroupJoin_aggregate_anonymous_key_selectors2(bool async)
{
await base.GroupJoin_aggregate_anonymous_key_selectors2(async);

AssertSql(
"""
SELECT [c].[CustomerID], (
SELECT COALESCE(SUM(CAST(LEN([o].[CustomerID]) AS int)), 0)
FROM [Orders] AS [o]
WHERE [c].[CustomerID] = [o].[CustomerID] AND 1996 = DATEPART(year, [o].[OrderDate])) AS [Sum]
FROM [Customers] AS [c]
""");
}

public override async Task GroupJoin_aggregate_anonymous_key_selectors_one_argument(bool async)
{
await base.GroupJoin_aggregate_anonymous_key_selectors_one_argument(async);

AssertSql(
"""
SELECT [c].[CustomerID], (
SELECT COALESCE(SUM(CAST(LEN([o].[CustomerID]) AS int)), 0)
FROM [Orders] AS [o]
WHERE [c].[CustomerID] = [o].[CustomerID]) AS [Sum]
FROM [Customers] AS [c]
""");
}

public override async Task GroupJoin_aggregate_nested_anonymous_key_selectors(bool async)
{
await base.GroupJoin_aggregate_nested_anonymous_key_selectors(async);

AssertSql();
}

public override async Task Inner_join_with_tautology_predicate_converts_to_cross_join(bool async)
{
await base.Inner_join_with_tautology_predicate_converts_to_cross_join(async);
Expand Down Expand Up @@ -992,6 +1041,13 @@ ORDER BY [o].[OrderID]
""");
}

public override async Task Join_with_key_selectors_being_nested_anonymous_objects(bool async)
{
await base.Join_with_key_selectors_being_nested_anonymous_objects(async);

AssertSql();
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down

0 comments on commit 0754221

Please sign in to comment.