Skip to content

Commit 2d68ad0

Browse files
rojiroji
authored
Correct COALESCE logic for DefaultIfEmpty (#37233) (#37237)
Closes #37178 (cherry picked from commit 48b0897)
1 parent deb7fa0 commit 2d68ad0

File tree

8 files changed

+341
-5
lines changed

8 files changed

+341
-5
lines changed

‎src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
245245
&& method.GetGenericMethodDefinition() == _fakeDefaultIfEmptyMethodInfo.Value
246246
&& Visit(methodCallExpression.Arguments[0]) is ShapedQueryExpression source)
247247
{
248-
((SelectExpression)source.QueryExpression).MakeProjectionNullable(_sqlExpressionFactory);
248+
((SelectExpression)source.QueryExpression).MakeProjectionNullable(_sqlExpressionFactory, source.ShaperExpression.Type.IsNullableType());
249249
return source.UpdateShaperExpression(MarkShaperNullable(source.ShaperExpression));
250250
}
251251

@@ -645,7 +645,7 @@ protected override ShapedQueryExpression TranslateConcat(ShapedQueryExpression s
645645
{
646646
if (defaultValue == null)
647647
{
648-
((SelectExpression)source.QueryExpression).ApplyDefaultIfEmpty(_sqlExpressionFactory);
648+
((SelectExpression)source.QueryExpression).ApplyDefaultIfEmpty(_sqlExpressionFactory, source.ShaperExpression.Type.IsNullableType());
649649
return source.UpdateShaperExpression(MarkShaperNullable(source.ShaperExpression));
650650
}
651651

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

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ public sealed partial class SelectExpression : TableExpressionBase
5353

5454
private static ConstructorInfo? _quotingConstructor;
5555

56+
private static readonly bool UseOldBehavior37178 =
57+
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue37178", out var enabled) && enabled;
58+
5659
/// <summary>
5760
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
5861
/// 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)
25002503
/// </summary>
25012504
/// <param name="sqlExpressionFactory">A factory to use for generating required sql expressions.</param>
25022505
public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory)
2506+
=> ApplyDefaultIfEmpty(sqlExpressionFactory, shaperNullable: null);
2507+
2508+
/// <summary>
2509+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
2510+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
2511+
/// any release. You should only use it directly in your code with extreme caution and knowing that
2512+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
2513+
/// </summary>
2514+
[EntityFrameworkInternal]
2515+
public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory, bool shaperNullable)
2516+
=> ApplyDefaultIfEmpty(sqlExpressionFactory, (bool?)shaperNullable);
2517+
2518+
/// <summary>
2519+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
2520+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
2521+
/// any release. You should only use it directly in your code with extreme caution and knowing that
2522+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
2523+
/// </summary>
2524+
[EntityFrameworkInternal]
2525+
public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory, bool? shaperNullable)
25032526
{
25042527
var nullSqlExpression = sqlExpressionFactory.ApplyDefaultTypeMapping(
25052528
new SqlConstantExpression(null, typeof(string), null));
@@ -2527,7 +2550,7 @@ [new ProjectionExpression(nullSqlExpression, "empty")],
25272550
_tables.Add(dummySelectExpression);
25282551
_tables.Add(joinTable);
25292552

2530-
MakeProjectionNullable(sqlExpressionFactory);
2553+
MakeProjectionNullable(sqlExpressionFactory, shaperNullable);
25312554
}
25322555

