From c66939e5d8f16558cb529f91bf69a615d5dc4db9 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Tue, 25 Nov 2025 10:36:37 +0100 Subject: [PATCH] Correct COALESCE logic for DefaultIfEmpty (#37233) Closes #37178 (cherry picked from commit 48b08978230fad54a2bac315108ef627e5d96ae3) --- ...yableMethodTranslatingExpressionVisitor.cs | 4 +- .../Query/SqlExpressions/SelectExpression.cs | 45 ++++++++++++++-- .../Query/GearsOfWarQueryTestBase.cs | 27 ++++++++++ .../Query/GearsOfWarQuerySqlServerTest.cs | 54 +++++++++++++++++++ .../Query/TPCGearsOfWarQuerySqlServerTest.cs | 54 +++++++++++++++++++ .../Query/TPTGearsOfWarQuerySqlServerTest.cs | 54 +++++++++++++++++++ .../TemporalGearsOfWarQuerySqlServerTest.cs | 54 +++++++++++++++++++ .../Query/GearsOfWarQuerySqliteTest.cs | 54 +++++++++++++++++++ 8 files changed, 341 insertions(+), 5 deletions(-) diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs index b03aa5a0b23..d6bbbe1a20d 100644 --- a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs +++ b/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)); } diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index 2d57cfb6298..b36f1622561 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/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; + /// /// 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) /// /// A factory to use for generating required sql expressions. public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory) + => ApplyDefaultIfEmpty(sqlExpressionFactory, shaperNullable: null); + + /// + /// 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. + /// + [EntityFrameworkInternal] + public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory, bool shaperNullable) + => ApplyDefaultIfEmpty(sqlExpressionFactory, (bool?)shaperNullable); + + /// + /// 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. + /// + [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); } /// @@ -2537,7 +2560,17 @@ [new ProjectionExpression(nullSqlExpression, "empty")], /// doing so can result in application failures when updating to a new Entity Framework Core release. /// [EntityFrameworkInternal] - public void MakeProjectionNullable(ISqlExpressionFactory sqlExpressionFactory) + public void MakeProjectionNullable(ISqlExpressionFactory sqlExpressionFactory, bool shaperNullable) + => MakeProjectionNullable(sqlExpressionFactory, (bool?)shaperNullable); + + /// + /// 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. + /// + [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 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, diff --git a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs index 26fabfcffa7..def20200899 100644 --- a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs +++ b/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() + .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() + .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() + .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( diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs index 98be5c5301e..e79c08d4039 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs +++ b/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); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs index b7564780a5d..fb6a72e683e 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs +++ b/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); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs index 87f087292a6..6f10a8b9225 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs +++ b/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); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs index 79bd89db146..eaf5865171b 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs +++ b/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); diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs index 01826c44bf9..fc72d765c07 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs @@ -5518,6 +5518,60 @@ LEFT JOIN ( """); } + 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 "e" +LEFT JOIN ( + SELECT "m"."Rating" + FROM "Missions" AS "m" + WHERE "m"."Id" = -1 +) AS "m0" ON 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 "e" +LEFT JOIN ( + SELECT "m"."Rating" + 2.0 AS "c" + FROM "Missions" AS "m" + WHERE "m"."Id" = -1 +) AS "m0" ON 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 "e" +LEFT JOIN ( + SELECT "m"."Id" + 2 AS "c" + FROM "Missions" AS "m" + WHERE "m"."Id" = -1 +) AS "m0" ON 1 +"""); + } + public override async Task Select_null_conditional_with_inheritance(bool async) { await base.Select_null_conditional_with_inheritance(async);