[release/10.0] Correct COALESCE logic for DefaultIfEmpty (#37233)

src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs

@@ -245,7 +245,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
             && method.GetGenericMethodDefinition() == _fakeDefaultIfEmptyMethodInfo.Value
             && Visit(methodCallExpression.Arguments[0]) is ShapedQueryExpression source)
         {
-            ((SelectExpression)source.QueryExpression).MakeProjectionNullable(_sqlExpressionFactory);
+            ((SelectExpression)source.QueryExpression).MakeProjectionNullable(_sqlExpressionFactory, source.ShaperExpression.Type.IsNullableType());
             return source.UpdateShaperExpression(MarkShaperNullable(source.ShaperExpression));
         }
 
@@ -645,7 +645,7 @@ protected override ShapedQueryExpression TranslateConcat(ShapedQueryExpression s
     {
         if (defaultValue == null)
         {
-            ((SelectExpression)source.QueryExpression).ApplyDefaultIfEmpty(_sqlExpressionFactory);
+            ((SelectExpression)source.QueryExpression).ApplyDefaultIfEmpty(_sqlExpressionFactory, source.ShaperExpression.Type.IsNullableType());
             return source.UpdateShaperExpression(MarkShaperNullable(source.ShaperExpression));
         }
 

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

@@ -53,6 +53,9 @@ public sealed partial class SelectExpression : TableExpressionBase
 
     private static ConstructorInfo? _quotingConstructor;
 
+    private static readonly bool UseOldBehavior37178 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue37178", out var enabled) && enabled;
+
     /// <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
@@ -2500,6 +2503,26 @@ static bool IsNullableProjection(ProjectionExpression projectionExpression)
     /// </summary>
     /// <param name="sqlExpressionFactory">A factory to use for generating required sql expressions.</param>
     public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory)
+        => ApplyDefaultIfEmpty(sqlExpressionFactory, shaperNullable: null);
+
+    /// <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
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    [EntityFrameworkInternal]
+    public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory, bool shaperNullable)
+        => ApplyDefaultIfEmpty(sqlExpressionFactory, (bool?)shaperNullable);
+
+    /// <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
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    [EntityFrameworkInternal]
+    public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory, bool? shaperNullable)
     {
         var nullSqlExpression = sqlExpressionFactory.ApplyDefaultTypeMapping(
             new SqlConstantExpression(null, typeof(string), null));
@@ -2527,7 +2550,7 @@ [new ProjectionExpression(nullSqlExpression, "empty")],
         _tables.Add(dummySelectExpression);
         _tables.Add(joinTable);
 
-        MakeProjectionNullable(sqlExpressionFactory);
+        MakeProjectionNullable(sqlExpressionFactory, shaperNullable);
     }
 
     /// <summary>
@@ -2537,7 +2560,17 @@ [new ProjectionExpression(nullSqlExpression, "empty")],
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     [EntityFrameworkInternal]
-    public void MakeProjectionNullable(ISqlExpressionFactory sqlExpressionFactory)
+    public void MakeProjectionNullable(ISqlExpressionFactory sqlExpressionFactory, bool shaperNullable)
+        => MakeProjectionNullable(sqlExpressionFactory, (bool?)shaperNullable);
+
+    /// <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
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    [EntityFrameworkInternal]
+    public void MakeProjectionNullable(ISqlExpressionFactory sqlExpressionFactory, bool? shaperNullable)
     {
         // Go over all projected columns and make them nullable; for non-nullable value types, add a SQL COALESCE as well.
 
@@ -2551,7 +2584,13 @@ public void MakeProjectionNullable(ISqlExpressionFactory sqlExpressionFactory)
                 var p => p
             };
 
