Stop moving pending selector after DefaultIfEmpty in nav expansion

src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs

@@ -464,24 +464,24 @@ public virtual void ApplyDefaultIfEmpty()
             throw new InvalidOperationException(InMemoryStrings.DefaultIfEmptyAppliedAfterProjection);
         }
 
-        var projectionMapping = new Dictionary<ProjectionMember, Expression>();
-        foreach (var (projectionMember, expression) in _projectionMapping)
-        {
-            projectionMapping[projectionMember] = expression is EntityProjectionExpression entityProjectionExpression
-                ? MakeEntityProjectionNullable(entityProjectionExpression)
-                : MakeReadValueNullable(expression);
-        }
-
-        _projectionMapping = projectionMapping;
-        var projectionMappingExpressions = _projectionMappingExpressions.Select(e => MakeReadValueNullable(e)).ToList();
-        _projectionMappingExpressions.Clear();
-        _projectionMappingExpressions.AddRange(projectionMappingExpressions);
-        _groupingParameter = null;
-
         ServerQueryExpression = Call(
             EnumerableMethods.DefaultIfEmptyWithArgument.MakeGenericMethod(typeof(ValueBuffer)),
             ServerQueryExpression,
             Constant(new ValueBuffer(Enumerable.Repeat((object?)null, _projectionMappingExpressions.Count).ToArray())));
+
+        ReplaceProjection(_projectionMapping.ToDictionary(
+            kv => kv.Key,
+            kv => kv.Value switch
+            {
+                EntityProjectionExpression p => MakeEntityProjectionNullable(p),
+
+                var p when !p.Type.IsNullableType()
+                    => Coalesce(
+                        MakeReadValueNullable(p),
+                        p.Type.GetDefaultValueConstant()),
+
+                var p => p
+            }));
     }
 
     /// <summary>

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

@@ -2378,20 +2378,25 @@ [new ProjectionExpression(nullSqlExpression, "empty")],
         _tables.Add(dummySelectExpression);
         _tables.Add(joinTable);
 
+        // Go over all projected columns and make them nullable; for non-nullable value types, add a SQL COALESCE as well.
         var projectionMapping = new Dictionary<ProjectionMember, Expression>();
-        foreach (var projection in _projectionMapping)
+        foreach (var (projectionMember, projection) in _projectionMapping)
         {
-            var projectionToAdd = projection.Value;
-            if (projectionToAdd is StructuralTypeProjectionExpression typeProjection)
+            var newProjection = projection switch
             {
-                projectionToAdd = typeProjection.MakeNullable();
-            }
-            else if (projectionToAdd is ColumnExpression column)
+                StructuralTypeProjectionExpression p => p.MakeNullable(),
+                ColumnExpression column => column.MakeNullable(),
+                var p => p
+            };
+
+            if (newProjection is SqlExpression { Type: var type } newSqlProjection && !type.IsNullableType())
             {
-                projectionToAdd = column.MakeNullable();
+                newProjection = sqlExpressionFactory.Coalesce(
+                    newSqlProjection,
+                    sqlExpressionFactory.Constant(type.GetDefaultValue(), type));
             }
 
-            projectionMapping[projection.Key] = projectionToAdd;
+            projectionMapping[projectionMember] = newProjection;
         }
 
         // ChildIdentifiers shouldn't be required to be updated since during translation they should be empty.

src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs

