From 07542218b7c8df2b82b9d621ecc525b40ef4764a Mon Sep 17 00:00:00 2001 From: maumar Date: Wed, 30 Oct 2024 01:34:49 -0700 Subject: [PATCH] Fix to #35007 - EF Core fails to translate LINQ query to SQL if JOIN 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 --- ...yableMethodNormalizingExpressionVisitor.cs | 58 ++++++++++++---- .../NorthwindJoinQueryRelationalTestBase.cs | 2 + .../Query/NorthwindJoinQueryTestBase.cs | 66 +++++++++++++++++++ .../Query/NorthwindJoinQuerySqlServerTest.cs | 56 ++++++++++++++++ 4 files changed, 171 insertions(+), 11 deletions(-) diff --git a/src/EFCore/Query/Internal/QueryableMethodNormalizingExpressionVisitor.cs b/src/EFCore/Query/Internal/QueryableMethodNormalizingExpressionVisitor.cs index 493a95e91b5..aae8897fda4 100644 --- a/src/EFCore/Query/Internal/QueryableMethodNormalizingExpressionVisitor.cs +++ b/src/EFCore/Query/Internal/QueryableMethodNormalizingExpressionVisitor.cs @@ -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; @@ -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 outerArguments } + && innerKeySelector.Body is NewExpression { Arguments: ReadOnlyCollection 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]), diff --git a/test/EFCore.Relational.Specification.Tests/Query/NorthwindJoinQueryRelationalTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/NorthwindJoinQueryRelationalTestBase.cs index 9f7af143391..7d65c1a99bf 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/NorthwindJoinQueryRelationalTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/NorthwindJoinQueryRelationalTestBase.cs @@ -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 diff --git a/test/EFCore.Specification.Tests/Query/NorthwindJoinQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindJoinQueryTestBase.cs index 6f3350b748f..4e602af3a4f 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindJoinQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindJoinQueryTestBase.cs @@ -654,6 +654,55 @@ join o in ss.Set().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().GroupJoin( + ss.Set(), + 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().GroupJoin( + ss.Set(), + 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().GroupJoin( + ss.Set(), + 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().GroupJoin( + ss.Set(), + 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) @@ -909,4 +958,21 @@ join o in ss.Set().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().Order().Take(10).Join( + ss.Set(), + 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); + }); } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindJoinQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindJoinQuerySqlServerTest.cs index 48c3758e51a..69d5a37d484 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindJoinQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindJoinQuerySqlServerTest.cs @@ -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); @@ -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);