Fix to #15535 - Query: Navigation rewrite does not preserve the type …

…of result

Problem was that we were only storing generic method definition for cardinality reducing operators (First/Single) because nav rewrite changes source types a lot.
However, those operators are applied after pending selector, so they are guaranteed to be of the same type as originally. We only need to track the changes that happen e.g. during member pushdown, e.g.:
customers.FirstOrDefault<Customer>().Id is changed to customers.Select(c => c.Id).FirstOrDefault<int>()

src/EFCore/Query/NavigationExpansion/NavigationExpansionExpression.cs

@@ -3,8 +3,10 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
+using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.EntityFrameworkCore.Query.Expressions.Internal;
 using Microsoft.EntityFrameworkCore.Query.Internal;
 
@@ -105,7 +107,7 @@ public virtual void Print(ExpressionPrinter expressionPrinter)
 
  if (State.PendingCardinalityReducingOperator != null)
  {
- expressionPrinter.StringBuilder.Append(".Pending" + State.PendingCardinalityReducingOperator.Name + "()");
+ expressionPrinter.StringBuilder.Append(".Pending" + State.PendingCardinalityReducingOperator.Name + "<" + State.PendingCardinalityReducingOperator.GetGenericArguments().Single().ShortDisplayName() + ">()");
  }
  }
  }

src/EFCore/Query/NavigationExpansion/Visitors/NavigationExpandingVisitor.cs

@@ -2,6 +2,7 @@
 // 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 System.Reflection;
 using Microsoft.EntityFrameworkCore.Extensions.Internal;
@@ -71,7 +72,8 @@ private Expression ProcessMemberPushdown(
  applyPendingSelector: true,
  navigationExpansionExpression.State.PendingOrderings,
  navigationExpansionExpression.State.PendingIncludeChain,
- navigationExpansionExpression.State.PendingCardinalityReducingOperator,
+ // we need to remap cardinality reducing operator since it's source type has now changed
+ navigationExpansionExpression.State.PendingCardinalityReducingOperator.GetGenericMethodDefinition().MakeGenericMethod(newPendingSelector.Type),
  navigationExpansionExpression.State.PendingTags,
  navigationExpansionExpression.State.CustomRootMappings,
  navigationExpansionExpression.State.MaterializeCollectionNavigation);
@@ -154,7 +156,8 @@ private Expression ProcessMemberPushdown(
  applyPendingSelector: true,
  navigationExpansionExpression.State.PendingOrderings,
  navigationExpansionExpression.State.PendingIncludeChain,
- navigationExpansionExpression.State.PendingCardinalityReducingOperator,
+ // we need to remap cardinality reducing operator since it's source type has now changed
+ navigationExpansionExpression.State.PendingCardinalityReducingOperator.GetGenericMethodDefinition().MakeGenericMethod(combinedKeySelectorBody.Type),
  navigationExpansionExpression.State.PendingTags,
  navigationExpansionExpression.State.CustomRootMappings,
  materializeCollectionNavigation: null);
@@ -186,15 +189,15 @@ private Expression ProcessMemberPushdown(
  }
  }
 
- // TODO idk if thats needed
  var newState = new NavigationExpansionExpressionState(
  navigationExpansionExpression.State.CurrentParameter,
  navigationExpansionExpression.State.SourceMappings,
  Expression.Lambda(boundSelectorBody, navigationExpansionExpression.State.CurrentParameter),
  applyPendingSelector: true,
  navigationExpansionExpression.State.PendingOrderings,
  navigationExpansionExpression.State.PendingIncludeChain,
- navigationExpansionExpression.State.PendingCardinalityReducingOperator,
+ // we need to remap cardinality reducing operator since it's source type has now changed
+ navigationExpansionExpression.State.PendingCardinalityReducingOperator.GetGenericMethodDefinition().MakeGenericMethod(boundSelectorBody.Type),
  navigationExpansionExpression.State.PendingTags,
  navigationExpansionExpression.State.CustomRootMappings,
  navigationExpansionExpression.State.MaterializeCollectionNavigation);
