Fix to #11690 - Handling of casts to nullable type differs between 2.…

…1.0-preview2 and 2.0.0

Problem was that we changed behavior of queries like:

customers.Select(c => (int?)c.Orders.Where(o => false).FirstOrDefault().Id)

In 2.0 this query would return collection of nulls, but in preview2 it would return collection of 0s. We made this change to address the following (very similar) scenario:

customers.Select(c => c.Orders.Where(o => false).Select(o => o.Id).FirstOrDefault())

which should have been returning 0s, but was returning nulls instead.

Fix is to add convert to nullable type whenever we perform member pushdown of a non-nullable property on a query containing FirstOrDefault/SingleOrDefault/LastOrDefault.
However, if the query doesn't require pushdown and *-OrDefault operator is applied on list of non-nullable scalars, rather than list of entities, we keep the query as is.

src/EFCore.Specification.Tests/Query/AsyncGearsOfWarQueryTestBase.cs

@@ -1605,6 +1605,15 @@ await AssertIncludeQuery<Gear>(
  new List<IExpectedInclude> { new ExpectedInclude<Officer>(o => o.Reports, "Reports") });
  }
 
+ [ConditionalFact]
+ public virtual async Task Include_collection_with_complex_OrderBy3()
+ {
+ await AssertIncludeQuery<Gear>(
+ os => os.OfType<Officer>()
+ .Include(o => o.Reports)
+ .OrderBy(o => o.Weapons.OrderBy(w => w.Id).Select(w => w.IsAutomatic).FirstOrDefault()),
+ new List<IExpectedInclude> { new ExpectedInclude<Officer>(o => o.Reports, "Reports") });
+ }
 
  [ConditionalFact]
  public virtual async Task Correlated_collection_with_complex_OrderBy()

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

@@ -617,6 +617,16 @@ public virtual void Where_enum_has_flag()
 
  [ConditionalFact]
  public virtual void Where_enum_has_flag_subquery()