@@ -966,23 +966,21 @@ private Expression ProcessContains(NavigationExpansionExpression source, Express
 
     private NavigationExpansionExpression ProcessDefaultIfEmpty(NavigationExpansionExpression source)
     {
-        source.UpdateSource(
-            Expression.Call(
-                QueryableMethods.DefaultIfEmptyWithoutArgument.MakeGenericMethod(source.SourceElementType),
-                source.Source));
+        // Apply any pending selector, since previous Selects can't be moved after DefaultIfEmpty (change of meaning, see #36208).
+        // Mark the entity(s) being selected as optional.
+        source = (NavigationExpansionExpression)_pendingSelectorExpandingExpressionVisitor.Visit(source);
+        var newStructure = SnapshotExpression(source.PendingSelector);
+        newStructure = _entityReferenceOptionalMarkingExpressionVisitor.Visit(newStructure);
+        var queryable = Reduce(source);
 
-        var pendingSelector = source.PendingSelector;
-        _entityReferenceOptionalMarkingExpressionVisitor.Visit(pendingSelector);
-        if (!pendingSelector.Type.IsNullableType())
-        {
-            pendingSelector = Expression.Coalesce(
-                Expression.Convert(pendingSelector, pendingSelector.Type.MakeNullable()),
-                pendingSelector.Type.GetDefaultValueConstant());
-        }
+        var result = Expression.Call(
+            QueryableMethods.DefaultIfEmptyWithoutArgument.MakeGenericMethod(queryable.Type.GetSequenceType()),
+            queryable);
 
-        source.ApplySelector(pendingSelector);
+        var navigationTree = new NavigationTreeExpression(newStructure);
+        var parameterName = GetParameterName("e");
 
-        return source;
+        return new NavigationExpansionExpression(result, navigationTree, navigationTree, parameterName);
     }
 
     private NavigationExpansionExpression ProcessDistinct(NavigationExpansionExpression source, MethodInfo genericMethod)

src/Shared/SharedTypeExtensions.cs

@@ -394,7 +394,6 @@ public static MethodInfo GetGenericMethod(
 
     private static readonly Dictionary<Type, object> CommonTypeDictionary = new()
     {
-#pragma warning disable IDE0034 // Simplify 'default' expression - default causes default(object)
         { typeof(int), default(int) },
         { typeof(Guid), default(Guid) },
         { typeof(DateOnly), default(DateOnly) },
@@ -412,25 +411,15 @@ public static MethodInfo GetGenericMethod(
         { typeof(ushort), default(ushort) },
         { typeof(ulong), default(ulong) },
         { typeof(sbyte), default(sbyte) }
-#pragma warning restore IDE0034 // Simplify 'default' expression
     };
 
     public static object? GetDefaultValue(
         [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
         this Type type)
-    {
-        if (!type.IsValueType)
-        {
-            return null;
-        }
+        => type.IsNullableType() ? null : RuntimeHelpers.GetUninitializedObject(type);
 
-        // A bit of perf code to avoid calling Activator.CreateInstance for common types and
-        // to avoid boxing on every call. This is about 50% faster than just calling CreateInstance
-        // for all value types.
-        return CommonTypeDictionary.TryGetValue(type, out var value)
-            ? value
-            : Activator.CreateInstance(type);
-    }
+    public static ConstantExpression GetDefaultValueConstant(this Type type)
+        => Expression.Constant(type.GetDefaultValue(), type);
 
     [RequiresUnreferencedCode("Gets all types from the given assembly - unsafe for trimming")]
     public static IEnumerable<TypeInfo> GetConstructibleTypes(
@@ -648,7 +637,4 @@ public static IEnumerable<string> GetNamespaces(this Type type)
             }
         }
     }
-
-    public static ConstantExpression GetDefaultValueConstant(this Type type)
-        => Expression.Constant(type.IsValueType ? RuntimeHelpers.GetUninitializedObject(type) : null, type);
 }

test/EFCore.Cosmos.FunctionalTests/Query/NorthwindAggregateOperatorsQueryCosmosTest.cs

@@ -2573,26 +2573,26 @@ public override async Task Contains_over_entityType_should_materialize_when_comp
         AssertSql();
     }
 
-    public override async Task Average_after_default_if_empty_does_not_throw(bool async)
+    public override async Task Average_after_DefaultIfEmpty_does_not_throw(bool async)
     {
         // Contains over subquery. Issue #17246.
-        await AssertTranslationFailed(() => base.Average_after_default_if_empty_does_not_throw(async));
+        await AssertTranslationFailed(() => base.Average_after_DefaultIfEmpty_does_not_throw(async));
 
         AssertSql();
     }
 
-    public override async Task Max_after_default_if_empty_does_not_throw(bool async)
+    public override async Task Max_after_DefaultIfEmpty_does_not_throw(bool async)
     {
         // Contains over subquery. Issue #17246.
-        await AssertTranslationFailed(() => base.Max_after_default_if_empty_does_not_throw(async));
+        await AssertTranslationFailed(() => base.Max_after_DefaultIfEmpty_does_not_throw(async));
 
         AssertSql();
     }
 
-    public override async Task Min_after_default_if_empty_does_not_throw(bool async)
+    public override async Task Min_after_DefaultIfEmpty_does_not_throw(bool async)
     {
         // Contains over subquery. Issue #17246.
-        await AssertTranslationFailed(() => base.Min_after_default_if_empty_does_not_throw(async));
+        await AssertTranslationFailed(() => base.Min_after_DefaultIfEmpty_does_not_throw(async));
 
         AssertSql();
     }

test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs

@@ -202,56 +202,64 @@ public override async Task Queryable_reprojection(bool async)
         AssertSql();
     }
 
