Skip to content

Query: Defer inline-ing owned navigation expansion till all joins are generated #26641

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
1 commit merged into from
Nov 19, 2021
Merged

Query: Defer inline-ing owned navigation expansion till all joins are generated #26641

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
8 changes: 0 additions & 8 deletions src/EFCore.InMemory/Query/Internal/InMemoryQueryableMethodTranslatingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1197,14 +1197,6 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
?? methodCallExpression.Update(null!, new[] { source, methodCallExpression.Arguments[1] });
}

if (methodCallExpression.TryGetEFPropertyArguments(out source, out navigationName))
{
source = Visit(source);

return TryExpand(source, MemberIdentity.Create(navigationName))
?? methodCallExpression.Update(source, new[] { methodCallExpression.Arguments[0] });
}

return base.VisitMethodCall(methodCallExpression);
}

Expand Down
135 changes: 120 additions & 15 deletions src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
Expand Down Expand Up @@ -999,6 +1000,7 @@ private static readonly MethodInfo _objectEqualsMethodInfo
private readonly ISqlExpressionFactory _sqlExpressionFactory;

private SelectExpression _selectExpression;
private DeferredOwnedExpansionRemovingVisitor _deferredOwnedExpansionRemover;

public SharedTypeEntityExpandingExpressionVisitor(
RelationalSqlTranslatingExpressionVisitor sqlTranslator,
Expand All @@ -1007,13 +1009,15 @@ public SharedTypeEntityExpandingExpressionVisitor(
_sqlTranslator = sqlTranslator;
_sqlExpressionFactory = sqlExpressionFactory;
_selectExpression = null!;
_deferredOwnedExpansionRemover = null!;
}

public Expression Expand(SelectExpression selectExpression, Expression lambdaBody)
{
_selectExpression = selectExpression;
_deferredOwnedExpansionRemover = new(_selectExpression);

return Visit(lambdaBody);
return _deferredOwnedExpansionRemover.Visit(Visit(lambdaBody));
}

protected override Expression VisitMember(MemberExpression memberExpression)
Expand All @@ -1034,14 +1038,6 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
?? methodCallExpression.Update(null!, new[] { source, methodCallExpression.Arguments[1] });
}

if (methodCallExpression.TryGetEFPropertyArguments(out source, out navigationName))
{
source = Visit(source);

return TryExpand(source, MemberIdentity.Create(navigationName))
?? methodCallExpression.Update(source, new[] { methodCallExpression.Arguments[1] });
}

return base.VisitMethodCall(methodCallExpression);
}

Expand All @@ -1053,7 +1049,13 @@ protected override Expression VisitExtension(Expression extensionExpression)

private Expression? TryExpand(Expression? source, MemberIdentity member)
{

source = source.UnwrapTypeConversion(out var convertedType);
var doee = source as DeferredOwnedExpansionExpression;
if (doee is not null)
{
source = _deferredOwnedExpansionRemover.UnwrapDeferredEntityProjectionExpression(doee);
}
if (source is not EntityShaperExpression entityShaperExpression)
{
return null;
Expand Down Expand Up @@ -1139,11 +1141,7 @@ outerKey is NewArrayExpression newArrayExpression
Expression.Quote(correlationPredicate));
}

var entityProjectionExpression = (EntityProjectionExpression)
(entityShaperExpression.ValueBufferExpression is ProjectionBindingExpression projectionBindingExpression
? _selectExpression.GetProjection(projectionBindingExpression)
: entityShaperExpression.ValueBufferExpression);

var entityProjectionExpression = GetEntityProjectionExpression(entityShaperExpression);
var innerShaper = entityProjectionExpression.BindNavigation(navigation);
if (innerShaper == null)
{
Expand Down Expand Up @@ -1206,7 +1204,23 @@ outerKey is NewArrayExpression newArrayExpression
makeNullable);

var joinPredicate = _sqlTranslator.Translate(Expression.Equal(outerKey, innerKey))!;
// Following conditions should match conditions for pushdown on outer during SelectExpression.AddJoin method
Copy link
Contributor

@maumar maumar Nov 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

should we add a helper method on select expression for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about it. I had ran into similar for right side of join expression and eventually ended up using a bool out parameter since it was inside SelectExpression. If we expose a method right now then we tie into contract of supporting it/not breaking it ever. So I deferred to put actual method when we really need to expose is, especially if a provider needs to know.

var pushdownRequired = _selectExpression.Limit != null
|| _selectExpression.Offset != null
|| _selectExpression.IsDistinct
|| _selectExpression.GroupBy.Count > 0;
_selectExpression.AddLeftJoin(innerSelectExpression, joinPredicate);

// If pushdown was required on SelectExpression then we need to fetch the updated entity projection
if (pushdownRequired)
{
if (doee is not null)
{
entityShaperExpression = _deferredOwnedExpansionRemover.UnwrapDeferredEntityProjectionExpression(doee);
}
entityProjectionExpression = GetEntityProjectionExpression(entityShaperExpression);
}

var leftJoinTable = _selectExpression.Tables.Last();

innerShaper = new RelationalEntityShaperExpression(
Expand All @@ -1219,13 +1233,104 @@ outerKey is NewArrayExpression newArrayExpression
entityProjectionExpression.AddNavigationBinding(navigation, innerShaper);
}

return innerShaper;
return doee is not null
? doee.AddNavigation(targetEntityType, navigation)
: new DeferredOwnedExpansionExpression(targetEntityType,
(ProjectionBindingExpression)entityShaperExpression.ValueBufferExpression,
navigation);
}

private static Expression AddConvertToObject(Expression expression)
=> expression.Type.IsValueType
? Expression.Convert(expression, typeof(object))
: expression;

