Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix to #15535 - Query: Navigation rewrite does not preserve the type of result #15951

Merged
merged 1 commit into from
Jun 5, 2019
Merged

Fix to #15535 - Query: Navigation rewrite does not preserve the type of result #15951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion src/EFCore/Query/NavigationExpansion/NavigationExpansionExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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() + ">()");
}
}
}
Expand Down
18 changes: 11 additions & 7 deletions src/EFCore/Query/NavigationExpansion/Visitors/NavigationExpandingVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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() },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this entire code will be removed once entity equality is in

addNullCheck: true);

var nullKeyExpression = NavigationExpansionHelpers.CreateNullKeyExpression(
navigationKeyAccessExpression.Type,
navigationBindingExpression.EntityType.FindPrimaryKey().Properties.Count);
keyCount: 1);

var newNavigationExpansionExpressionState = new NavigationExpansionExpressionState(
navigationExpansionExpression.State.CurrentParameter,
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Query/NavigationExpansion/Visitors/NavigationExpandingVisitor_MethodCall.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 1 addition & 3 deletions src/EFCore/Query/NavigationExpansion/Visitors/NavigationExpansionReducingVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
38 changes: 19 additions & 19 deletions test/EFCore.Specification.Tests/FindTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand Down
11 changes: 0 additions & 11 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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) =>
Expand Down
20 changes: 0 additions & 20 deletions test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.ResultOperators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand All @@ -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"),
Expand All @@ -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));
Expand All @@ -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"),
Expand All @@ -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),
Expand All @@ -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
Expand All @@ -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),
Expand All @@ -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"),
Expand Down
Loading