From 1956bd4fa614e0ed26b8c353da7f4bddaeb64dee Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Mon, 29 Jul 2019 13:30:13 +0200 Subject: [PATCH] Allow set operations over other mapping types Including different mapping types on the two sides, as long as the result is compatible. Fixes #16725 --- .../Query/SqlExpressions/SelectExpression.cs | 29 ++++++----------- .../SimpleQueryCosmosTest.SetOperations.cs | 3 ++ .../Query/SimpleQueryInMemoryTest.cs | 10 ++++++ .../SimpleQueryTestBase.SetOperations.cs | 23 ++++++++++++++ .../SimpleQuerySqlServerTest.SetOperations.cs | 31 +++++++++++++++++++ 5 files changed, 76 insertions(+), 20 deletions(-) diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index 4904b926f8c..cf8f5fc1b47 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -452,17 +452,19 @@ public Expression ApplySetOperation( continue; } - if (joinedMapping.Value1 is ColumnExpression && joinedMapping.Value2 is ColumnExpression - || joinedMapping.Value1 is ScalarSubqueryExpression && joinedMapping.Value2 is ScalarSubqueryExpression) + if (joinedMapping.Value1 is SqlExpression innerColumn1 + && joinedMapping.Value2 is SqlExpression innerColumn2) { - handleColumnMapping( - joinedMapping.Key, - select1, (SqlExpression)joinedMapping.Value1, - select2, (SqlExpression)joinedMapping.Value2); + // The actual columns may actually be different, but we don't care as long as the type and alias + // coming out of the two operands are the same + var alias = joinedMapping.Key.Last?.Name; + select1.AddToProjection(innerColumn1, alias); + select2.AddToProjection(innerColumn2, alias); + _projectionMapping[joinedMapping.Key] = innerColumn1; continue; } - throw new InvalidOperationException("Non-matching or unknown projection mapping type in set operation"); + throw new InvalidOperationException($"Non-matching or unknown projection mapping type in set operation ({joinedMapping.Value1.GetType().Name} and {joinedMapping.Value2.GetType().Name})"); } } @@ -520,19 +522,6 @@ ColumnExpression addSetOperationColumnProjections( return column1; } - - void handleColumnMapping( - ProjectionMember projectionMember, - SelectExpression select1, SqlExpression innerColumn1, - SelectExpression select2, SqlExpression innerColumn2) - { - // The actual columns may actually be different, but we don't care as long as the type and alias - // coming out of the two operands are the same - var alias = projectionMember.Last?.Name; - select1.AddToProjection(innerColumn1, alias); - select2.AddToProjection(innerColumn2, alias); - _projectionMapping[projectionMember] = innerColumn1; - } } public IDictionary PushdownIntoSubquery() diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.SetOperations.cs b/test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.SetOperations.cs index a40d56d866a..4a378153466 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.SetOperations.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.SetOperations.cs @@ -40,5 +40,8 @@ public override void Include_Union_only_on_one_side_throws() {} public override void Include_Union_different_includes_throws() {} public override Task SubSelect_Union(bool isAsync) => Task.CompletedTask; public override Task Client_eval_Union_FirstOrDefault(bool isAsync) => Task.CompletedTask; + public override Task GroupBy_Select_Union(bool isAsync) => Task.CompletedTask; + public override Task Union_over_different_projection_types(bool isAsync) => Task.CompletedTask; + } } diff --git a/test/EFCore.InMemory.FunctionalTests/Query/SimpleQueryInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/Query/SimpleQueryInMemoryTest.cs index 80c167d331c..a0bc648a979 100644 --- a/test/EFCore.InMemory.FunctionalTests/Query/SimpleQueryInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/Query/SimpleQueryInMemoryTest.cs @@ -217,6 +217,16 @@ public override Task Select_Except_reference_projection(bool isAsync) return Task.CompletedTask; //base.Select_Except_reference_projection(isAsync); } + public override Task GroupBy_Select_Union(bool isAsync) + { + return Task.CompletedTask; //base.GroupBy_Select_Union(isAsync); + } + + public override Task Union_over_different_projection_types(bool isAsync) + { + return Task.CompletedTask; //base.Union_over_different_projection_types(isAsync); + } + #endregion [ConditionalFact(Skip = "Issue#16564")] diff --git a/test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.SetOperations.cs b/test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.SetOperations.cs index c79dc9002bc..9beba4f02cb 100644 --- a/test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.SetOperations.cs +++ b/test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.SetOperations.cs @@ -339,5 +339,28 @@ public virtual Task Client_eval_Union_FirstOrDefault(bool isAsync) .Union(cs)); private static Customer ClientSideMethod(Customer c) => c; + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task GroupBy_Select_Union(bool isAsync) + => AssertQuery(isAsync, cs => cs + .Where(c => c.City == "Berlin") + .GroupBy(c => c.CustomerID) + .Select(g => new { CustomerID = g.Key, Count = g.Count() }) + .Union(cs + .Where(c => c.City == "London") + .GroupBy(c => c.CustomerID) + .Select(g => new { CustomerID = g.Key, Count = g.Count() }))); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Union_over_different_projection_types(bool isAsync) + => AssertQuery(isAsync, cs => cs + .Where(c => c.City == "Berlin") + .GroupBy(c => c.CustomerID) + .Select(g => new { CustomerID = g.Key, Count = g.Count() }) + .Union(cs + .Where(c => c.City == "London") + .Select(c => new { c.CustomerID, Count = c.ContactName.Length }))); } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.SetOperations.cs b/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.SetOperations.cs index 3bdf17787a8..f8dcb278038 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.SetOperations.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.SetOperations.cs @@ -334,5 +334,36 @@ FROM [Orders] AS [o0] WHERE ([c0].[CustomerID] = [o0].[CustomerID]) AND [o0].[CustomerID] IS NOT NULL) AS [Orders] FROM [Customers] AS [c0]"); } + + public override async Task GroupBy_Select_Union(bool isAsync) + { + await base.GroupBy_Select_Union(isAsync); + + AssertSql( + @"SELECT [c].[CustomerID], COUNT(*) AS [Count] +FROM [Customers] AS [c] +WHERE ([c].[City] = N'Berlin') AND [c].[City] IS NOT NULL +GROUP BY [c].[CustomerID] +UNION +SELECT [c0].[CustomerID], COUNT(*) AS [Count] +FROM [Customers] AS [c0] +WHERE ([c0].[City] = N'London') AND [c0].[City] IS NOT NULL +GROUP BY [c0].[CustomerID]"); + } + + public override async Task Union_over_different_projection_types(bool isAsync) + { + await base.Union_over_different_projection_types(isAsync); + + AssertSql( + @"SELECT [c].[CustomerID], COUNT(*) AS [Count] +FROM [Customers] AS [c] +WHERE ([c].[City] = N'Berlin') AND [c].[City] IS NOT NULL +GROUP BY [c].[CustomerID] +UNION +SELECT [c0].[CustomerID], CAST(LEN([c0].[ContactName]) AS int) AS [Count] +FROM [Customers] AS [c0] +WHERE ([c0].[City] = N'London') AND [c0].[City] IS NOT NULL"); + } } }