private EntityProjectionExpression GetEntityProjectionExpression(EntityShaperExpression entityShaperExpression)
=> entityShaperExpression.ValueBufferExpression switch
{
ProjectionBindingExpression projectionBindingExpression
=> (EntityProjectionExpression)_selectExpression.GetProjection(projectionBindingExpression),
EntityProjectionExpression entityProjectionExpression => entityProjectionExpression,
_ => throw new InvalidOperationException(),
};

private sealed class DeferredOwnedExpansionExpression : Expression
{
private readonly IEntityType _entityType;

public DeferredOwnedExpansionExpression(
IEntityType entityType,
ProjectionBindingExpression projectionBindingExpression,
INavigation navigation)
{
_entityType = entityType;
ProjectionBindingExpression = projectionBindingExpression;
NavigationChain = new List<INavigation> { navigation };
}

private DeferredOwnedExpansionExpression(
IEntityType entityType,
ProjectionBindingExpression projectionBindingExpression,
List<INavigation> navigationChain)
{
_entityType = entityType;
ProjectionBindingExpression = projectionBindingExpression;
NavigationChain = navigationChain;
}

public ProjectionBindingExpression ProjectionBindingExpression { get; }
public List<INavigation> NavigationChain { get; }

public DeferredOwnedExpansionExpression AddNavigation(IEntityType entityType, INavigation navigation)
{
var navigationChain = new List<INavigation>(NavigationChain.Count + 1);
navigationChain.AddRange(NavigationChain);
navigationChain.Add(navigation);

return new DeferredOwnedExpansionExpression(
entityType,
ProjectionBindingExpression,
navigationChain);
}

public override Type Type => _entityType.ClrType;

public override ExpressionType NodeType => ExpressionType.Extension;
}

private sealed class DeferredOwnedExpansionRemovingVisitor : ExpressionVisitor
{
private readonly SelectExpression _selectExpression;

public DeferredOwnedExpansionRemovingVisitor(SelectExpression selectExpression)
{
_selectExpression = selectExpression;
}

[return: NotNullIfNotNull("expression")]
public override Expression? Visit(Expression? expression)
=> expression switch
{
DeferredOwnedExpansionExpression doee => UnwrapDeferredEntityProjectionExpression(doee),
// For the source entity shaper or owned collection expansion
EntityShaperExpression _ or ShapedQueryExpression _ => expression,
_ => base.Visit(expression),
};

public EntityShaperExpression UnwrapDeferredEntityProjectionExpression(DeferredOwnedExpansionExpression doee)
{
var entityProjection = (EntityProjectionExpression)_selectExpression.GetProjection(doee.ProjectionBindingExpression);
var entityShaper = entityProjection.BindNavigation(doee.NavigationChain[0])!;

for (var i = 1; i < doee.NavigationChain.Count; i++)
{
entityProjection = (EntityProjectionExpression)entityShaper.ValueBufferExpression;
entityShaper = entityProjection.BindNavigation(doee.NavigationChain[i])!;
}

return entityShaper;
}
}
}

private ShapedQueryExpression TranslateTwoParameterSelector(ShapedQueryExpression source, LambdaExpression resultSelector)
Expand Down
28 changes: 28 additions & 0 deletions test/EFCore.InMemory.FunctionalTests/Query/OwnedEntityQueryInMemoryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,33 @@ protected class Foo
public virtual Bar? Bar { get; set; }
}
#nullable disable

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Owned_references_on_same_level_expanded_at_different_times_around_take(bool async)
{
var contextFactory = await InitializeAsync<MyContext26592>(seed: c => c.Seed());
using var context = contextFactory.CreateContext();

await base.Owned_references_on_same_level_expanded_at_different_times_around_take_helper(context, async);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Owned_references_on_same_level_nested_expanded_at_different_times_around_take(bool async)
{
var contextFactory = await InitializeAsync<MyContext26592>(seed: c => c.Seed());
using var context = contextFactory.CreateContext();

await base.Owned_references_on_same_level_nested_expanded_at_different_times_around_take_helper(context, async);
}

protected class MyContext26592 : MyContext26592Base
{
public MyContext26592(DbContextOptions options)
: base(options)
{
}
}
}
}
49 changes: 49 additions & 0 deletions test/EFCore.Relational.Specification.Tests/Query/OwnedEntityQueryRelationalTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,5 +164,54 @@ protected class PublishTokenType25680
public string TokenGroupId { get; set; }
public string IssuerName { get; set; }
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Owned_reference_mapped_to_different_table_updated_correctly_after_subquery_pushdown(bool async)
{
var contextFactory = await InitializeAsync<MyContext26592>(seed: c => c.Seed());
using var context = contextFactory.CreateContext();

await base.Owned_references_on_same_level_expanded_at_different_times_around_take_helper(context, async);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Owned_reference_mapped_to_different_table_nested_updated_correctly_after_subquery_pushdown(bool async)
{
var contextFactory = await InitializeAsync<MyContext26592>(seed: c => c.Seed());
using var context = contextFactory.CreateContext();

await base.Owned_references_on_same_level_nested_expanded_at_different_times_around_take_helper(context, async);
}

protected class MyContext26592 : MyContext26592Base
{
public MyContext26592(DbContextOptions options)
: base(options)
{
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Company>(
b =>
{
b.OwnsOne(e => e.CustomerData).ToTable("CustomerData");
b.OwnsOne(e => e.SupplierData).ToTable("SupplierData");
});

modelBuilder.Entity<Owner>(
b =>
{
b.OwnsOne(e => e.OwnedEntity, o =>
{
o.ToTable("IntermediateOwnedEntity");
o.OwnsOne(e => e.CustomerData).ToTable("IM_CustomerData");
o.OwnsOne(e => e.SupplierData).ToTable("IM_SupplierData");
});
});
}
}
}
}
Loading