-    public override async Task Default_if_empty_top_level(bool async)
+    public override async Task DefaultIfEmpty_top_level(bool async)
     {
         // Cosmos client evaluation. Issue #17246.
-        await AssertTranslationFailed(() => base.Default_if_empty_top_level(async));
+        await AssertTranslationFailed(() => base.DefaultIfEmpty_top_level(async));
 
         AssertSql();
     }
 
-    public override async Task Join_with_default_if_empty_on_both_sources(bool async)
+    public override async Task Join_with_DefaultIfEmpty_on_both_sources(bool async)
     {
         // Cosmos client evaluation. Issue #17246.
-        await AssertTranslationFailed(() => base.Join_with_default_if_empty_on_both_sources(async));
+        await AssertTranslationFailed(() => base.Join_with_DefaultIfEmpty_on_both_sources(async));
 
         AssertSql();
     }
 
-    public override async Task Default_if_empty_top_level_followed_by_projecting_constant(bool async)
+    public override async Task DefaultIfEmpty_top_level_followed_by_constant_Select(bool async)
     {
         // Cosmos client evaluation. Issue #17246.
-        await AssertTranslationFailed(() => base.Default_if_empty_top_level_followed_by_projecting_constant(async));
+        await AssertTranslationFailed(() => base.DefaultIfEmpty_top_level_followed_by_constant_Select(async));
 
         AssertSql();
     }
 
-    public override async Task Default_if_empty_top_level_positive(bool async)
+    public override async Task DefaultIfEmpty_top_level_preceded_by_constant_Select(bool async)
     {
         // Cosmos client evaluation. Issue #17246.
-        await AssertTranslationFailed(() => base.Default_if_empty_top_level_positive(async));
+        await AssertTranslationFailed(() => base.DefaultIfEmpty_top_level_preceded_by_constant_Select(async));
 
         AssertSql();
     }
 
-    public override async Task Default_if_empty_top_level_arg(bool async)
+    public override async Task DefaultIfEmpty_top_level_positive(bool async)
     {
-        await base.Default_if_empty_top_level_arg(async);
+        // Cosmos client evaluation. Issue #17246.
+        await AssertTranslationFailed(() => base.DefaultIfEmpty_top_level_positive(async));
+
+        AssertSql();
+    }
+
+    public override async Task DefaultIfEmpty_top_level_arg(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_arg(async);
 
         AssertSql();
     }
 
-    public override async Task Default_if_empty_top_level_arg_followed_by_projecting_constant(bool async)
+    public override async Task DefaultIfEmpty_top_level_arg_followed_by_projecting_constant(bool async)
     {
-        await base.Default_if_empty_top_level_arg_followed_by_projecting_constant(async);
+        await base.DefaultIfEmpty_top_level_arg_followed_by_projecting_constant(async);
 
         AssertSql();
     }
 
-    public override async Task Default_if_empty_top_level_projection(bool async)
+    public override async Task DefaultIfEmpty_top_level_projection(bool async)
     {
         // Cosmos client evaluation. Issue #17246.
-        await AssertTranslationFailed(() => base.Default_if_empty_top_level_projection(async));
+        await AssertTranslationFailed(() => base.DefaultIfEmpty_top_level_projection(async));
 
         AssertSql();
     }

test/EFCore.InMemory.FunctionalTests/Query/NorthwindSelectQueryInMemoryTest.cs

@@ -14,8 +14,4 @@ public override Task
             () => base
                 .SelectMany_with_collection_being_correlated_subquery_which_references_non_mapped_properties_from_inner_and_outer_entity(
                     async));
-
-    public override async Task SelectMany_correlated_with_outer_3(bool async)
-        // DefaultIfEmpty. Issue #17536.
-        => await Assert.ThrowsAsync<EqualException>(() => base.SelectMany_correlated_with_outer_3(async));
 }

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

@@ -1899,21 +1899,21 @@ public virtual Task Min_over_default_returns_default(bool async)
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual Task Average_after_default_if_empty_does_not_throw(bool async)
+    public virtual Task Average_after_DefaultIfEmpty_does_not_throw(bool async)
         => AssertAverage(
             async,
             ss => ss.Set<Order>().Where(o => o.OrderID == 10243).Select(o => o.OrderID).DefaultIfEmpty());
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual Task Max_after_default_if_empty_does_not_throw(bool async)
+    public virtual Task Max_after_DefaultIfEmpty_does_not_throw(bool async)
         => AssertMax(
             async,
             ss => ss.Set<Order>().Where(o => o.OrderID == 10243).Select(o => o.OrderID).DefaultIfEmpty());
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual Task Min_after_default_if_empty_does_not_throw(bool async)
+    public virtual Task Min_after_DefaultIfEmpty_does_not_throw(bool async)
         => AssertMin(
             async,
             ss => ss.Set<Order>().Where(o => o.OrderID == 10243).Select(o => o.OrderID).DefaultIfEmpty());

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

