Skip to content

Commit 7a337e9

Browse files
authored
Additional set operation parentheses fix (#36124)
Follow-up for 246b2d2
1 parent dd185ae commit 7a337e9

File tree

4 files changed

+43
-4
lines changed

4 files changed

+43
-4
lines changed

src/EFCore.Relational/Query/QuerySqlGenerator.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,10 +1368,13 @@ static string GetSetOperation(SetOperationBase operation)
13681368
protected virtual void GenerateSetOperationOperand(SetOperationBase setOperation, SelectExpression operand)
13691369
{
13701370
// INTERSECT has higher precedence over UNION and EXCEPT, but otherwise evaluation is left-to-right.
1371-
// To preserve evaluation order, add parentheses whenever a set operation is nested within a different set operation.
1371+
// To preserve evaluation order, add parentheses whenever a set operation is nested within a different set operation
1372+
// - including different distinctness.
13721373
// In addition, EXCEPT is non-commutative (unlike UNION/INTERSECT), so add parentheses for that case too (see #36105).
13731374
if (TryUnwrapBareSetOperation(operand, out var nestedSetOperation)
1374-
&& (nestedSetOperation is ExceptExpression || nestedSetOperation.GetType() != setOperation.GetType()))
1375+
&& (nestedSetOperation is ExceptExpression
1376+
|| nestedSetOperation.GetType() != setOperation.GetType()
1377+
|| nestedSetOperation.IsDistinct != setOperation.IsDistinct))
13751378
{
13761379
_relationalCommandBuilder.AppendLine("(");
13771380

src/EFCore.Sqlite.Core/Query/Internal/SqliteQuerySqlGenerator.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,13 @@ protected override void GenerateSetOperationOperand(SetOperationBase setOperatio
104104
// The following is a copy-paste of the base implementation from QuerySqlGenerator, adding the SELECT.
105105

106106
// INTERSECT has higher precedence over UNION and EXCEPT, but otherwise evaluation is left-to-right.
107-
// To preserve evaluation order, add parentheses whenever a set operation is nested within a different set operation.
107+
// To preserve evaluation order, add parentheses whenever a set operation is nested within a different set operation
108+
// - including different distinctness.
108109
// In addition, EXCEPT is non-commutative (unlike UNION/INTERSECT), so add parentheses for that case too (see #36105).
109110
if (TryUnwrapBareSetOperation(operand, out var nestedSetOperation)
110-
&& (nestedSetOperation is ExceptExpression || nestedSetOperation.GetType() != setOperation.GetType()))
111+
&& (nestedSetOperation is ExceptExpression
112+
|| nestedSetOperation.GetType() != setOperation.GetType()
113+
|| nestedSetOperation.IsDistinct != setOperation.IsDistinct))
111114
{
112115
Sql.AppendLine("SELECT * FROM (");
113116

test/EFCore.Specification.Tests/Query/NorthwindSetOperationsQueryTestBase.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,17 @@ public virtual Task Union_Intersect(bool async)
231231
.Union(ss.Set<Customer>().Where(c => c.City == "London"))
232232
.Intersect(ss.Set<Customer>().Where(c => c.ContactName.Contains("Thomas"))));
233233

234+
// The evaluation order of Concat and Union can matter: A UNION ALL (B UNION C) can be different from (A UNION ALL B) UNION C.
235+
// Make sure parentheses are added.
236+
[ConditionalTheory]
237+
[MemberData(nameof(IsAsyncData))]
238+
public virtual Task Union_inside_Concat(bool async)
239+
=> AssertQuery(
240+
async,
241+
ss => ss.Set<Customer>().Where(c => c.City == "Berlin")
242+
.Concat(ss.Set<Customer>().Where(c => c.City == "London")
243+
.Union(ss.Set<Customer>().Where(c => c.City == "Berlin"))));
244+
234245
[ConditionalTheory]
235246
[MemberData(nameof(IsAsyncData))]
236247
public virtual Task Union_Take_Union_Take(bool async)

test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSetOperationsQuerySqlServerTest.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,28 @@ WHERE [c1].[ContactName] LIKE N'%Thomas%'
200200
""");
201201
}
202202

203+
public override async Task Union_inside_Concat(bool async)
204+
{
205+
await base.Union_inside_Concat(async);
206+
207+
AssertSql(
208+
"""
209+
SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
210+
FROM [Customers] AS [c]
211+
WHERE [c].[City] = N'Berlin'
212+
UNION ALL
213+
(
214+
SELECT [c0].[CustomerID], [c0].[Address], [c0].[City], [c0].[CompanyName], [c0].[ContactName], [c0].[ContactTitle], [c0].[Country], [c0].[Fax], [c0].[Phone], [c0].[PostalCode], [c0].[Region]
215+
FROM [Customers] AS [c0]
216+
WHERE [c0].[City] = N'London'
217+
UNION
218+
SELECT [c1].[CustomerID], [c1].[Address], [c1].[City], [c1].[CompanyName], [c1].[ContactName], [c1].[ContactTitle], [c1].[Country], [c1].[Fax], [c1].[Phone], [c1].[PostalCode], [c1].[Region]
219+
FROM [Customers] AS [c1]
220+
WHERE [c1].[City] = N'Berlin'
221+
)
222+
""");
223+
}
224+
203225
public override async Task Union_Take_Union_Take(bool async)
204226
{
205227
await base.Union_Take_Union_Take(async);

0 commit comments

Comments
 (0)