Skip to content

Fix DefaultIfEmpty and nullability within SelectMany selector #36238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics.CodeAnalysis;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Query.Internal;
@@ -231,6 +232,16 @@ [new FromSqlExpression(alias, sqlQueryRootExpression.Sql, sqlQueryRootExpression
protected override Expression VisitMethodCall(MethodCallExpression methodCallExpression)
{
var method = methodCallExpression.Method;

if (method.DeclaringType == typeof(RelationalQueryableMethodTranslatingExpressionVisitor)
&& method.IsGenericMethod
&& method.GetGenericMethodDefinition() == _fakeDefaultIfEmptyMethodInfo.Value
&& Visit(methodCallExpression.Arguments[0]) is ShapedQueryExpression source)
{
((SelectExpression)source.QueryExpression).MakeProjectionNullable(_sqlExpressionFactory);
return source.UpdateShaperExpression(MarkShaperNullable(source.ShaperExpression));
}

var translated = base.VisitMethodCall(methodCallExpression);

// For Contains over a collection parameter, if the provider hasn't implemented TranslateCollection (e.g. OPENJSON on SQL
@@ -1196,7 +1207,10 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
&& methodCallExpression.Method.GetGenericMethodDefinition() == QueryableMethods.DefaultIfEmptyWithoutArgument)
{
_defaultIfEmpty = true;
return Visit(methodCallExpression.Arguments[0]);

return Expression.Call(
_fakeDefaultIfEmptyMethodInfo.Value.MakeGenericMethod(methodCallExpression.Method.GetGenericArguments()[0]),
Visit(methodCallExpression.Arguments[0]));
}

if (!SupportsLiftingDefaultIfEmpty(methodCallExpression.Method))
@@ -2273,6 +2287,13 @@ private ShapedQueryExpression CreateShapedQueryExpressionForValuesExpression(
return new ShapedQueryExpression(selectExpression, shaperExpression);
}

private static IQueryable<TSource?> FakeDefaultIfEmpty<TSource>(IQueryable<TSource> source)
=> throw new UnreachableException();

private static readonly Lazy<MethodInfo> _fakeDefaultIfEmptyMethodInfo = new(
() => typeof(RelationalQueryableMethodTranslatingExpressionVisitor)
.GetMethod(nameof(FakeDefaultIfEmpty), BindingFlags.NonPublic | BindingFlags.Static)!);

/// <summary>
/// This visitor has been obsoleted; Extend RelationalTypeMappingPostprocessor instead, and invoke it from
/// <see cref="RelationalQueryTranslationPostprocessor.ProcessTypeMappings" />.
13 changes: 13 additions & 0 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
Original file line number Diff line number Diff line change
@@ -2399,7 +2399,20 @@ [new ProjectionExpression(nullSqlExpression, "empty")],
_tables.Add(dummySelectExpression);
_tables.Add(joinTable);

MakeProjectionNullable(sqlExpressionFactory);
}

/// <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)
{
// 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 (projectionMember, projection) in _projectionMapping)
{
Original file line number Diff line number Diff line change
@@ -3398,7 +3398,7 @@ public override async Task SelectMany_primitive_select_subquery(bool async)
// Cosmos client evaluation. Issue #17246.
Assert.Equal(
CoreStrings.ExpressionParameterizationExceptionSensitive(
"value(Microsoft.EntityFrameworkCore.Query.NorthwindMiscellaneousQueryTestBase`1+<>c__DisplayClass175_0[Microsoft.EntityFrameworkCore.Query.NorthwindQueryCosmosFixture`1[Microsoft.EntityFrameworkCore.TestUtilities.NoopModelCustomizer]]).ss.Set().Any()"),
"value(Microsoft.EntityFrameworkCore.Query.NorthwindMiscellaneousQueryTestBase`1+<>c__DisplayClass177_0[Microsoft.EntityFrameworkCore.Query.NorthwindQueryCosmosFixture`1[Microsoft.EntityFrameworkCore.TestUtilities.NoopModelCustomizer]]).ss.Set().Any()"),
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.SelectMany_primitive_select_subquery(async))).Message);

@@ -3550,6 +3550,22 @@ public override async Task SelectMany_correlated_subquery_simple(bool async)
AssertSql();
}

public override async Task SelectMany_correlated_with_DefaultIfEmpty_and_Select_value_type_in_selector_throws(bool async)
{
// The test "passes" since the base implementation expects InvalidOperation (but for a different reason).
await base.SelectMany_correlated_with_DefaultIfEmpty_and_Select_value_type_in_selector_throws(async);

AssertSql();
}