@@ -2232,60 +2232,69 @@ protected class Foo
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual Task Default_if_empty_top_level(bool async)
+    public virtual Task DefaultIfEmpty_top_level(bool async)
         => AssertQuery(
             async,
-            ss => from e in ss.Set<Employee>().Where(c => c.EmployeeID == NonExistentID).DefaultIfEmpty()
-                  select e);
+            ss => ss.Set<Employee>().Where(c => c.EmployeeID == NonExistentID).DefaultIfEmpty());
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual Task Join_with_default_if_empty_on_both_sources(bool async)
+    public virtual Task Join_with_DefaultIfEmpty_on_both_sources(bool async)
         => AssertQuery(
             async,
-            ss => (from e in ss.Set<Employee>().Where(c => c.EmployeeID == NonExistentID).DefaultIfEmpty()
-                   select e).Join(
-                from e in ss.Set<Employee>().Where(c => c.EmployeeID == NonExistentID).DefaultIfEmpty()
-                select e, o => o, i => i, (o, i) => o),
+            ss => ss.Set<Employee>()
+                .Where(c => c.EmployeeID == NonExistentID)
+                .DefaultIfEmpty()
+                .Join(
+                    ss.Set<Employee>()
+                        .Where(c => c.EmployeeID == NonExistentID)
+                        .DefaultIfEmpty(),
+                    o => o, i => i, (o, i) => o),
             assertEmpty: true);
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual Task Default_if_empty_top_level_followed_by_projecting_constant(bool async)
+    public virtual Task DefaultIfEmpty_top_level_followed_by_constant_Select(bool async)
         => AssertQuery(
             async,
-            ss => from e in ss.Set<Employee>().Where(c => c.EmployeeID == NonExistentID).DefaultIfEmpty()
-                  select "Foo");
+            ss => ss.Set<Employee>().Where(c => c.EmployeeID == NonExistentID).DefaultIfEmpty().Select(_ => "Foo"));
+
+    [ConditionalTheory] // #36208
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task DefaultIfEmpty_top_level_preceded_by_constant_Select(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Employee>().Where(e => e.EmployeeID == NonExistentID).Select(_ => "Foo").DefaultIfEmpty());
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual Task Default_if_empty_top_level_arg(bool async)
+    public virtual Task DefaultIfEmpty_top_level_arg(bool async)
         => AssertTranslationFailed(
             () => AssertQuery(
                 async,
-                ss => from e in ss.Set<Employee>().Where(c => c.EmployeeID == NonExistentID).DefaultIfEmpty(new Employee())
-                      select e));
+                ss => ss.Set<Employee>().Where(c => c.EmployeeID == NonExistentID).DefaultIfEmpty(new Employee())));
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual Task Default_if_empty_top_level_arg_followed_by_projecting_constant(bool async)
+    public virtual Task DefaultIfEmpty_top_level_arg_followed_by_projecting_constant(bool async)
         => AssertTranslationFailed(
             () => AssertQueryScalar(
                 async,
-                ss => from e in ss.Set<Employee>().Where(c => c.EmployeeID == NonExistentID).DefaultIfEmpty(new Employee())
-                      select 42));
+                ss => ss.Set<Employee>()
+                    .Where(c => c.EmployeeID == NonExistentID)
+                    .DefaultIfEmpty(new Employee())
+                    .Select(_ => 42)));
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual Task Default_if_empty_top_level_positive(bool async)
+    public virtual Task DefaultIfEmpty_top_level_positive(bool async)
         => AssertQuery(
             async,
-            ss => from e in ss.Set<Employee>().Where(c => c.EmployeeID > 0).DefaultIfEmpty()
-                  select e);
+            ss => ss.Set<Employee>().Where(c => c.EmployeeID > 0).DefaultIfEmpty());
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual Task Default_if_empty_top_level_projection(bool async)
+    public virtual Task DefaultIfEmpty_top_level_projection(bool async)
         => AssertQueryScalar(
             async,
             ss => from e in ss.Set<Employee>().Where(e => e.EmployeeID == NonExistentID).Select(e => e.EmployeeID).DefaultIfEmpty()