diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index 3254d901ac7..676ef9ce4fb 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -2132,57 +2132,67 @@ static void GetPartitions(SelectExpression selectExpression, SqlExpression sqlEx static SqlExpression? TryExtractJoinKey(SelectExpression outer, SelectExpression inner, bool allowNonEquality) { - if (inner.Limit == null - && inner.Offset == null - && inner.Predicate != null) - { - var outerColumnExpressions = new List(); - var joinPredicate = TryExtractJoinKey( - outer, - inner, - inner.Predicate, - outerColumnExpressions, - allowNonEquality, - out var predicate); + if (inner.Limit != null + || inner.Offset != null) + { + return null; + } - if (joinPredicate != null) - { - joinPredicate = RemoveRedundantNullChecks(joinPredicate, outerColumnExpressions); - } - // TODO: verify the case for GroupBy. See issue#24474 - // We extract join predicate from Predicate part but GroupBy would have last Having. Changing predicate can change groupings + var predicate = inner.GroupBy.Count > 0 ? inner.Having : inner.Predicate; + if (predicate == null) + { + return null; + } - // we can't convert apply to join in case of distinct and groupby, if the projection doesn't already contain the join keys - // since we can't add the missing keys to the projection - only convert to join if all the keys are already there - if (joinPredicate != null - && (inner.IsDistinct - || inner.GroupBy.Count > 0)) - { - var innerKeyColumns = new List(); - InnerKeyColumns(inner.Tables, joinPredicate, innerKeyColumns); + var outerColumnExpressions = new List(); + var joinPredicate = TryExtractJoinKey( + outer, + inner, + predicate, + outerColumnExpressions, + allowNonEquality, + out var updatedPredicate); + + if (joinPredicate != null) + { + joinPredicate = RemoveRedundantNullChecks(joinPredicate, outerColumnExpressions); + } - // if projection has already been applied we can use it directly - // otherwise we extract future projection columns from projection mapping - // and based on that we determine whether we can convert from APPLY to JOIN - var projectionColumns = inner.Projection.Count > 0 - ? inner.Projection.Select(p => p.Expression) - : ExtractColumnsFromProjectionMapping(inner._projectionMapping); + // we can't convert apply to join in case of distinct and groupby, if the projection doesn't already contain the join keys + // since we can't add the missing keys to the projection - only convert to join if all the keys are already there + if (joinPredicate != null + && (inner.IsDistinct + || inner.GroupBy.Count > 0)) + { + var innerKeyColumns = new List(); + PopulateInnerKeyColumns(inner.Tables, joinPredicate, innerKeyColumns); + + // if projection has already been applied we can use it directly + // otherwise we extract future projection columns from projection mapping + // and based on that we determine whether we can convert from APPLY to JOIN + var projectionColumns = inner.Projection.Count > 0 + ? inner.Projection.Select(p => p.Expression) + : ExtractColumnsFromProjectionMapping(inner._projectionMapping); - foreach (var innerColumn in innerKeyColumns) + foreach (var innerColumn in innerKeyColumns) + { + if (!projectionColumns.Contains(innerColumn)) { - if (!projectionColumns.Contains(innerColumn)) - { - return null; - } + return null; } } + } - inner.Predicate = predicate; - - return joinPredicate; + if (inner.GroupBy.Count > 0) + { + inner.Having = updatedPredicate; + } + else + { + inner.Predicate = updatedPredicate; } - return null; + return joinPredicate; static SqlExpression? TryExtractJoinKey( SelectExpression outer, @@ -2310,12 +2320,12 @@ static bool IsContainedColumn(SelectExpression selectExpression, SqlExpression s } } - static void InnerKeyColumns(IEnumerable tables, SqlExpression joinPredicate, List resultColumns) + static void PopulateInnerKeyColumns(IEnumerable tables, SqlExpression joinPredicate, List resultColumns) { if (joinPredicate is SqlBinaryExpression sqlBinaryExpression) { - InnerKeyColumns(tables, sqlBinaryExpression.Left, resultColumns); - InnerKeyColumns(tables, sqlBinaryExpression.Right, resultColumns); + PopulateInnerKeyColumns(tables, sqlBinaryExpression.Left, resultColumns); + PopulateInnerKeyColumns(tables, sqlBinaryExpression.Right, resultColumns); } else if (joinPredicate is ColumnExpression columnExpression && tables.Contains(columnExpression.Table)) diff --git a/test/EFCore.Specification.Tests/Query/NorthwindGroupByQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindGroupByQueryTestBase.cs index 3fe9700ec58..f11d23ba64b 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindGroupByQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindGroupByQueryTestBase.cs @@ -1898,6 +1898,37 @@ join o in ss.Set() on a.LastOrderID equals o.OrderID entryCount: 126); } + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task GroupBy_Aggregate_Join_converted_from_SelectMany(bool async) + { + return AssertQuery( + async, + ss => from c in ss.Set() + from o in ss.Set().GroupBy(o => o.CustomerID) + .Where(g => g.Count() > 5) + .Select(g => new { CustomerID = g.Key, LastOrderID = g.Max(o => o.OrderID) }) + .Where(c1 => c.CustomerID == c1.CustomerID) + select c, + entryCount: 63); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task GroupBy_Aggregate_LeftJoin_converted_from_SelectMany(bool async) + { + return AssertQuery( + async, + ss => from c in ss.Set() + from o in ss.Set().GroupBy(o => o.CustomerID) + .Where(g => g.Count() > 5) + .Select(g => new { CustomerID = g.Key, LastOrderID = g.Max(o => o.OrderID) }) + .Where(c1 => c.CustomerID == c1.CustomerID) + .DefaultIfEmpty() + select c, + entryCount: 91); + } + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Join_GroupBy_Aggregate_multijoins(bool async) @@ -2486,8 +2517,8 @@ public virtual Task GroupBy_aggregate_without_selectMany_selecting_first(bool as async, ss => from id in (from o in ss.Set() - group o by o.CustomerID into g - select g.Min(x => x.OrderID)) + group o by o.CustomerID into g + select g.Min(x => x.OrderID)) from o in ss.Set() where o.OrderID == id select o, diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindGroupByQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindGroupByQuerySqlServerTest.cs index e1fac97d2ad..1a474be074f 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindGroupByQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindGroupByQuerySqlServerTest.cs @@ -1420,6 +1420,36 @@ HAVING COUNT(*) > 5 INNER JOIN [Orders] AS [o0] ON [t].[LastOrderID] = [o0].[OrderID]"); } + public override async Task GroupBy_Aggregate_Join_converted_from_SelectMany(bool async) + { + await base.GroupBy_Aggregate_Join_converted_from_SelectMany(async); + + AssertSql( + @"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] +FROM [Customers] AS [c] +INNER JOIN ( + SELECT [o].[CustomerID] + FROM [Orders] AS [o] + GROUP BY [o].[CustomerID] + HAVING COUNT(*) > 5 +) AS [t] ON [c].[CustomerID] = [t].[CustomerID]"); + } + + public override async Task GroupBy_Aggregate_LeftJoin_converted_from_SelectMany(bool async) + { + await base.GroupBy_Aggregate_LeftJoin_converted_from_SelectMany(async); + + AssertSql( + @"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] +FROM [Customers] AS [c] +LEFT JOIN ( + SELECT [o].[CustomerID], MAX([o].[OrderID]) AS [LastOrderID] + FROM [Orders] AS [o] + GROUP BY [o].[CustomerID] + HAVING COUNT(*) > 5 +) AS [t] ON [c].[CustomerID] = [t].[CustomerID]"); + } + public override async Task Join_GroupBy_Aggregate_multijoins(bool async) { await base.Join_GroupBy_Aggregate_multijoins(async);