public override async Task SelectMany_correlated_with_Select_value_type_and_DefaultIfEmpty_in_selector(bool async)
{
// Cosmos client evaluation. Issue #17246.
await AssertTranslationFailed(() => base.SelectMany_correlated_with_Select_value_type_and_DefaultIfEmpty_in_selector(async));

AssertSql();
}

public override async Task SelectMany_correlated_subquery_hard(bool async)
{
// Cosmos client evaluation. Issue #17246.
Original file line number Diff line number Diff line change
@@ -2036,6 +2036,40 @@ from e in ss.Set<Employee>().Where(e => e.City == c.City)
select new { c, e },
assertOrder: true);

// DefaultIfEmpty() over empty set followed by Select() to a non-nullable type - should throw "Nullable object must have a value".
// (same happens for DefaultIfEmpty().Select() at the toplevel without SelectMany)
[ConditionalTheory] // #35950
[MemberData(nameof(IsAsyncData))]
public virtual Task SelectMany_correlated_with_DefaultIfEmpty_and_Select_value_type_in_selector_throws(bool async)
=> Assert.ThrowsAsync<InvalidOperationException>(
() => AssertQuery(
async,
ss =>
from c in ss.Set<Customer>()
from o in c.Orders
.Where(x => x.CustomerID == "NONEXISTENT") // Produce empty set for DefaultIfEmpty
.DefaultIfEmpty()
.Select(x => x.OrderID)
select o));

// DefaultIfEmpty() after Select() to a non-nullable type - should add a COALESCE to the CLR default (0 here).
// Note that within the SelectMany selector, DIE is lifted out (and the INNER JOIN/CROSS APPLY is converted to
// LEFT JOIN/OUTER APPLY). But the COALESCE must still be applied.
[ConditionalTheory] // #35950
[MemberData(nameof(IsAsyncData))]
public virtual Task SelectMany_correlated_with_Select_value_type_and_DefaultIfEmpty_in_selector(bool async)
=> AssertQuery(
async,
ss =>
from c in ss.Set<Customer>()
from o in c.Orders
.Where(x => x.CustomerID == "NONEXISTENT") // Produce empty set for DefaultIfEmpty
.Take(2)
.OrderBy(x => true)
.Select(x => x.OrderID)
.DefaultIfEmpty()
select o);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task SelectMany_correlated_subquery_hard(bool async)
Original file line number Diff line number Diff line change
@@ -6338,6 +6338,38 @@ FROM [Customers] AS [c]
""");
}

public override async Task SelectMany_correlated_with_DefaultIfEmpty_and_Select_value_type_in_selector_throws(bool async)
{
await base.SelectMany_correlated_with_DefaultIfEmpty_and_Select_value_type_in_selector_throws(async);

AssertSql(
"""
SELECT [o0].[OrderID]
FROM [Customers] AS [c]
LEFT JOIN (
SELECT [o].[OrderID], [o].[CustomerID]
FROM [Orders] AS [o]
WHERE [o].[CustomerID] = N'NONEXISTENT'
) AS [o0] ON [c].[CustomerID] = [o0].[CustomerID]
""");
}

public override async Task SelectMany_correlated_with_Select_value_type_and_DefaultIfEmpty_in_selector(bool async)
{
await base.SelectMany_correlated_with_Select_value_type_and_DefaultIfEmpty_in_selector(async);

AssertSql(
"""
SELECT COALESCE([o0].[OrderID], 0)
FROM [Customers] AS [c]
OUTER APPLY (
SELECT TOP(2) [o].[OrderID]
FROM [Orders] AS [o]
WHERE [c].[CustomerID] = [o].[CustomerID] AND [o].[CustomerID] = N'NONEXISTENT'
) AS [o0]
""");
}

public override async Task Select_Property_when_shadow_unconstrained_generic_method(bool async)
{
await base.Select_Property_when_shadow_unconstrained_generic_method(async);
Original file line number Diff line number Diff line change
@@ -323,6 +323,11 @@ public override Task Complex_nested_query_doesnt_try_binding_to_grandparent_when
public override Task SelectMany_correlated_subquery_hard(bool async)
=> null;

public override async Task SelectMany_correlated_with_Select_value_type_and_DefaultIfEmpty_in_selector(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(() => base.SelectMany_correlated_with_Select_value_type_and_DefaultIfEmpty_in_selector(async))).Message);

public override async Task Concat_string_int(bool async)
{
await base.Concat_string_int(async);