+ {
+ AssertQuery<Gear>(
+ gs => gs.Where(g => g.Rank.HasFlag(gs.OrderBy(x => x.Nickname).ThenBy(x => x.SquadId).Select(x => x.Rank).FirstOrDefault())));
+
+ AssertQuery<Gear>(
+ gs => gs.Where(g => MilitaryRank.Corporal.HasFlag(gs.OrderBy(x => x.Nickname).ThenBy(x => x.SquadId).Select(x => x.Rank).FirstOrDefault())));
+ }
+
+ [ConditionalFact]
+ public virtual void Where_enum_has_flag_subquery_with_pushdown()
  {
  AssertQuery<Gear>(
  gs => gs.Where(g => g.Rank.HasFlag(gs.OrderBy(x => x.Nickname).ThenBy(x => x.SquadId).FirstOrDefault().Rank)));
@@ -1090,13 +1100,27 @@ public virtual void Optional_Navigation_Null_Coalesce_To_Clr_Type()
 
  [ConditionalFact]
  public virtual void Where_subquery_boolean()
+ {
+ AssertQuery<Gear>(
+ gs => gs.Where(g => g.Weapons.OrderBy(w => w.Id).Select(w => w.IsAutomatic).FirstOrDefault()));
+ }
+
+ [ConditionalFact]
+ public virtual void Where_subquery_boolean_with_pushdown()
  {
  AssertQuery<Gear>(
  gs => gs.Where(g => g.Weapons.OrderBy(w => w.Id).FirstOrDefault().IsAutomatic));
  }
 
  [ConditionalFact]
  public virtual void Where_subquery_distinct_firstordefault_boolean()
+ {
+ AssertQuery<Gear>(
+ gs => gs.Where(g => g.HasSoulPatch && g.Weapons.Distinct().OrderBy(w => w.Id).Select(w => w.IsAutomatic).FirstOrDefault()));
+ }
+
+ [ConditionalFact]
+ public virtual void Where_subquery_distinct_firstordefault_boolean_with_pushdown()
  {
  AssertQuery<Gear>(
  gs => gs.Where(g => g.HasSoulPatch && g.Weapons.Distinct().OrderBy(w => w.Id).FirstOrDefault().IsAutomatic));
@@ -1111,7 +1135,23 @@ public virtual void Where_subquery_distinct_first_boolean()
  }
 
  [ConditionalFact]
- public virtual void Where_subquery_distinct_singleordefault_boolean()
+ public virtual void Where_subquery_distinct_singleordefault_boolean1()
+ {
+ AssertQuery<Gear>(
+ gs => gs.OrderBy(g => g.Nickname).Where(g => g.HasSoulPatch && g.Weapons.Where(w => w.Name.Contains("Lancer")).Distinct().Select(w => w.IsAutomatic).SingleOrDefault()),
+ assertOrder: true);
+ }
+
+ [ConditionalFact]
+ public virtual void Where_subquery_distinct_singleordefault_boolean2()
+ {
+ AssertQuery<Gear>(
+ gs => gs.OrderBy(g => g.Nickname).Where(g => g.HasSoulPatch && g.Weapons.Where(w => w.Name.Contains("Lancer")).Select(w => w.IsAutomatic).Distinct().SingleOrDefault()),
+ assertOrder: true);
+ }
+
+ [ConditionalFact]
+ public virtual void Where_subquery_distinct_singleordefault_boolean_with_pushdown()
  {
  AssertQuery<Gear>(
  gs => gs.OrderBy(g => g.Nickname).Where(g => g.HasSoulPatch && g.Weapons.Where(w => w.Name.Contains("Lancer")).Distinct().SingleOrDefault().IsAutomatic),
@@ -1155,6 +1195,13 @@ public virtual void Where_subquery_distinct_last_boolean()
 
  [ConditionalFact]
  public virtual void Where_subquery_distinct_orderby_firstordefault_boolean()
+ {
+ AssertQuery<Gear>(
+ gs => gs.Where(g => g.HasSoulPatch && g.Weapons.Distinct().OrderBy(w => w.Id).Select(w => w.IsAutomatic).FirstOrDefault()));
+ }
+
+ [ConditionalFact]
+ public virtual void Where_subquery_distinct_orderby_firstordefault_boolean_with_pushdown()
  {
  AssertQuery<Gear>(
  gs => gs.Where(g => g.HasSoulPatch && g.Weapons.Distinct().OrderBy(w => w.Id).FirstOrDefault().IsAutomatic));
@@ -4785,6 +4832,16 @@ public virtual void Include_collection_with_complex_OrderBy2()
  new List<IExpectedInclude> { new ExpectedInclude<Officer>(o => o.Reports, "Reports") });
  }
 
+ [ConditionalFact]
+ public virtual void Include_collection_with_complex_OrderBy3()
+ {
+ AssertIncludeQuery<Gear>(
+ os => os.OfType<Officer>()
+ .Include(o => o.Reports)
+ .OrderBy(o => o.Weapons.OrderBy(w => w.Id).Select(w => w.IsAutomatic).FirstOrDefault()),
+ new List<IExpectedInclude> { new ExpectedInclude<Officer>(o => o.Reports, "Reports") });
+ }
+
  [ConditionalFact]
  public virtual void Correlated_collection_with_complex_OrderBy()
  {
@@ -4823,6 +4880,83 @@ public virtual void Cast_to_derived_type_after_OfType_works()
  AssertQuery<Gear>(
  gs => gs.OfType<Officer>().Cast<Officer>());
  }
+
+ [ConditionalFact]
+ public virtual void Select_subquery_boolean()
+ {
+ AssertQueryScalar<Gear>(
+ gs => gs.Select(g => g.Weapons.OrderBy(w => w.Id).Select(w => w.IsAutomatic).FirstOrDefault()));
+ }
+
+ [ConditionalFact]
+ public virtual void Select_subquery_boolean_with_pushdown()
+ {
+ AssertQueryScalar<Gear>(
+ gs => gs.Select(g => g.Weapons.OrderBy(w => w.Id).FirstOrDefault().IsAutomatic));
+ }
+
+ [ConditionalFact]
+ public virtual void Select_subquery_boolean_empty()
+ {
+ AssertQueryScalar<Gear>(
+ gs => gs.Select(g => g.Weapons.Where(w => w.Name == "BFG").OrderBy(w => w.Id).Select(w => w.IsAutomatic).FirstOrDefault()));
+ }
+
+ [ConditionalFact]
+ public virtual void Select_subquery_boolean_empty_with_pushdown()
+ {
+ AssertQueryScalar<Gear>(
+ gs => gs.Select(g => (bool?)g.Weapons.Where(w => w.Name == "BFG").OrderBy(w => w.Id).FirstOrDefault().IsAutomatic),
+ gs => gs.Select(g => (bool?)null));
+ }
+
+ [ConditionalFact]
+ public virtual void Select_subquery_distinct_singleordefault_boolean1()
+ {
+ AssertQueryScalar<Gear>(
+ gs => gs.Where(g => g.HasSoulPatch).Select(g => g.Weapons.Where(w => w.Name.Contains("Lancer")).Distinct().Select(w => w.IsAutomatic).SingleOrDefault()),
+ assertOrder: true);
+ }
+
+ [ConditionalFact]
+ public virtual void Select_subquery_distinct_singleordefault_boolean2()
+ {
+ AssertQueryScalar<Gear>(
+ gs => gs.Where(g => g.HasSoulPatch).Select(g => g.Weapons.Where(w => w.Name.Contains("Lancer")).Select(w => w.IsAutomatic).Distinct().SingleOrDefault()),
+ assertOrder: true);
+ }
+
+ [ConditionalFact]
+ public virtual void Select_subquery_distinct_singleordefault_boolean_with_pushdown()
+ {
+ AssertQueryScalar<Gear>(
+ gs => gs.Where(g => g.HasSoulPatch).Select(g => g.Weapons.Where(w => w.Name.Contains("Lancer")).Distinct().SingleOrDefault().IsAutomatic),
+ assertOrder: true);
+ }
+
+ [ConditionalFact]
+ public virtual void Select_subquery_distinct_singleordefault_boolean_empty1()
+ {
+ AssertQueryScalar<Gear>(
+ gs => gs.Where(g => g.HasSoulPatch).Select(g => g.Weapons.Where(w => w.Name == "BFG").Distinct().Select(w => w.IsAutomatic).SingleOrDefault()));
+ }
+
+ [ConditionalFact]
+ public virtual void Select_subquery_distinct_singleordefault_boolean_empty2()
+ {
+ AssertQueryScalar<Gear>(
+ gs => gs.Where(g => g.HasSoulPatch).Select(g => g.Weapons.Where(w => w.Name == "BFG").Select(w => w.IsAutomatic).Distinct().SingleOrDefault()));
+ }
+
+ [ConditionalFact]
+ public virtual void Select_subquery_distinct_singleordefault_boolean_empty_with_pushdown()
+ {
+ AssertQueryScalar<Gear>(
+ gs => gs.Where(g => g.HasSoulPatch).Select(g => (bool?)g.Weapons.Where(w => w.Name == "BFG").Distinct().SingleOrDefault().IsAutomatic),
+ gs => gs.Where(g => g.HasSoulPatch).Select(g => (bool?)null),
+ assertOrder: true);
+ }
+
  // Remember to add any new tests to Async version of this test class
 
  protected GearsOfWarContext CreateContext() => Fixture.CreateContext();

src/EFCore/Query/ExpressionVisitors/Internal/SubQueryMemberPushDownExpressionVisitor.cs

@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using System.Linq;
 using System.Linq.Expressions;
 using JetBrains.Annotations;
@@ -55,6 +56,24 @@ ro is ConcatResultOperator
  _queryCompilationContext.UpdateMapping(querySourceMapping);
 
  subQueryModel.SelectClause.Selector = VisitMember(memberExpression.Update(subQueryModel.SelectClause.Selector));
+
+ var selector = subQueryModel.SelectClause.Selector;
+ if (IsFirstSingleLastOrDefault(subQueryExpression.QueryModel.ResultOperators.LastOrDefault())
+ && !selector.Type.IsNullableType())
+ {
+ var oldType = selector.Type;
+ subQueryModel.SelectClause.Selector
+ = Expression.Convert(
+ selector,
+ selector.Type.MakeNullable());
+
+ subQueryModel.ResultTypeOverride = subQueryModel.SelectClause.Selector.Type;
+
+ return Expression.Convert(
+ new SubQueryExpression(subQueryModel),
+ oldType);
+ }
+
  subQueryModel.ResultTypeOverride = subQueryModel.SelectClause.Selector.Type;
 
  return new SubQueryExpression(subQueryModel);
@@ -72,22 +91,45 @@ ro is ConcatResultOperator
  var newSubQueryExpression = new SubQueryExpression(queryModel);
 
  var mainFromClause = new MainFromClause(queryModel.GetNewName("subquery"), queryModel.SelectClause.Selector.Type, newSubQueryExpression);
- var selector = Expression.MakeMemberAccess(
+ Expression selector = Expression.MakeMemberAccess(
  new QuerySourceReferenceExpression(mainFromClause),
  memberExpression.Member);
 
- var subqueryModel = new QueryModel(mainFromClause, new SelectClause(selector));
- subqueryModel.ResultOperators.Add(finalResultOperator);
- var subqueryExpression = new SubQueryExpression(subqueryModel);
-
- return subqueryExpression;
+ if (IsFirstSingleLastOrDefault(finalResultOperator)
+ && !selector.Type.IsNullableType())
+ {
+ var oldType = selector.Type;
+ selector = Expression.Convert(
+ selector,
+ selector.Type.MakeNullable());
+
+ var subqueryModel = new QueryModel(mainFromClause, new SelectClause(selector));
+ subqueryModel.ResultOperators.Add(finalResultOperator);
+
+ return Expression.Convert(
+ new SubQueryExpression(subqueryModel),
+ oldType);
+ }
+ else
+ {
+ var subqueryModel = new QueryModel(mainFromClause, new SelectClause(selector));
+ subqueryModel.ResultOperators.Add(finalResultOperator);
+ var subqueryExpression = new SubQueryExpression(subqueryModel);
+
+ return subqueryExpression;
+ }
  }
  }
  }
 
  return memberExpression.Update(newExpression);
  }
 