-            if (newProjection is SqlExpression { Type: var type } newSqlProjection && !type.IsNullableType())
+            // The DefaultIfEmpty translation integrates the original source query as a LEFT JOIN, causing null to be returned when no
+            // rows matched (the default case). If the projected type is nullable that's perfect as-is, but if it's a non-nullable value
+            // type, we need to apply a COALESCE to get the CLR default instead.
+            // Note that the projections observed above in _projectionMapping don't contain accurate nullability information,
+            // since SQL expressions get Nullable<T> stripped out. So we instead flow the shaper nullability into here.
+            if (newProjection is SqlExpression { Type: var type } newSqlProjection
+                && (UseOldBehavior37178 || shaperNullable is null ? !type.IsNullableType() : shaperNullable is false))
             {
                 newProjection = sqlExpressionFactory.Coalesce(
                     newSqlProjection,

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

@@ -4651,6 +4651,33 @@ from w in g.Weapons.Where(ww => ww.Id > prm).DefaultIfEmpty()
                   });
     }
 
+    [ConditionalTheory, MemberData(nameof(IsAsyncData))]
+    public virtual Task DefaultIfEmpty_top_level_over_column_with_nullable_value_type(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Mission>()
+                .Where(m => m.Id == -1) // Non-existent id, to exercise DefaultIfEmpty
+                .Select(c => c.Rating)
+                .DefaultIfEmpty());
+
+    [ConditionalTheory, MemberData(nameof(IsAsyncData))]
+    public virtual Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Mission>()
+                .Where(m => m.Id == -1) // Non-existent id, to exercise DefaultIfEmpty
+                .Select(m => m.Rating + 2)
+                .DefaultIfEmpty());
+
+    [ConditionalTheory, MemberData(nameof(IsAsyncData))]
+    public virtual Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Mission>()
+                .Where(m => m.Id == -1) // Non-existent id, to exercise DefaultIfEmpty
+                .Select(m => m.Id + 2)
+                .DefaultIfEmpty());
+
     [ConditionalTheory, MemberData(nameof(IsAsyncData))]
     public virtual Task Join_with_inner_being_a_subquery_projecting_single_property(bool async)
         => AssertQuery(

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

@@ -6307,6 +6307,60 @@ WHERE [w].[Id] > @prm
 """);
     }
 
+    public override async Task DefaultIfEmpty_top_level_over_column_with_nullable_value_type(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_over_column_with_nullable_value_type(async);
+
+        AssertSql(
+            """
+SELECT [m0].[Rating]
+FROM (
+    SELECT 1 AS empty
+) AS [e]
+LEFT JOIN (
+    SELECT [m].[Rating]
+    FROM [Missions] AS [m]
+    WHERE [m].[Id] = -1
+) AS [m0] ON 1 = 1
+""");
+    }
+
+    public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(async);
+
+        AssertSql(
+            """
+SELECT [m0].[c]
+FROM (
+    SELECT 1 AS empty
+) AS [e]
+LEFT JOIN (
+    SELECT [m].[Rating] + 2.0E0 AS [c]
+    FROM [Missions] AS [m]
+    WHERE [m].[Id] = -1
+) AS [m0] ON 1 = 1
+""");
+    }
+
+    public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(async);
+
+        AssertSql(
+            """
+SELECT COALESCE([m0].[c], 0)
+FROM (
+    SELECT 1 AS empty
+) AS [e]
+LEFT JOIN (
+    SELECT [m].[Id] + 2 AS [c]
+    FROM [Missions] AS [m]
+    WHERE [m].[Id] = -1
+) AS [m0] ON 1 = 1
+""");
+    }
+
     public override async Task Join_with_inner_being_a_subquery_projecting_single_property(bool async)
     {
         await base.Join_with_inner_being_a_subquery_projecting_single_property(async);

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

@@ -10941,6 +10941,60 @@ WHERE [w].[Id] > @prm
 """);
     }
 
