Addressed review comments

Also found some more skipped set operation tests, unskipped and
rearranged them.

Fixes dotnet#12568

src/EFCore.Relational/Query/Pipeline/SqlExpressions/SelectExpression.cs

@@ -502,18 +502,8 @@ void HandleEntityMapping(
                  if (commonParentEntityType != projection1.EntityType)
                  {
                      // The first source has been up-cast by the set operation, so we also need to change the shaper expression.
-                     var entityShaperExpression =
-                         shaperExpression as EntityShaperExpression ?? (
-                             shaperExpression is UnaryExpression unary
-                             && unary.NodeType == ExpressionType.Convert
-                             && unary.Type == commonParentEntityType.ClrType
-                                 ? unary.Operand as EntityShaperExpression : null);
-
-                     if (entityShaperExpression != null)
-                     {
-                         shaperExpression = new EntityShaperExpression(
-                             commonParentEntityType, entityShaperExpression.ValueBufferExpression, entityShaperExpression.Nullable);
-                     }
+                     // The EntityShaperExpression may be buried under Convert nodes produced by Cast operators, preserve those.
+                     shaperExpression = UpdateEntityShaperEntityType(shaperExpression, commonParentEntityType);
                  }
             }
 
@@ -567,6 +557,21 @@ void HandleColumnMapping(
                 var outerColumn = new ColumnExpression(projectionExpression1, select1, IsNullableProjection(projectionExpression1));
                 _projectionMapping[projectionMember] = outerColumn;
             }
+
+            static Expression UpdateEntityShaperEntityType(Expression shaperExpression, IEntityType newEntityType)
+            {
+                switch (shaperExpression)
+                {
+                    case EntityShaperExpression entityShaperExpression:
+                        return new EntityShaperExpression(newEntityType, entityShaperExpression.ValueBufferExpression, entityShaperExpression.Nullable);
+                    case UnaryExpression unary when unary.NodeType == ExpressionType.Convert:
+                        return Convert(UpdateEntityShaperEntityType(unary.Operand, newEntityType), unary.Type);
+                    case UnaryExpression unary when unary.NodeType == ExpressionType.ConvertChecked:
+                        return ConvertChecked(UpdateEntityShaperEntityType(unary.Operand, newEntityType), unary.Type);
+                    default:
+                        throw new Exception($"Unexpected expression type {shaperExpression.GetType().Name} encountered in {nameof(UpdateEntityShaperEntityType)}");
+                }
+            }
         }
 
         public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
@@ -716,8 +721,7 @@ public RelationalCollectionShaperExpression ApplyCollectionJoin(int collectionId
             {
                 if (IsDistinct || Limit != null || Offset != null || IsSetOperation)
                 {
-                    var pushdown = PushdownIntoSubquery();
-                    outer = new SqlRemappingVisitor(pushdown).Remap(outer);
+                    outer = new SqlRemappingVisitor(PushdownIntoSubquery()).Remap(outer);
                 }
 
                 if (innerSelectExpression.Offset != null
@@ -1332,12 +1336,7 @@ public enum SetOperationType
         /// <summary>
         /// Represents an SQL EXCEPT set operation.
         /// </summary>
-        Except    = 4,
-
-        /// <summary>
-        /// Represents a custom, provider-specific set operation.
-        /// </summary>
-        Other     = 9999
+        Except    = 4
     }
 }
 

src/EFCore/Properties/CoreStrings.resx

@@ -1186,4 +1186,7 @@
   <data name="UnableToDiscriminate" xml:space="preserve">
     <value>Unable to materialize entity of type '{entityType}'. No discriminators matched '{discriminator}'.</value>
   </data>
-</root>
+  <data name="SetOperationWithDifferentIncludesInOperands" xml:space="preserve">
+    <value>When performing a set operation, both operands must have the same Include operations.</value>
+  </data>
+</root>

src/EFCore/Query/NavigationExpansion/NavigationTreeNode.cs

@@ -132,17 +132,14 @@ public static NavigationTreeNode Create(
             return result;
         }
 
-        public List<NavigationTreeNode> Flatten()
+        public IEnumerable<NavigationTreeNode> Flatten()
         {
-            var result = new List<NavigationTreeNode>();
-            result.Add(this);
+            yield return this;
 
-            foreach (var child in Children)
+            foreach (var child in Children.SelectMany(c => c.Flatten()))
             {
-                result.AddRange(child.Flatten());
+                yield return child;
             }
-
-            return result;
         }
 
         // TODO: just make property settable?

src/EFCore/Query/NavigationExpansion/Visitors/NavigationExpandingVisitor_MethodCall.cs

@@ -7,6 +7,7 @@
 using System.Linq.Expressions;
 using System.Reflection;
 using System.Xml;
+using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
@@ -873,6 +874,35 @@ private Expression ProcessSetOperation(MethodCallExpression methodCallExpression
             var source2 = VisitSourceExpression(methodCallExpression.Arguments[1]);
             var preProcessResult2 = PreProcessTerminatingOperation(source2);
 
+            // Compare the include chains from each side to make sure they're identical. We don't allow set operations over
+            // operands with different include chains.
+            var current1 = preProcessResult1.state.PendingIncludeChain?.NavigationTreeNode;
+            var current2 = preProcessResult2.state.PendingIncludeChain?.NavigationTreeNode;
+            while (true)
+            {
+                if (current1 == null)
+                {
+                    if (current2 == null)
+                    {
+                        break;
+                    }
+                    throw new NotSupportedException(CoreStrings.SetOperationWithDifferentIncludesInOperands);
+                }
+
+                if (current2 == null)
+                {
+                    throw new NotSupportedException(CoreStrings.SetOperationWithDifferentIncludesInOperands);
+                }
+
+                if (current1.FromMappings.Zip(current2.FromMappings, (m1, m2) => (m1, m2))
+                    .Any(t => !t.m1.SequenceEqual(t.m2)))
+                {
+                    throw new NotSupportedException(CoreStrings.SetOperationWithDifferentIncludesInOperands);
+                }
+
+                (current1, current2) = (current1.Parent, current2.Parent);
+            }
+
             // If the siblings are different types, one is derived from the other the set operation returns the less derived type.
             // Find that.
             var clrType1 = preProcessResult1.state.CurrentParameter.Type;