+ private bool IsFirstSingleLastOrDefault(ResultOperatorBase resultOperator)
+ => (resultOperator is FirstResultOperator first && first.ReturnDefaultWhenEmpty)
+ || (resultOperator is SingleResultOperator single && single.ReturnDefaultWhenEmpty)
+ || (resultOperator is LastResultOperator last && last.ReturnDefaultWhenEmpty);
+
  /// <summary>
  /// This API supports the Entity Framework Core infrastructure and is not intended to be used
  /// directly from your code. This API may change or be removed in future releases.
@@ -121,5 +163,31 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
 
  return newMethodCallExpression;
  }
+
+ /// <summary>
+ /// This API supports the Entity Framework Core infrastructure and is not intended to be used
+ /// directly from your code. This API may change or be removed in future releases.
+ /// </summary>
+ protected override Expression VisitUnary(UnaryExpression unaryExpression)
+ {
+ // remove double convert that could be introduced in the member pushdown (when we introduce cast to nullable for FirstOrDefault cases)
+ if (unaryExpression.NodeType == ExpressionType.Convert
+ || unaryExpression.NodeType == ExpressionType.ConvertChecked)
+ {
+ var newOperand = Visit(unaryExpression.Operand);
+ if (newOperand is UnaryExpression innerUnaryExpression
+ && (innerUnaryExpression.NodeType == ExpressionType.Convert
+ || innerUnaryExpression.NodeType == ExpressionType.ConvertChecked)
+ && innerUnaryExpression.Operand.Type == unaryExpression.Type
+ && innerUnaryExpression.Operand.Type != typeof(object))
+ {
+ return innerUnaryExpression.Operand;
+ }
+
+ return unaryExpression.Update(newOperand);
+ }
+
+ return base.VisitUnary(unaryExpression);
+ }
  }
 }

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