@@ -343,12 +346,12 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression)
  {
  var navigationKeyAccessExpression = NavigationExpansionHelpers.CreateKeyAccessExpression(
  navigationBindingExpression,
- navigationBindingExpression.EntityType.FindPrimaryKey().Properties,
+ new[] { navigationBindingExpression.EntityType.FindPrimaryKey().Properties.First() },
  addNullCheck: true);
 
  var nullKeyExpression = NavigationExpansionHelpers.CreateNullKeyExpression(
  navigationKeyAccessExpression.Type,
- navigationBindingExpression.EntityType.FindPrimaryKey().Properties.Count);
+ keyCount: 1);
 
  var newNavigationExpansionExpressionState = new NavigationExpansionExpressionState(
  navigationExpansionExpression.State.CurrentParameter,
@@ -357,7 +360,8 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression)
  applyPendingSelector: true,
  navigationExpansionExpression.State.PendingOrderings,
  navigationExpansionExpression.State.PendingIncludeChain,
- navigationExpansionExpression.State.PendingCardinalityReducingOperator,
+ // we need to remap cardinality reducing operator since it's source type has now changed
+ navigationExpansionExpression.State.PendingCardinalityReducingOperator?.GetGenericMethodDefinition().MakeGenericMethod(navigationKeyAccessExpression.Type),
  navigationExpansionExpression.State.PendingTags,
  navigationExpansionExpression.State.CustomRootMappings,
  navigationExpansionExpression.State.MaterializeCollectionNavigation);

src/EFCore/Query/NavigationExpansion/Visitors/NavigationExpandingVisitor_MethodCall.cs

@@ -1167,7 +1167,7 @@ private Expression ProcessCardinalityReducingOperation(MethodCallExpression meth
 
  var source = VisitSourceExpression(methodCallExpression.Arguments[0]);
  var applyOrderingsResult = ApplyPendingOrderings(source.Operand, source.State);
- applyOrderingsResult.state.PendingCardinalityReducingOperator = methodCallExpression.Method.GetGenericMethodDefinition();
+ applyOrderingsResult.state.PendingCardinalityReducingOperator = methodCallExpression.Method;
 
  return new NavigationExpansionExpression(applyOrderingsResult.source, applyOrderingsResult.state, methodCallExpression.Type);
  }

src/EFCore/Query/NavigationExpansion/Visitors/NavigationExpansionReducingVisitor.cs

@@ -77,9 +77,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
 
  if (state.PendingCardinalityReducingOperator != null)
  {
- var terminatingOperatorMethodInfo = state.PendingCardinalityReducingOperator.MakeGenericMethod(parameter.Type);
-
- result = Expression.Call(terminatingOperatorMethodInfo, result);
+ result = Expression.Call(state.PendingCardinalityReducingOperator, result);
  }
 
  if (state.MaterializeCollectionNavigation != null)

test/EFCore.Specification.Tests/FindTestBase.cs

