Skip to content

Query: Improvements to Navigation Expansion #19377

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jan 1, 2020
Merged
Show file tree
Hide file tree
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
3 changes: 2 additions & 1 deletion src/EFCore.Cosmos/Query/Internal/QuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,8 @@ protected override Expression VisitSelect(SelectExpression selectExpression)
}
else
{
throw new InvalidOperationException(CoreStrings.QueryFailed(selectExpression.Print(), GetType().Name));
// TODO: See Issue#18923
throw new InvalidOperationException("Cosmos Sql API does not support Offset without Limit.");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,8 @@ protected Expression ExpandNavigation(

var outerKeySelector = _navigationExpandingExpressionVisitor.GenerateLambda(
outerKey, _source.CurrentParameter);
var innerKeySelector = _navigationExpandingExpressionVisitor.GenerateLambda(
_navigationExpandingExpressionVisitor.ExpandNavigationsInLambdaExpression(
innerSource,
Expression.Lambda(innerKey, innerParameter)),
innerSource.CurrentParameter);
var innerKeySelector = _navigationExpandingExpressionVisitor.ProcessLambdaExpression(
innerSource, Expression.Lambda(innerKey, innerParameter));

var resultSelectorOuterParameter = Expression.Parameter(_source.SourceElementType, "o");
var resultSelectorInnerParameter = Expression.Parameter(innerSource.SourceElementType, "i");
Expand Down Expand Up @@ -349,10 +346,22 @@ protected override Expression VisitMember(MemberExpression memberExpression)
{
Check.NotNull(memberExpression, nameof(memberExpression));

if (UnwrapEntityReference(memberExpression.Expression) is EntityReference entityReferece)
var innerExpression = memberExpression.Expression.UnwrapTypeConversion(out var convertedType);
if (UnwrapEntityReference(innerExpression) is EntityReference entityReference)
{
// If it is mapped property then, it would get converted to a column so we don't need to expand includes.
var property = entityReferece.EntityType.FindProperty(memberExpression.Member);
var entityType = entityReference.EntityType;
if (convertedType != null)
{
entityType = entityType.GetTypesInHierarchy()
.FirstOrDefault(et => et.ClrType == convertedType);
if (entityType == null)
{
return base.VisitMember(memberExpression);
}
}

var property = entityType.FindProperty(memberExpression.Member);
if (property != null)
{
return memberExpression;
Expand All @@ -366,7 +375,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
{
Check.NotNull(methodCallExpression, nameof(methodCallExpression));

if (methodCallExpression.TryGetEFPropertyArguments(out var _, out var __))
if (methodCallExpression.TryGetEFPropertyArguments(out _, out _))
{
// If it is EF.Property then, it would get converted to a column or throw
// so we don't need to expand includes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Utilities;

Expand Down Expand Up @@ -316,9 +314,6 @@ private set
public virtual NavigationTreeNode Right { get; }
public virtual ParameterExpression CurrentParameter { get; private set; }

protected override Expression VisitChildren(ExpressionVisitor visitor)
=> throw new InvalidOperationException(CoreStrings.QueryFailed(this.Print(), GetType().Name));

public virtual void SetParameter(string parameterName) => CurrentParameter = Parameter(Type, parameterName);

public override ExpressionType NodeType => ExpressionType.Extension;
Expand Down
283 changes: 120 additions & 163 deletions src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp

ShapedQueryExpression CheckTranslated(ShapedQueryExpression translated)
{
if (translated == null)
{
throw new InvalidOperationException(
CoreStrings.TranslationFailed(methodCallExpression.Print()));
}

return translated;
return translated ?? throw new InvalidOperationException(CoreStrings.TranslationFailed(methodCallExpression.Print()));
}

var method = methodCallExpression.Method;
Expand Down
3 changes: 3 additions & 0 deletions src/Shared/EnumerableMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace Microsoft.EntityFrameworkCore
{
internal static class EnumerableMethods
{
public static MethodInfo AsEnumerable { get; }
public static MethodInfo Cast { get; }
public static MethodInfo OfType { get; }

Expand Down Expand Up @@ -130,6 +131,8 @@ static EnumerableMethods()
.GetMethods(BindingFlags.Public | BindingFlags.Static | BindingFlags.DeclaredOnly)
.ToList();

AsEnumerable = enumerableMethods.Single(
mi => mi.Name == nameof(Enumerable.AsEnumerable) && mi.IsGenericMethod && mi.GetParameters().Length == 1);
Cast = enumerableMethods.Single(
mi => mi.Name == nameof(Enumerable.Cast) && mi.GetParameters().Length == 1);
OfType = enumerableMethods.Single(
Expand Down
4 changes: 3 additions & 1 deletion src/Shared/ExpressionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ public static Expression UnwrapTypeConversion(this Expression expression, out Ty
{
convertedType = null;
while (expression is UnaryExpression unaryExpression
&& unaryExpression.NodeType == ExpressionType.Convert)
&& (unaryExpression.NodeType == ExpressionType.Convert
|| unaryExpression.NodeType == ExpressionType.ConvertChecked
|| unaryExpression.NodeType == ExpressionType.TypeAs))
{
expression = unaryExpression.Operand;
if (unaryExpression.Type != typeof(object) // Ignore object conversion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3988,6 +3988,57 @@ public override Task Subquery_DefaultIfEmpty_Any(bool async)
return base.Subquery_DefaultIfEmpty_Any(async);
}

[ConditionalTheory(Skip = "Issue #17246")]
public override Task Projection_skip_collection_projection(bool async)
{
return base.Projection_skip_collection_projection(async);
}

[ConditionalTheory(Skip = "Issue #17246")]
public override Task Projection_take_collection_projection(bool async)
{
return base.Projection_take_collection_projection(async);
}

[ConditionalTheory(Skip = "Issue #17246")]
public override Task Projection_skip_take_collection_projection(bool async)
{
return base.Projection_skip_take_collection_projection(async);
}

public override Task Projection_skip_projection(bool async)
{
return AssertTranslationFailed(() => base.Projection_skip_projection(async));
}

public override Task Projection_take_projection(bool async)
{
return AssertTranslationFailed(() => base.Projection_take_projection(async));
}

public override Task Projection_skip_take_projection(bool async)
{
return AssertTranslationFailed(() => base.Projection_skip_take_projection(async));
}

[ConditionalTheory(Skip = "Issue #17246")]
public override Task Collection_projection_skip(bool async)
{
return base.Collection_projection_skip(async);
}

[ConditionalTheory(Skip = "Issue #17246")]
public override Task Collection_projection_take(bool async)
{
return base.Collection_projection_take(async);
}

[ConditionalTheory(Skip = "Issue #17246")]
public override Task Collection_projection_skip_take(bool async)
{
return base.Collection_projection_skip_take(async);
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down
12 changes: 12 additions & 0 deletions test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,18 @@ public override Task No_ignored_include_warning_when_implicit_load(bool async)
return base.No_ignored_include_warning_when_implicit_load(async);
}

[ConditionalTheory(Skip = "Skip withouth Take #18923")]
public override Task Client_method_skip_loads_owned_navigations(bool async)
{
return base.Client_method_skip_loads_owned_navigations(async);
}

[ConditionalTheory(Skip = "Skip withouth Take #18923")]
public override Task Client_method_skip_loads_owned_navigations_variation_2(bool async)
{
return base.Client_method_skip_loads_owned_navigations_variation_2(async);
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,59 +103,34 @@ public virtual void Throws_when_subquery_main_from_clause()
public virtual void Throws_when_select_many()
{
using var context = CreateContext();
Assert.Equal(
CoreStrings.QueryFailed(
"c1 => int[] { 1, 2, 3, }",
"NavigationExpandingExpressionVisitor"),
Assert.Throws<InvalidOperationException>(
() => (from c1 in context.Customers
from i in new[] { 1, 2, 3 }
select c1)
.ToList()).Message);

AssertTranslationFailed(
() => (from c1 in context.Customers
from i in new[] { 1, 2, 3 }
select c1)
.ToList());
}

[ConditionalFact]
public virtual void Throws_when_join()
{
using var context = CreateContext();
var message = Assert.Throws<InvalidOperationException>(
AssertTranslationFailed(
() => (from e1 in context.Employees
join i in new uint[] { 1, 2, 3 } on e1.EmployeeID equals i
select e1)
.ToList()).Message;

Assert.Equal(
CoreStrings.QueryFailed(
@"DbSet<Employee>
.Join(
inner: __p_0,
outerKeySelector: e1 => e1.EmployeeID,
innerKeySelector: i => i,
resultSelector: (e1, i) => e1)",
"NavigationExpandingExpressionVisitor"),
message, ignoreLineEndingDifferences: true);
.ToList());
}

[ConditionalFact]
public virtual void Throws_when_group_join()
{
using var context = CreateContext();
var message = Assert.Throws<InvalidOperationException>(
AssertTranslationFailed(
() => (from e1 in context.Employees
join i in new uint[] { 1, 2, 3 } on e1.EmployeeID equals i into g
select e1)
.ToList()).Message;

Assert.Equal(
CoreStrings.QueryFailed(
@"DbSet<Employee>
.GroupJoin(
inner: __p_0,
outerKeySelector: e1 => e1.EmployeeID,
innerKeySelector: i => i,
resultSelector: (e1, g) => e1)",
"NavigationExpandingExpressionVisitor"),
message, ignoreLineEndingDifferences: true);
.ToList());
}

[ConditionalFact(Skip = "Issue#18923")]
Expand Down
20 changes: 8 additions & 12 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1559,7 +1559,7 @@ public virtual async Task Select_navigation_with_concat_and_count(bool async)
ss => ss.Set<Gear>().Where(g => !g.HasSoulPatch).Select(g => g.Weapons.Concat(g.Weapons).Count())))).Message;

Assert.Equal(
CoreStrings.QueryFailed(
CoreStrings.TranslationFailed(
@"(MaterializeCollectionNavigation(
navigation: Navigation: Gear.Weapons,
subquery: (NavigationExpansionExpression
Expand Down Expand Up @@ -1588,7 +1588,7 @@ public virtual async Task Select_navigation_with_concat_and_count(bool async)
Value: (EntityReference: Gear)
Expression: g), ""FullName"") != null && EF.Property<string>((NavigationTreeExpression
Value: (EntityReference: Gear)
Expression: g), ""FullName"") == EF.Property<string>(i, ""OwnerFullName""))))", "NavigationExpandingExpressionVisitor"),
Expression: g), ""FullName"") == EF.Property<string>(i, ""OwnerFullName""))))"),
message, ignoreLineEndingDifferences: true);
}

Expand All @@ -1602,7 +1602,7 @@ public virtual async Task Concat_with_collection_navigations(bool async)
ss => ss.Set<Gear>().Where(g => g.HasSoulPatch).Select(g => g.Weapons.Union(g.Weapons).Count())))).Message;