@@ -2221,6 +2221,37 @@ public override async Task Include_collection_with_complex_OrderBy2()
  @"SELECT [o].[Nickname], [o].[SquadId], [o].[AssignedCityName], [o].[CityOrBirthName], [o].[Discriminator], [o].[FullName], [o].[HasSoulPatch], [o].[LeaderNickname], [o].[LeaderSquadId], [o].[Rank]
 FROM [Gears] AS [o]
 WHERE [o].[Discriminator] = N'Officer'
+ORDER BY (
+ SELECT TOP(1) [w].[IsAutomatic]
+ FROM [Weapons] AS [w]
+ WHERE [o].[FullName] = [w].[OwnerFullName]
+ ORDER BY [w].[Id]
+), [o].[Nickname], [o].[SquadId]",
+ //
+ @"SELECT [o.Reports].[Nickname], [o.Reports].[SquadId], [o.Reports].[AssignedCityName], [o.Reports].[CityOrBirthName], [o.Reports].[Discriminator], [o.Reports].[FullName], [o.Reports].[HasSoulPatch], [o.Reports].[LeaderNickname], [o.Reports].[LeaderSquadId], [o.Reports].[Rank]
+FROM [Gears] AS [o.Reports]
+INNER JOIN (
+ SELECT [o0].[Nickname], [o0].[SquadId], (
+ SELECT TOP(1) [w0].[IsAutomatic]
+ FROM [Weapons] AS [w0]
+ WHERE [o0].[FullName] = [w0].[OwnerFullName]
+ ORDER BY [w0].[Id]
+ ) AS [c]
+ FROM [Gears] AS [o0]
+ WHERE [o0].[Discriminator] = N'Officer'
+) AS [t] ON ([o.Reports].[LeaderNickname] = [t].[Nickname]) AND ([o.Reports].[LeaderSquadId] = [t].[SquadId])
+WHERE [o.Reports].[Discriminator] IN (N'Officer', N'Gear')
+ORDER BY [t].[c], [t].[Nickname], [t].[SquadId]");
+ }
+
+ public override async Task Include_collection_with_complex_OrderBy3()
+ {
+ await base.Include_collection_with_complex_OrderBy3();
+
+ AssertSql(
+ @"SELECT [o].[Nickname], [o].[SquadId], [o].[AssignedCityName], [o].[CityOrBirthName], [o].[Discriminator], [o].[FullName], [o].[HasSoulPatch], [o].[LeaderNickname], [o].[LeaderSquadId], [o].[Rank]
+FROM [Gears] AS [o]
+WHERE [o].[Discriminator] = N'Officer'
 ORDER BY COALESCE((
  SELECT TOP(1) [w].[IsAutomatic]
  FROM [Weapons] AS [w]