Skip to content

Commit

Permalink
Support ExecuteUpdate on entity with owned types
Browse files Browse the repository at this point in the history
Referencing unowned properties only for now.

Fixes dotnet#29618
  • Loading branch information
roji committed Nov 25, 2022
1 parent 124d334 commit f072ffd
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,7 @@ public virtual Expression Translate(SelectExpression selectExpression, Expressio
return null;
}

if (!(expression is NewExpression
|| expression is MemberInitExpression
|| expression is EntityShaperExpression
|| expression is IncludeExpression))
if (expression is not NewExpression and not MemberInitExpression and not EntityShaperExpression and not IncludeExpression)
{
if (_indexBasedBinding)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public class RelationalQueryableMethodTranslatingExpressionVisitor : QueryableMe
private readonly ISqlExpressionFactory _sqlExpressionFactory;
private readonly bool _subquery;

private UpdatePropertySelectorUnwrappingExpressionVisitor? _updatePropertySelectorUnwrappingExpressionVisitor;

/// <summary>
/// Creates a new instance of the <see cref="QueryableMethodTranslatingExpressionVisitor" /> class.
/// </summary>
Expand Down Expand Up @@ -1196,7 +1198,10 @@ static Expression PruneOwnedIncludes(IncludeExpression includeExpression)
{
var left = RemapLambdaBody(source, propertyExpression);

if (!TryProcessPropertyAccess(RelationalDependencies.Model, ref left, out var ese))
_updatePropertySelectorUnwrappingExpressionVisitor ??= new UpdatePropertySelectorUnwrappingExpressionVisitor();
left = _updatePropertySelectorUnwrappingExpressionVisitor.Visit(left);

if (!IsValidPropertyAccess(RelationalDependencies.Model, left, out var ese))
{
AddTranslationErrorDetails(RelationalStrings.InvalidPropertyInSetProperty(propertyExpression.Print()));
return null;
Expand Down Expand Up @@ -1399,46 +1404,29 @@ when methodCallExpression.Method.IsGenericMethod
}
}

static bool TryProcessPropertyAccess(
static bool IsValidPropertyAccess(
IModel model,
ref Expression expression,
Expression expression,
[NotNullWhen(true)] out EntityShaperExpression? entityShaperExpression)
{
// Unwrap any object/base-type Convert nodes around the property access expression
expression = expression.UnwrapTypeConversion(out _);

// Identify property access (direct, EF.Property...), while also unwrapping object/base-type Convert nodes on the expression
// being accessed.
if (expression is MemberExpression memberExpression
&& memberExpression.Expression.UnwrapTypeConversion(out _) is EntityShaperExpression ese)
if (expression is MemberExpression { Expression: EntityShaperExpression ese })
{
expression = memberExpression.Update(ese);

entityShaperExpression = ese;
return true;
}

if (expression is MethodCallExpression mce)
{
if (mce.TryGetEFPropertyArguments(out var source, out _)
&& source.UnwrapTypeConversion(out _) is EntityShaperExpression ese1)
&& source is EntityShaperExpression ese1)
{
if (source != ese1)
{
var rewrittenArguments = mce.Arguments.ToArray();
rewrittenArguments[0] = ese1;
expression = mce.Update(mce.Object, rewrittenArguments);
}

entityShaperExpression = ese1;
return true;
}

if (mce.TryGetIndexerArguments(model, out var source2, out _)
&& source2.UnwrapTypeConversion(out _) is EntityShaperExpression ese2)
&& source2 is EntityShaperExpression ese2)
{
expression = mce.Update(ese2, mce.Arguments);

entityShaperExpression = ese2;
return true;
}
Expand Down Expand Up @@ -1468,6 +1456,29 @@ static Expression GetEntitySource(IModel model, Expression propertyAccessExpress
}
}

// For property setter selectors in ExecuteUpdate, this unwraps casts to interface/base class (#29618), as well as IncludeExpressions
// (which occur when the target entity has owned entities, #28727).
private class UpdatePropertySelectorUnwrappingExpressionVisitor : ExpressionVisitor
{
[return: NotNullIfNotNull(nameof(node))]
public override Expression? Visit(Expression? node)
{
if (node is null)
{
return node;
}

node = node.UnwrapTypeConversion(out _);

if (node is IncludeExpression includeExpression)
{
node = Visit(includeExpression.EntityExpression);
}

return base.Visit(node);
}
}

/// <summary>
/// Checks weather the current select expression can be used as-is for execute a delete operation,
/// or whether it must be pushed down into a subquery.
Expand Down Expand Up @@ -1662,11 +1673,9 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
}

protected override Expression VisitExtension(Expression extensionExpression)
=> extensionExpression is EntityShaperExpression
|| extensionExpression is ShapedQueryExpression
|| extensionExpression is GroupByShaperExpression
? extensionExpression
: base.VisitExtension(extensionExpression);
=> extensionExpression is EntityShaperExpression or ShapedQueryExpression or GroupByShaperExpression
? extensionExpression
: base.VisitExtension(extensionExpression);

private Expression? TryExpand(Expression? source, MemberIdentity member)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,25 @@ public class OtherReference
}

#nullable enable

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Update_non_owned_property_on_entity_with_owned(bool async)
{
var contextFactory = await InitializeAsync<Context28671>(
onModelCreating: mb =>
{
mb.Entity<Owner>().OwnsOne(o => o.OwnedReference);
});

await AssertUpdate(
async,
contextFactory.CreateContext,
ss => ss.Set<Owner>(),
s => s.SetProperty(o => o.Title, "SomeValue"),
rowsAffectedCount: 0);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Delete_predicate_based_on_optional_navigation(bool async)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ public override async Task Delete_aggregate_root_when_table_sharing_with_non_own
AssertSql();
}

public override async Task Update_non_owned_property_on_entity_with_owned(bool async)
{
await base.Update_non_owned_property_on_entity_with_owned(async);

AssertSql(
"""
UPDATE [o]
SET [o].[Title] = N'SomeValue'
FROM [Owner] AS [o]
""");
}

public override async Task Delete_predicate_based_on_optional_navigation(bool async)
{
await base.Delete_predicate_based_on_optional_navigation(async);
Expand Down

0 comments on commit f072ffd

Please sign in to comment.