25332556
/// <summary>
@@ -2537,7 +2560,17 @@ [new ProjectionExpression(nullSqlExpression, "empty")],
25372560
/// doing so can result in application failures when updating to a new Entity Framework Core release.
25382561
/// </summary>
25392562
[EntityFrameworkInternal]
2540-
public void MakeProjectionNullable(ISqlExpressionFactory sqlExpressionFactory)
2563+
public void MakeProjectionNullable(ISqlExpressionFactory sqlExpressionFactory, bool shaperNullable)
2564+
=> MakeProjectionNullable(sqlExpressionFactory, (bool?)shaperNullable);
2565+
2566+
/// <summary>
2567+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
2568+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
2569+
/// any release. You should only use it directly in your code with extreme caution and knowing that
2570+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
2571+
/// </summary>
2572+
[EntityFrameworkInternal]
2573+
public void MakeProjectionNullable(ISqlExpressionFactory sqlExpressionFactory, bool? shaperNullable)
25412574
{
25422575
// Go over all projected columns and make them nullable; for non-nullable value types, add a SQL COALESCE as well.
25432576

@@ -2551,7 +2584,13 @@ public void MakeProjectionNullable(ISqlExpressionFactory sqlExpressionFactory)
25512584
var p => p
25522585
};
25532586

2554-
if (newProjection is SqlExpression { Type: var type } newSqlProjection && !type.IsNullableType())
2587+
// The DefaultIfEmpty translation integrates the original source query as a LEFT JOIN, causing null to be returned when no
2588+
// rows matched (the default case). If the projected type is nullable that's perfect as-is, but if it's a non-nullable value
2589+
// type, we need to apply a COALESCE to get the CLR default instead.
2590+
// Note that the projections observed above in _projectionMapping don't contain accurate nullability information,
2591+
// since SQL expressions get Nullable<T> stripped out. So we instead flow the shaper nullability into here.
2592+
if (newProjection is SqlExpression { Type: var type } newSqlProjection
2593+
&& (UseOldBehavior37178 || shaperNullable is null ? !type.IsNullableType() : shaperNullable is false))
25552594
{
25562595
newProjection = sqlExpressionFactory.Coalesce(
25572596
newSqlProjection,

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4651,6 +4651,33 @@ from w in g.Weapons.Where(ww => ww.Id > prm).DefaultIfEmpty()
46514651
});
46524652
}
46534653

4654+
[ConditionalTheory, MemberData(nameof(IsAsyncData))]
4655+
public virtual Task DefaultIfEmpty_top_level_over_column_with_nullable_value_type(bool async)
4656+
=> AssertQuery(
4657+
async,
4658+
ss => ss.Set<Mission>()
4659+
.Where(m => m.Id == -1) // Non-existent id, to exercise DefaultIfEmpty
4660+
.Select(c => c.Rating)
4661+
.DefaultIfEmpty());
4662+
4663+
[ConditionalTheory, MemberData(nameof(IsAsyncData))]
4664+
public virtual Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(bool async)
4665+
=> AssertQuery(
4666+
async,
4667+
ss => ss.Set<Mission>()
4668+
.Where(m => m.Id == -1) // Non-existent id, to exercise DefaultIfEmpty
4669+
.Select(m => m.Rating + 2)
4670+
.DefaultIfEmpty());
4671+
4672+
[ConditionalTheory, MemberData(nameof(IsAsyncData))]
4673+
public virtual Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(bool async)
4674+
=> AssertQuery(
4675+
async,
4676+
ss => ss.Set<Mission>()
4677+
.Where(m => m.Id == -1) // Non-existent id, to exercise DefaultIfEmpty
4678+
.Select(m => m.Id + 2)
4679+
.DefaultIfEmpty());
4680+
46544681
[ConditionalTheory, MemberData(nameof(IsAsyncData))]
46554682
public virtual Task Join_with_inner_being_a_subquery_projecting_single_property(bool async)
46564683
=> AssertQuery(

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6307,6 +6307,60 @@ WHERE [w].[Id] > @prm
63076307
""");
63086308
}
63096309

6310+
public override async Task DefaultIfEmpty_top_level_over_column_with_nullable_value_type(bool async)
6311+
{
6312+
await base.DefaultIfEmpty_top_level_over_column_with_nullable_value_type(async);
6313+
6314+
AssertSql(
6315+
"""
6316+
SELECT [m0].[Rating]
6317+
FROM (
6318+
SELECT 1 AS empty
6319+
) AS [e]
6320+
LEFT JOIN (
6321+
SELECT [m].[Rating]
6322+
FROM [Missions] AS [m]
6323+
WHERE [m].[Id] = -1
6324+
) AS [m0] ON 1 = 1
6325+
""");
6326+
}
6327+
6328+
public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(bool async)
6329+
{
6330+
await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(async);
6331+
6332+
AssertSql(
6333+
"""
6334+
SELECT [m0].[c]
6335+
FROM (
6336+
SELECT 1 AS empty
6337+
) AS [e]
6338+
LEFT JOIN (
6339+
SELECT [m].[Rating] + 2.0E0 AS [c]
6340+
FROM [Missions] AS [m]
6341+
WHERE [m].[Id] = -1
6342+
) AS [m0] ON 1 = 1
6343+
""");
6344+
}
6345+
6346+
public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(bool async)
6347+
{
6348+
await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(async);
6349+
6350+
AssertSql(
6351+
"""
6352+
SELECT COALESCE([m0].[c], 0)
6353+
FROM (
6354+
SELECT 1 AS empty
6355+
) AS [e]
6356+
LEFT JOIN (
6357+
SELECT [m].[Id] + 2 AS [c]
6358+
FROM [Missions] AS [m]
6359+
WHERE [m].[Id] = -1
6360+
) AS [m0] ON 1 = 1
6361+
""");
6362+
}
6363+
63106364
public override async Task Join_with_inner_being_a_subquery_projecting_single_property(bool async)
63116365
{
63126366
await base.Join_with_inner_being_a_subquery_projecting_single_property(async);

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10941,6 +10941,60 @@ WHERE [w].[Id] > @prm
1094110941
""");
1094210942
}
1094310943