Assert.Equal(
CoreStrings.QueryFailed(
CoreStrings.TranslationFailed(
@"(MaterializeCollectionNavigation(
navigation: Navigation: Gear.Weapons,
subquery: (NavigationExpansionExpression
Expand Down Expand Up @@ -1631,7 +1631,7 @@ public virtual async Task Concat_with_collection_navigations(bool async)
Value: (EntityReference: Gear)
Expression: g), ""FullName"") != null && EF.Property<string>((NavigationTreeExpression
Value: (EntityReference: Gear)
Expression: g), ""FullName"") == EF.Property<string>(i, ""OwnerFullName""))))", "NavigationExpandingExpressionVisitor"),
Expression: g), ""FullName"") == EF.Property<string>(i, ""OwnerFullName""))))"),
message, ignoreLineEndingDifferences: true);
}

Expand Down Expand Up @@ -3106,19 +3106,15 @@ orderby FavoriteWeapon(g.Weapons).Name descending

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Client_method_on_collection_navigation_in_additional_from_clause(bool async)
public virtual Task Client_method_on_collection_navigation_in_additional_from_clause(bool async)
{
var message = (await Assert.ThrowsAsync<InvalidOperationException>(
return AssertTranslationFailed(
() => AssertQuery(
async,
ss => from g in ss.Set<Gear>().OfType<Officer>()
from v in Veterans(g.Reports)
select new { g = g.Nickname, v = v.Nickname },
elementSorter: e => e.g + e.v))).Message;

Assert.StartsWith(
CoreStrings.QueryFailed("", "").Substring(0, 35),
message);
elementSorter: e => e.g + e.v));
}

[ConditionalTheory(Skip = "Issue #17328")]
Expand Down Expand Up @@ -7397,7 +7393,7 @@ public virtual Task OrderBy_bool_coming_from_optional_navigation(bool async)
ss => ss.Set<Weapon>().Select(w => w.SynergyWith).OrderBy(g => MaybeScalar<bool>(g, () => g.IsAutomatic)),
assertOrder: true);
}

[ConditionalFact]
public virtual void Byte_array_filter_by_length_parameter_compiled()
{
Expand Down
Loading