@@ -264,7 +264,7 @@ public virtual void Find_derived_type_using_base_set_tracked()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual void Find_derived_using_base_set_type_from_store()
  {
  using (var context = CreateContext())
@@ -297,7 +297,7 @@ public virtual void Find_shadow_key_from_store()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual void Returns_null_for_shadow_key_not_in_store()
  {
  using (var context = CreateContext())
@@ -414,7 +414,7 @@ public virtual async Task Find_int_key_tracked_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Find_int_key_from_store_async()
  {
  using (var context = CreateContext())
@@ -423,7 +423,7 @@ public virtual async Task Find_int_key_from_store_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Returns_null_for_int_key_not_in_store_async()
  {
  using (var context = CreateContext())
@@ -447,7 +447,7 @@ public virtual async Task Find_nullable_int_key_tracked_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Find_nullable_int_key_from_store_async()
  {
  using (var context = CreateContext())
@@ -456,7 +456,7 @@ public virtual async Task Find_nullable_int_key_from_store_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Returns_null_for_nullable_int_key_not_in_store_async()
  {
  using (var context = CreateContext())
@@ -480,7 +480,7 @@ public virtual async Task Find_string_key_tracked_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Find_string_key_from_store_async()
  {
  using (var context = CreateContext())
@@ -489,7 +489,7 @@ public virtual async Task Find_string_key_from_store_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Returns_null_for_string_key_not_in_store_async()
  {
  using (var context = CreateContext())
@@ -514,7 +514,7 @@ public virtual async Task Find_composite_key_tracked_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Find_composite_key_from_store_async()
  {
  using (var context = CreateContext())
@@ -523,7 +523,7 @@ public virtual async Task Find_composite_key_from_store_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Returns_null_for_composite_key_not_in_store_async()
  {
  using (var context = CreateContext())
@@ -547,7 +547,7 @@ public virtual async Task Find_base_type_tracked_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Find_base_type_from_store_async()
  {
  using (var context = CreateContext())
@@ -556,7 +556,7 @@ public virtual async Task Find_base_type_from_store_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Returns_null_for_base_type_not_in_store_async()
  {
  using (var context = CreateContext())
@@ -580,7 +580,7 @@ public virtual async Task Find_derived_type_tracked_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Find_derived_type_from_store_async()
  {
  using (var context = CreateContext())
@@ -591,7 +591,7 @@ public virtual async Task Find_derived_type_from_store_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Returns_null_for_derived_type_not_in_store_async()
  {
  using (var context = CreateContext())
@@ -600,7 +600,7 @@ public virtual async Task Returns_null_for_derived_type_not_in_store_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Find_base_type_using_derived_set_tracked_async()
  {
  using (var context = CreateContext())
@@ -615,7 +615,7 @@ public virtual async Task Find_base_type_using_derived_set_tracked_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Find_base_type_using_derived_set_from_store_async()
  {
  using (var context = CreateContext())
@@ -639,7 +639,7 @@ public virtual async Task Find_derived_type_using_base_set_tracked_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Find_derived_using_base_set_type_from_store_async()
  {
  using (var context = CreateContext())
@@ -663,7 +663,7 @@ public virtual async Task Find_shadow_key_tracked_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Find_shadow_key_from_store_async()
  {
  using (var context = CreateContext())
@@ -672,7 +672,7 @@ public virtual async Task Find_shadow_key_from_store_async()
  }
  }
 
- [Fact(Skip = "Issue#15535")]
+ [Fact]
  public virtual async Task Returns_null_for_shadow_key_not_in_store_async()
  {
  using (var context = CreateContext())

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

@@ -765,8 +765,6 @@ public virtual Task Where_bitwise_or_enum(bool isAsync)
  [MemberData(nameof(IsAsyncData))]
  public virtual Task Bitwise_projects_values_in_select(bool isAsync)
  {
- // Issue#15535
- isAsync = false;
  return AssertFirst<Gear>(
  isAsync,
  gs => gs
@@ -875,9 +873,6 @@ public virtual Task Where_has_flag_with_nullable_parameter(bool isAsync)
  [MemberData(nameof(IsAsyncData))]
  public virtual Task Select_enum_has_flag(bool isAsync)
  {
- //issue #15865
- isAsync = false;
-
  return AssertFirst<Gear>(
  isAsync,
  gs => gs.Where(g => g.Rank.HasFlag(MilitaryRank.Corporal))
@@ -1540,9 +1535,6 @@ where Maybe(t1.Gear, () => t1.Gear.Nickname) == Maybe(t2.Gear, () => t2.Gear.Nic
  [MemberData(nameof(IsAsyncData))]
  public virtual Task Optional_Navigation_Null_Coalesce_To_Clr_Type(bool isAsync)
  {
- //issue #15865
- isAsync = false;
-
  return AssertFirst<Weapon>(
  isAsync,
  ws => ws.OrderBy(w => w.Id).Select(
@@ -3057,9 +3049,6 @@ public virtual Task Count_with_unflattened_groupjoin_is_evaluated_on_client(bool
  [MemberData(nameof(IsAsyncData))]
  public virtual Task FirstOrDefault_with_manually_created_groupjoin_is_translated_to_sql(bool isAsync)
  {
- // issue #15865
- isAsync = false;
-
  return AssertFirstOrDefault<Squad, Gear>(
  isAsync,
  (ss, gs) =>

test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.ResultOperators.cs

@@ -825,9 +825,6 @@ public virtual Task Select_Select_Distinct_Count(bool isAsync)
  [MemberData(nameof(IsAsyncData))]
  public virtual Task Single_Throws(bool isAsync)
  {
- // TODO: See issue#15535
- isAsync = false;
-
  return Assert.ThrowsAsync<InvalidOperationException>(
  async () => await AssertSingle<Customer>(isAsync, cs => cs));
  }
@@ -847,9 +844,6 @@ public virtual Task Single_Predicate(bool isAsync)
  [MemberData(nameof(IsAsyncData))]
  public virtual Task Where_Single(bool isAsync)
  {
- // TODO: See issue#15535
- isAsync = false;
-
  return AssertSingle<Customer>(
  isAsync,
  cs => cs.Where(c => c.CustomerID == "ALFKI"),
@@ -860,9 +854,6 @@ public virtual Task Where_Single(bool isAsync)
  [MemberData(nameof(IsAsyncData))]
  public virtual Task SingleOrDefault_Throws(bool isAsync)
  {
- // TODO: See issue#15535
- isAsync = false;
-
  return Assert.ThrowsAsync<InvalidOperationException>(
  async () =>
  await AssertSingleOrDefault<Customer>(isAsync, cs => cs));
@@ -883,9 +874,6 @@ public virtual Task SingleOrDefault_Predicate(bool isAsync)
  [MemberData(nameof(IsAsyncData))]
  public virtual Task Where_SingleOrDefault(bool isAsync)
  {
- // TODO: See issue#15535
- isAsync = false;
-
  return AssertSingleOrDefault<Customer>(
  isAsync,
  cs => cs.Where(c => c.CustomerID == "ALFKI"),
@@ -896,8 +884,6 @@ public virtual Task Where_SingleOrDefault(bool isAsync)
  [MemberData(nameof(IsAsyncData))]
  public virtual Task First(bool isAsync)
  {
- // TODO: See issue#15535
- isAsync = false;
  return AssertFirst<Customer>(
  isAsync,
  cs => cs.OrderBy(c => c.ContactName),
@@ -919,8 +905,6 @@ public virtual Task First_Predicate(bool isAsync)
  [MemberData(nameof(IsAsyncData))]
  public virtual Task Where_First(bool isAsync)
  {
- // TODO: See issue#15535
- isAsync = false;
  return AssertFirst<Customer>(
  isAsync,
  // ReSharper disable once ReplaceWithSingleCallToFirst
@@ -932,8 +916,6 @@ public virtual Task Where_First(bool isAsync)
  [MemberData(nameof(IsAsyncData))]
  public virtual Task FirstOrDefault(bool isAsync)
  {
- // TODO: See issue#15535
- isAsync = false;
  return AssertFirstOrDefault<Customer>(
  isAsync,
  cs => cs.OrderBy(c => c.ContactName),
@@ -955,8 +937,6 @@ public virtual Task FirstOrDefault_Predicate(bool isAsync)
  [MemberData(nameof(IsAsyncData))]
  public virtual Task Where_FirstOrDefault(bool isAsync)
  {
- // TODO: See issue#15535
- isAsync = false;
  return AssertFirstOrDefault<Customer>(
  isAsync,
  cs => cs.OrderBy(c => c.ContactName).Where(c => c.City == "London"),