From 32522c3971b235fb532966b613d7fd75749c2018 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Thu, 13 Apr 2023 23:45:58 +0200 Subject: [PATCH] Stop pushing down to subquery for non-Concat set operations without limit/offset Closes #30684 --- .../Query/SqlExpressions/SelectExpression.cs | 17 ++--- .../NorthwindSetOperationsQueryTestBase.cs | 32 +++++++++ ...orthwindSetOperationsQuerySqlServerTest.cs | 68 +++++++++++++++++++ 3 files changed, 109 insertions(+), 8 deletions(-) diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index 1ee709f7f9e..881fdb7b8ee 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -2303,25 +2303,26 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi var entityProjectionValueComparers = new List(); var otherExpressions = new List(); - if (select1.Orderings.Count != 0 - || select1.Limit != null - || select1.Offset != null) + // Push down into a subquery if limit/offset are defined. If not, any orderings can be discarded as set operations don't preserve + // them. + // Note that in some databases it may be possible to preserve the internal ordering of the set operands for Concat, but we don't + // currently support that. + if (select1.Limit != null || select1.Offset != null) { // If we are pushing down here, we need to make sure to assign unique alias to subquery also. var subqueryAlias = GenerateUniqueAlias(_usedAliases, "t"); select1.PushdownIntoSubquery(); select1._tables[0].Alias = subqueryAlias; select1._tableReferences[0].Alias = subqueryAlias; - select1.ClearOrdering(); } + select1.ClearOrdering(); - if (select2.Orderings.Count != 0 - || select2.Limit != null - || select2.Offset != null) + // Do the same for the other side of the set operation + if (select2.Limit != null || select2.Offset != null) { select2.PushdownIntoSubquery(); - select2.ClearOrdering(); } + select2.ClearOrdering(); if (_clientProjections.Count > 0 || select2._clientProjections.Count > 0) diff --git a/test/EFCore.Specification.Tests/Query/NorthwindSetOperationsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindSetOperationsQueryTestBase.cs index f1bf3da4ab3..e85db42828e 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindSetOperationsQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindSetOperationsQueryTestBase.cs @@ -761,6 +761,38 @@ public virtual Task Union_over_scalarsubquery_scalarsubquery(bool async) ss => ss.Set().Select(o => o.OrderDetails.Count()) .Union(ss.Set().Select(o => o.OrderDetails.Count()))); + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Union_over_OrderBy_Take1(bool async) + => AssertQueryScalar( + async, + ss => ss.Set().OrderBy(o => o.OrderID).Take(5).Select(o => o.OrderDate) + .Union(ss.Set().Select(o => o.OrderDate))); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Union_over_OrderBy_without_Skip_Take1(bool async) + => AssertQueryScalar( + async, + ss => ss.Set().OrderBy(o => o.OrderID).Select(o => o.OrderDate) + .Union(ss.Set().Select(o => o.OrderDate))); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Union_over_OrderBy_Take2(bool async) + => AssertQueryScalar( + async, + ss => ss.Set().Select(o => o.OrderDate) + .Union(ss.Set().OrderBy(o => o.OrderID).Take(5).Select(o => o.OrderDate))); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Union_over_OrderBy_without_Skip_Take2(bool async) + => AssertQueryScalar( + async, + ss => ss.Set().Select(o => o.OrderDate) + .Union(ss.Set().OrderBy(o => o.OrderID).Select(o => o.OrderDate))); + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task OrderBy_Take_Union(bool async) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSetOperationsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSetOperationsQuerySqlServerTest.cs index bd94935de61..c5da6fee05d 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSetOperationsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSetOperationsQuerySqlServerTest.cs @@ -1020,6 +1020,74 @@ FROM [Orders] AS [o1] """); } + public override async Task Union_over_OrderBy_Take1(bool async) + { + await base.Union_over_OrderBy_Take1(async); + + AssertSql( +""" +@__p_0='5' + +SELECT [t].[OrderDate] +FROM ( + SELECT TOP(@__p_0) [o].[OrderDate], [o].[OrderID] + FROM [Orders] AS [o] + ORDER BY [o].[OrderID] +) AS [t] +UNION +SELECT [o0].[OrderDate] +FROM [Orders] AS [o0] +"""); + } + + public override async Task Union_over_OrderBy_without_Skip_Take1(bool async) + { + await base.Union_over_OrderBy_without_Skip_Take1(async); + + AssertSql( +""" +SELECT [o].[OrderDate] +FROM [Orders] AS [o] +UNION +SELECT [o0].[OrderDate] +FROM [Orders] AS [o0] +"""); + } + + public override async Task Union_over_OrderBy_Take2(bool async) + { + await base.Union_over_OrderBy_Take2(async); + + AssertSql( +""" +@__p_0='5' + +SELECT [o].[OrderDate] +FROM [Orders] AS [o] +UNION +SELECT [t0].[OrderDate] +FROM ( + SELECT TOP(@__p_0) [o0].[OrderDate], [o0].[OrderID] + FROM [Orders] AS [o0] + ORDER BY [o0].[OrderID] +) AS [t0] +"""); + } + + public override async Task Union_over_OrderBy_without_Skip_Take2(bool async) + { + await base.Union_over_OrderBy_without_Skip_Take2(async); + + AssertSql( +""" +SELECT [o].[OrderDate] +FROM [Orders] AS [o] +UNION +SELECT [o0].[OrderDate] +FROM [Orders] AS [o0] +"""); + } + public override async Task OrderBy_Take_Union(bool async) { await base.OrderBy_Take_Union(async);