10944+
public override async Task DefaultIfEmpty_top_level_over_column_with_nullable_value_type(bool async)
10945+
{
10946+
await base.DefaultIfEmpty_top_level_over_column_with_nullable_value_type(async);
10947+
10948+
AssertSql(
10949+
"""
10950+
SELECT [m0].[Rating]
10951+
FROM (
10952+
SELECT 1 AS empty
10953+
) AS [e]
10954+
LEFT JOIN (
10955+
SELECT [m].[Rating]
10956+
FROM [Missions] AS [m]
10957+
WHERE [m].[Id] = -1
10958+
) AS [m0] ON 1 = 1
10959+
""");
10960+
}
10961+
10962+
public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(bool async)
10963+
{
10964+
await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(async);
10965+
10966+
AssertSql(
10967+
"""
10968+
SELECT [m0].[c]
10969+
FROM (
10970+
SELECT 1 AS empty
10971+
) AS [e]
10972+
LEFT JOIN (
10973+
SELECT [m].[Rating] + 2.0E0 AS [c]
10974+
FROM [Missions] AS [m]
10975+
WHERE [m].[Id] = -1
10976+
) AS [m0] ON 1 = 1
10977+
""");
10978+
}
10979+
10980+
public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(bool async)
10981+
{
10982+
await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(async);
10983+
10984+
AssertSql(
10985+
"""
10986+
SELECT COALESCE([m0].[c], 0)
10987+
FROM (
10988+
SELECT 1 AS empty
10989+
) AS [e]
10990+
LEFT JOIN (
10991+
SELECT [m].[Id] + 2 AS [c]
10992+
FROM [Missions] AS [m]
10993+
WHERE [m].[Id] = -1
10994+
) AS [m0] ON 1 = 1
10995+
""");
10996+
}
10997+
1094410998
public override async Task Project_entity_and_collection_element(bool async)
1094510999
{
1094611000
await base.Project_entity_and_collection_element(async);

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9241,6 +9241,60 @@ WHERE [w].[Id] > @prm
92419241
""");
92429242
}
92439243

9244+
public override async Task DefaultIfEmpty_top_level_over_column_with_nullable_value_type(bool async)
9245+
{
9246+
await base.DefaultIfEmpty_top_level_over_column_with_nullable_value_type(async);
9247+
9248+
AssertSql(
9249+
"""
9250+
SELECT [m0].[Rating]
9251+
FROM (
9252+
SELECT 1 AS empty
9253+
) AS [e]
9254+
LEFT JOIN (
9255+
SELECT [m].[Rating]
9256+
FROM [Missions] AS [m]
9257+
WHERE [m].[Id] = -1
9258+
) AS [m0] ON 1 = 1
9259+
""");
9260+
}
9261+
9262+
public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(bool async)
9263+
{
9264+
await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(async);
9265+
9266+
AssertSql(
9267+
"""
9268+
SELECT [m0].[c]
9269+
FROM (
9270+
SELECT 1 AS empty
9271+
) AS [e]
9272+
LEFT JOIN (
9273+
SELECT [m].[Rating] + 2.0E0 AS [c]
9274+
FROM [Missions] AS [m]
9275+
WHERE [m].[Id] = -1
9276+
) AS [m0] ON 1 = 1
9277+
""");
9278+
}
9279+
9280+
public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(bool async)
9281+
{
9282+
await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(async);
9283+
9284+
AssertSql(
9285+
"""
9286+
SELECT COALESCE([m0].[c], 0)
9287+
FROM (
9288+
SELECT 1 AS empty
9289+
) AS [e]
9290+
LEFT JOIN (
9291+
SELECT [m].[Id] + 2 AS [c]
9292+
FROM [Missions] AS [m]
9293+
WHERE [m].[Id] = -1
9294+
) AS [m0] ON 1 = 1
9295+
""");
9296+
}
9297+
92449298
public override async Task Project_entity_and_collection_element(bool async)
92459299
{
92469300
await base.Project_entity_and_collection_element(async);

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1714,6 +1714,60 @@ WHERE [w].[Id] > @prm
17141714
""");
17151715
}
17161716

