[release/8.0] Merge release/8.0-staging => release/8.0

src/EFCore.Relational/Query/QuerySqlGenerator.cs

@@ -44,6 +44,9 @@ public class QuerySqlGenerator : SqlExpressionVisitor
     private static readonly bool UseOldBehavior32375 =
         AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32375", out var enabled32375) && enabled32375;
 
+    private static readonly bool UseOldBehavior36105 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue36105", out var enabled36105) && enabled36105;
+
     /// <summary>
     ///     Creates a new instance of the <see cref="QuerySqlGenerator" /> class.
     /// </summary>
@@ -1276,9 +1279,16 @@ static string GetSetOperation(SetOperationBase operation)
     protected virtual void GenerateSetOperationOperand(SetOperationBase setOperation, SelectExpression operand)
     {
         // INTERSECT has higher precedence over UNION and EXCEPT, but otherwise evaluation is left-to-right.
-        // To preserve meaning, add parentheses whenever a set operation is nested within a different set operation.
+        // To preserve evaluation order, add parentheses whenever a set operation is nested within a different set operation
+        // - including different distinctness.
+        // In addition, EXCEPT is non-commutative (unlike UNION/INTERSECT), so add parentheses for that case too (see #36105).
         if (IsNonComposedSetOperation(operand)
-            && operand.Tables[0].GetType() != setOperation.GetType())
+            && ((UseOldBehavior36105 && operand.Tables[0].GetType() != setOperation.GetType())
+                || (!UseOldBehavior36105
+                    && operand.Tables[0] is SetOperationBase nestedSetOperation
+                    && (nestedSetOperation is ExceptExpression
+                        || nestedSetOperation.GetType() != setOperation.GetType()
+                        || nestedSetOperation.IsDistinct != setOperation.IsDistinct))))
         {
             _relationalCommandBuilder.AppendLine("(");
             using (_relationalCommandBuilder.Indent())

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

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics.CodeAnalysis;
 using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
 using Microsoft.EntityFrameworkCore.Sqlite.Query.SqlExpressions.Internal;
 
@@ -14,6 +15,9 @@ namespace Microsoft.EntityFrameworkCore.Sqlite.Query.Internal;
 /// </summary>
 public class SqliteQuerySqlGenerator : QuerySqlGenerator
 {
+    private static readonly bool UseOldBehavior36112 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue36112", out var enabled36112) && enabled36112;
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -98,8 +102,63 @@ protected override void GenerateLimitOffset(SelectExpression selectExpression)
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     protected override void GenerateSetOperationOperand(SetOperationBase setOperation, SelectExpression operand)
-        // Sqlite doesn't support parentheses around set operation operands
-        => Visit(operand);
+    {
+        // Most databases support parentheses around set operations to determine evaluation order, but SQLite does not;
+        // however, we can instead wrap the nested set operation in a SELECT * FROM () to achieve the same effect.
+        // The following is a copy-paste of the base implementation from QuerySqlGenerator, adding the SELECT.
+
+        // INTERSECT has higher precedence over UNION and EXCEPT, but otherwise evaluation is left-to-right.
+        // To preserve evaluation order, add parentheses whenever a set operation is nested within a different set operation
+        // - including different distinctness.
+        // In addition, EXCEPT is non-commutative (unlike UNION/INTERSECT), so add parentheses for that case too (see #36105).
+        if (!UseOldBehavior36112
+            && TryUnwrapBareSetOperation(operand, out var nestedSetOperation)
+            && (nestedSetOperation is ExceptExpression
+                || nestedSetOperation.GetType() != setOperation.GetType()
+                || nestedSetOperation.IsDistinct != setOperation.IsDistinct))
+        {
+            Sql.AppendLine("SELECT * FROM (");
+
+            using (Sql.Indent())
+            {
+                Visit(operand);
+            }
+
+            Sql.AppendLine().Append(")");
+        }
+        else
+        {
+            Visit(operand);
+        }
+
+        static bool TryUnwrapBareSetOperation(SelectExpression selectExpression, [NotNullWhen(true)] out SetOperationBase? setOperation)
+        {
+            if (selectExpression is
+                {
+                    Tables: [SetOperationBase s],
+                    Predicate: null,
+                    Orderings: [],
+                    Offset: null,
+                    Limit: null,
+                    IsDistinct: false,
+                    Having: null,
+                    GroupBy: []
+                }
+                && selectExpression.Projection.Count == s.Source1.Projection.Count
+                && selectExpression.Projection.Select(
+                        (pe, index) => pe.Expression is ColumnExpression column
+                            && column.TableAlias == s.Alias
+                            && column.Name == s.Source1.Projection[index].Alias)
+                    .All(e => e))
+            {
+                setOperation = s;
+                return true;
+            }
+
+            setOperation = null;
+            return false;
+        }
+    }
 
     private void GenerateGlob(GlobExpression globExpression, bool negated = false)
     {

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

@@ -82,6 +82,19 @@ public virtual Task Except_nested(bool async)
                 .Except(ss.Set<Customer>().Where(s => s.City == "México D.F."))
                 .Except(ss.Set<Customer>().Where(e => e.City == "Seattle")));
 
+    // EXCEPT is non-commutative, unlike UNION/INTERSECT. Therefore, parentheses are needed in the following query
+    // to ensure that the inner EXCEPT is evaluated first. See #36105.
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Except_nested2(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Customer>()
+                .Except(ss.Set<Customer>()
+                    .Where(s => s.City == "Seattle")
+                    .Except(ss.Set<Customer>()
+                        .Where(e => e.City == "Seattle"))));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Except_non_entity(bool async)
@@ -221,6 +234,17 @@ public virtual Task Union_Intersect(bool async)
                 .Union(ss.Set<Customer>().Where(c => c.City == "London"))
                 .Intersect(ss.Set<Customer>().Where(c => c.ContactName.Contains("Thomas"))));
 
+    // 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.
+    // Make sure parentheses are added.
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Union_inside_Concat(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Customer>().Where(c => c.City == "Berlin")
+                .Concat(ss.Set<Customer>().Where(c => c.City == "London")
+                    .Union(ss.Set<Customer>().Where(c => c.City == "Berlin"))));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Union_Take_Union_Take(bool async)

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

@@ -201,6 +201,28 @@ WHERE [c1].[ContactName] LIKE N'%Thomas%'
 """);
     }
 
+    public override async Task Union_inside_Concat(bool async)
+    {
+        await base.Union_inside_Concat(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]
+WHERE [c].[City] = N'Berlin'
+UNION ALL
+(
+    SELECT [c0].[CustomerID], [c0].[Address], [c0].[City], [c0].[CompanyName], [c0].[ContactName], [c0].[ContactTitle], [c0].[Country], [c0].[Fax], [c0].[Phone], [c0].[PostalCode], [c0].[Region]
+    FROM [Customers] AS [c0]
+    WHERE [c0].[City] = N'London'
+    UNION
+    SELECT [c1].[CustomerID], [c1].[Address], [c1].[City], [c1].[CompanyName], [c1].[ContactName], [c1].[ContactTitle], [c1].[Country], [c1].[Fax], [c1].[Phone], [c1].[PostalCode], [c1].[Region]
+    FROM [Customers] AS [c1]
+    WHERE [c1].[City] = N'Berlin'
+)
+""");
+    }
+
     public override async Task Union_Take_Union_Take(bool async)
     {
         await base.Union_Take_Union_Take(async);
@@ -1234,21 +1256,44 @@ public override async Task Except_nested(bool async)
         await base.Except_nested(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]
-WHERE [c].[ContactTitle] = N'Owner'
-EXCEPT
-SELECT [c0].[CustomerID], [c0].[Address], [c0].[City], [c0].[CompanyName], [c0].[ContactName], [c0].[ContactTitle], [c0].[Country], [c0].[Fax], [c0].[Phone], [c0].[PostalCode], [c0].[Region]
-FROM [Customers] AS [c0]
-WHERE [c0].[City] = N'México D.F.'
+"""
+(
+    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]
+    WHERE [c].[ContactTitle] = N'Owner'
+    EXCEPT
+    SELECT [c0].[CustomerID], [c0].[Address], [c0].[City], [c0].[CompanyName], [c0].[ContactName], [c0].[ContactTitle], [c0].[Country], [c0].[Fax], [c0].[Phone], [c0].[PostalCode], [c0].[Region]
+    FROM [Customers] AS [c0]
+    WHERE [c0].[City] = N'México D.F.'
+)
 EXCEPT
 SELECT [c1].[CustomerID], [c1].[Address], [c1].[City], [c1].[CompanyName], [c1].[ContactName], [c1].[ContactTitle], [c1].[Country], [c1].[Fax], [c1].[Phone], [c1].[PostalCode], [c1].[Region]
 FROM [Customers] AS [c1]
 WHERE [c1].[City] = N'Seattle'
 """);
     }
 
+    public override async Task Except_nested2(bool async)
+    {
+        await base.Except_nested2(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]
+EXCEPT
+(
+    SELECT [c0].[CustomerID], [c0].[Address], [c0].[City], [c0].[CompanyName], [c0].[ContactName], [c0].[ContactTitle], [c0].[Country], [c0].[Fax], [c0].[Phone], [c0].[PostalCode], [c0].[Region]
+    FROM [Customers] AS [c0]
+    WHERE [c0].[City] = N'Seattle'
+    EXCEPT
+    SELECT [c1].[CustomerID], [c1].[Address], [c1].[City], [c1].[CompanyName], [c1].[ContactName], [c1].[ContactTitle], [c1].[Country], [c1].[Fax], [c1].[Phone], [c1].[PostalCode], [c1].[Region]
+    FROM [Customers] AS [c1]
+    WHERE [c1].[City] = N'Seattle'
+)
+""");
+    }
+
     public override async Task Intersect_non_entity(bool async)
     {
         await base.Intersect_non_entity(async);