+    public override async Task DefaultIfEmpty_top_level_over_column_with_nullable_value_type(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_over_column_with_nullable_value_type(async);
+
+        AssertSql(
+            """
+SELECT [m0].[Rating]
+FROM (
+    SELECT 1 AS empty
+) AS [e]
+LEFT JOIN (
+    SELECT [m].[Rating]
+    FROM [Missions] AS [m]
+    WHERE [m].[Id] = -1
+) AS [m0] ON 1 = 1
+""");
+    }
+
+    public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(async);
+
+        AssertSql(
+            """
+SELECT [m0].[c]
+FROM (
+    SELECT 1 AS empty
+) AS [e]
+LEFT JOIN (
+    SELECT [m].[Rating] + 2.0E0 AS [c]
+    FROM [Missions] AS [m]
+    WHERE [m].[Id] = -1
+) AS [m0] ON 1 = 1
+""");
+    }
+
+    public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(async);
+
+        AssertSql(
+            """
+SELECT COALESCE([m0].[c], 0)
+FROM (
+    SELECT 1 AS empty
+) AS [e]
+LEFT JOIN (
+    SELECT [m].[Id] + 2 AS [c]
+    FROM [Missions] AS [m]
+    WHERE [m].[Id] = -1
+) AS [m0] ON 1 = 1
+""");
+    }
+
     public override async Task Project_entity_and_collection_element(bool async)
     {
         await base.Project_entity_and_collection_element(async);

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

@@ -9241,6 +9241,60 @@ WHERE [w].[Id] > @prm
 """);
     }
 
+    public override async Task DefaultIfEmpty_top_level_over_column_with_nullable_value_type(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_over_column_with_nullable_value_type(async);
+
+        AssertSql(
+            """
+SELECT [m0].[Rating]
+FROM (
+    SELECT 1 AS empty
+) AS [e]
+LEFT JOIN (
+    SELECT [m].[Rating]
+    FROM [Missions] AS [m]
+    WHERE [m].[Id] = -1
+) AS [m0] ON 1 = 1
+""");
+    }
+
+    public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(async);
+
+        AssertSql(
+            """
+SELECT [m0].[c]
+FROM (
+    SELECT 1 AS empty
+) AS [e]
+LEFT JOIN (
+    SELECT [m].[Rating] + 2.0E0 AS [c]
+    FROM [Missions] AS [m]
+    WHERE [m].[Id] = -1
+) AS [m0] ON 1 = 1
+""");
+    }
+
+    public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(async);
+
+        AssertSql(
+            """
+SELECT COALESCE([m0].[c], 0)
+FROM (
+    SELECT 1 AS empty
+) AS [e]
+LEFT JOIN (
+    SELECT [m].[Id] + 2 AS [c]
+    FROM [Missions] AS [m]
+    WHERE [m].[Id] = -1
+) AS [m0] ON 1 = 1
+""");
+    }
+
     public override async Task Project_entity_and_collection_element(bool async)
     {
         await base.Project_entity_and_collection_element(async);

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

@@ -1714,6 +1714,60 @@ WHERE [w].[Id] > @prm
 """);
     }
 
+    public override async Task DefaultIfEmpty_top_level_over_column_with_nullable_value_type(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_over_column_with_nullable_value_type(async);
+
+        AssertSql(
+            """
+SELECT [m0].[Rating]
+FROM (
+    SELECT 1 AS empty
+) AS [e]
+LEFT JOIN (
+    SELECT [m].[Rating]
+    FROM [Missions] FOR SYSTEM_TIME AS OF '2010-01-01T00:00:00.0000000' AS [m]
+    WHERE [m].[Id] = -1
+) AS [m0] ON 1 = 1
+""");
+    }
+
+    public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(async);
+
+        AssertSql(
+            """
+SELECT [m0].[c]
+FROM (
+    SELECT 1 AS empty
+) AS [e]
+LEFT JOIN (
+    SELECT [m].[Rating] + 2.0E0 AS [c]
+    FROM [Missions] FOR SYSTEM_TIME AS OF '2010-01-01T00:00:00.0000000' AS [m]
+    WHERE [m].[Id] = -1
+) AS [m0] ON 1 = 1
+""");
+    }
+
+    public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(bool async)
+    {
+        await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(async);
+
+        AssertSql(
+            """
+SELECT COALESCE([m0].[c], 0)
+FROM (
+    SELECT 1 AS empty
+) AS [e]
+LEFT JOIN (
+    SELECT [m].[Id] + 2 AS [c]
+    FROM [Missions] FOR SYSTEM_TIME AS OF '2010-01-01T00:00:00.0000000' AS [m]
+    WHERE [m].[Id] = -1
+) AS [m0] ON 1 = 1
+""");
+    }
+
     public override async Task Select_null_propagation_works_for_navigations_with_composite_keys(bool async)
     {
         await base.Select_null_propagation_works_for_navigations_with_composite_keys(async);