1717+
public override async Task DefaultIfEmpty_top_level_over_column_with_nullable_value_type(bool async)
1718+
{
1719+
await base.DefaultIfEmpty_top_level_over_column_with_nullable_value_type(async);
1720+
1721+
AssertSql(
1722+
"""
1723+
SELECT [m0].[Rating]
1724+
FROM (
1725+
SELECT 1 AS empty
1726+
) AS [e]
1727+
LEFT JOIN (
1728+
SELECT [m].[Rating]
1729+
FROM [Missions] FOR SYSTEM_TIME AS OF '2010-01-01T00:00:00.0000000' AS [m]
1730+
WHERE [m].[Id] = -1
1731+
) AS [m0] ON 1 = 1
1732+
""");
1733+
}
1734+
1735+
public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(bool async)
1736+
{
1737+
await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_nullable_value_type(async);
1738+
1739+
AssertSql(
1740+
"""
1741+
SELECT [m0].[c]
1742+
FROM (
1743+
SELECT 1 AS empty
1744+
) AS [e]
1745+
LEFT JOIN (
1746+
SELECT [m].[Rating] + 2.0E0 AS [c]
1747+
FROM [Missions] FOR SYSTEM_TIME AS OF '2010-01-01T00:00:00.0000000' AS [m]
1748+
WHERE [m].[Id] = -1
1749+
) AS [m0] ON 1 = 1
1750+
""");
1751+
}
1752+
1753+
public override async Task DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(bool async)
1754+
{
1755+
await base.DefaultIfEmpty_top_level_over_arbitrary_expression_with_non_nullable_value_type(async);
1756+
1757+
AssertSql(
1758+
"""
1759+
SELECT COALESCE([m0].[c], 0)
1760+
FROM (
1761+
SELECT 1 AS empty
1762+
) AS [e]
1763+
LEFT JOIN (
1764+
SELECT [m].[Id] + 2 AS [c]
1765+
FROM [Missions] FOR SYSTEM_TIME AS OF '2010-01-01T00:00:00.0000000' AS [m]
1766+
WHERE [m].[Id] = -1
1767+
) AS [m0] ON 1 = 1
1768+
""");
1769+
}
1770+
17171771
public override async Task Select_null_propagation_works_for_navigations_with_composite_keys(bool async)
17181772
{
17191773
await base.Select_null_propagation_works_for_navigations_with_composite_keys(async);

0 commit comments

Comments
 (0)