diff --git a/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs b/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs index dc6a5b3282e..ab382da2f29 100644 --- a/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs @@ -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) { diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs index 7c0e5934361..73938ea9691 100644 --- a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs @@ -1195,8 +1195,8 @@ static Expression PruneOwnedIncludes(IncludeExpression includeExpression) foreach (var (propertyExpression, _) in propertyValueLambdaExpressions) { var left = RemapLambdaBody(source, propertyExpression); - left = left.UnwrapTypeConversion(out _); - if (!IsValidPropertyAccess(RelationalDependencies.Model, left, out var ese)) + + if (!TryProcessPropertyAccess(RelationalDependencies.Model, ref left, out var ese)) { AddTranslationErrorDetails(RelationalStrings.InvalidPropertyInSetProperty(propertyExpression.Print())); return null; @@ -1399,13 +1399,20 @@ when methodCallExpression.Method.IsGenericMethod } } - static bool IsValidPropertyAccess( + // For property setter selectors in ExecuteUpdate, we support only simple member access, EF.Function, etc. + // We also unwrap casts to interface/base class (#29618), as well as IncludeExpressions (which occur when the target entity has + // owned entities, #28727). + static bool TryProcessPropertyAccess( IModel model, - Expression expression, + ref Expression expression, [NotNullWhen(true)] out EntityShaperExpression? entityShaperExpression) { - if (expression is MemberExpression { Expression: EntityShaperExpression ese }) + expression = expression.UnwrapTypeConversion(out _); + + if (expression is MemberExpression { Expression : not null } memberExpression + && Unwrap(memberExpression.Expression) is EntityShaperExpression ese) { + expression = memberExpression.Update(ese); entityShaperExpression = ese; return true; } @@ -1413,15 +1420,23 @@ static bool IsValidPropertyAccess( if (expression is MethodCallExpression mce) { if (mce.TryGetEFPropertyArguments(out var source, out _) - && source is EntityShaperExpression ese1) + && Unwrap(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 is EntityShaperExpression ese2) + && Unwrap(source2) is EntityShaperExpression ese2) { + expression = mce.Update(ese2, mce.Arguments); entityShaperExpression = ese2; return true; } @@ -1429,6 +1444,18 @@ static bool IsValidPropertyAccess( entityShaperExpression = null; return false; + + static Expression Unwrap(Expression expression) + { + expression = expression.UnwrapTypeConversion(out _); + + while (expression is IncludeExpression includeExpression) + { + expression = includeExpression.EntityExpression; + } + + return expression; + } } static Expression GetEntitySource(IModel model, Expression propertyAccessExpression) @@ -1645,11 +1672,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) { diff --git a/src/Shared/ExpressionExtensions.cs b/src/Shared/ExpressionExtensions.cs index 0f63ad0e14d..2455863ca9a 100644 --- a/src/Shared/ExpressionExtensions.cs +++ b/src/Shared/ExpressionExtensions.cs @@ -24,10 +24,10 @@ public static LambdaExpression UnwrapLambdaFromQuote(this Expression expression) public static Expression? UnwrapTypeConversion(this Expression? expression, out Type? convertedType) { convertedType = null; - while (expression is UnaryExpression unaryExpression - && (unaryExpression.NodeType == ExpressionType.Convert - || unaryExpression.NodeType == ExpressionType.ConvertChecked - || unaryExpression.NodeType == ExpressionType.TypeAs)) + while (expression is UnaryExpression + { + NodeType: ExpressionType.Convert or ExpressionType.ConvertChecked or ExpressionType.TypeAs + } unaryExpression) { expression = unaryExpression.Operand; if (unaryExpression.Type != typeof(object) // Ignore object conversion diff --git a/test/EFCore.Relational.Specification.Tests/BulkUpdates/InheritanceBulkUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/BulkUpdates/InheritanceBulkUpdatesTestBase.cs index 08b871a55d8..684ef3b26cb 100644 --- a/test/EFCore.Relational.Specification.Tests/BulkUpdates/InheritanceBulkUpdatesTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/BulkUpdates/InheritanceBulkUpdatesTestBase.cs @@ -158,5 +158,26 @@ public virtual Task Update_where_keyless_entity_mapped_to_sql_query(bool async) s => s.SetProperty(e => e.Name, "Eagle"), rowsAffectedCount: 1)); + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Update_with_interface_in_property_expression(bool async) + => AssertUpdate( + async, + ss => ss.Set(), + e => e, + s => s.SetProperty(c => ((ISugary)c).SugarGrams, 0), + rowsAffectedCount: 1); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Update_with_interface_in_EF_Property_in_property_expression(bool async) + => AssertUpdate( + async, + ss => ss.Set(), + e => e, + // ReSharper disable once RedundantCast + s => s.SetProperty(c => EF.Property((ISugary)c, nameof(ISugary.SugarGrams)), 0), + rowsAffectedCount: 1); + protected abstract void ClearLog(); } diff --git a/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs index d28aa0ef245..10b54dc9b41 100644 --- a/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs @@ -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( + onModelCreating: mb => + { + mb.Entity().OwnsOne(o => o.OwnedReference); + }); + + await AssertUpdate( + async, + contextFactory.CreateContext, + ss => ss.Set(), + 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) diff --git a/test/EFCore.Relational.Specification.Tests/BulkUpdates/TPTInheritanceBulkUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/BulkUpdates/TPTInheritanceBulkUpdatesTestBase.cs index a3f21b1faed..1a4fe584e5e 100644 --- a/test/EFCore.Relational.Specification.Tests/BulkUpdates/TPTInheritanceBulkUpdatesTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/BulkUpdates/TPTInheritanceBulkUpdatesTestBase.cs @@ -56,4 +56,14 @@ public override Task Update_where_hierarchy_derived(bool async) => AssertTranslationFailed( RelationalStrings.ExecuteOperationOnTPT("ExecuteUpdate", "Kiwi"), () => base.Update_where_hierarchy_derived(async)); + + public override Task Update_with_interface_in_property_expression(bool async) + => AssertTranslationFailed( + RelationalStrings.ExecuteOperationOnTPT("ExecuteUpdate", "Coke"), + () => base.Update_with_interface_in_property_expression(async)); + + public override Task Update_with_interface_in_EF_Property_in_property_expression(bool async) + => AssertTranslationFailed( + RelationalStrings.ExecuteOperationOnTPT("ExecuteUpdate", "Coke"), + () => base.Update_with_interface_in_EF_Property_in_property_expression(async)); } diff --git a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/InheritanceBulkUpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/InheritanceBulkUpdatesSqlServerTest.cs index 846cf5f392c..e7dfdc2033d 100644 --- a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/InheritanceBulkUpdatesSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/InheritanceBulkUpdatesSqlServerTest.cs @@ -5,10 +5,14 @@ namespace Microsoft.EntityFrameworkCore.BulkUpdates; public class InheritanceBulkUpdatesSqlServerTest : InheritanceBulkUpdatesTestBase { - public InheritanceBulkUpdatesSqlServerTest(InheritanceBulkUpdatesSqlServerFixture fixture) + public InheritanceBulkUpdatesSqlServerTest( + InheritanceBulkUpdatesSqlServerFixture fixture, + ITestOutputHelper testOutputHelper) + : base(fixture) { ClearLog(); + // Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper); } [ConditionalFact] @@ -205,6 +209,32 @@ public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool AssertExecuteUpdateSql(); } + public override async Task Update_with_interface_in_property_expression(bool async) + { + await base.Update_with_interface_in_property_expression(async); + + AssertExecuteUpdateSql( +""" +UPDATE [d] +SET [d].[SugarGrams] = 0 +FROM [Drinks] AS [d] +WHERE [d].[Discriminator] = N'Coke' +"""); + } + + public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async) + { + await base.Update_with_interface_in_EF_Property_in_property_expression(async); + + AssertExecuteUpdateSql( +""" +UPDATE [d] +SET [d].[SugarGrams] = 0 +FROM [Drinks] AS [d] +WHERE [d].[Discriminator] = N'Coke' +"""); + } + protected override void ClearLog() => Fixture.TestSqlLoggerFactory.Clear(); diff --git a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs index 12a75a08257..e800288c5cf 100644 --- a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs @@ -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); diff --git a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPCInheritanceBulkUpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPCInheritanceBulkUpdatesSqlServerTest.cs index 51b28b3cad0..ca5b94fc7a7 100644 --- a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPCInheritanceBulkUpdatesSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPCInheritanceBulkUpdatesSqlServerTest.cs @@ -5,10 +5,13 @@ namespace Microsoft.EntityFrameworkCore.BulkUpdates; public class TPCInheritanceBulkUpdatesSqlServerTest : TPCInheritanceBulkUpdatesTestBase { - public TPCInheritanceBulkUpdatesSqlServerTest(TPCInheritanceBulkUpdatesSqlServerFixture fixture) + public TPCInheritanceBulkUpdatesSqlServerTest( + TPCInheritanceBulkUpdatesSqlServerFixture fixture, + ITestOutputHelper testOutputHelper) : base(fixture) { ClearLog(); + // Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper); } [ConditionalFact] @@ -176,6 +179,30 @@ FROM [Kiwi] AS [k] """); } + public override async Task Update_with_interface_in_property_expression(bool async) + { + await base.Update_with_interface_in_property_expression(async); + + AssertExecuteUpdateSql( +""" +UPDATE [c] +SET [c].[SugarGrams] = 0 +FROM [Coke] AS [c] +"""); + } + + public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async) + { + await base.Update_with_interface_in_EF_Property_in_property_expression(async); + + AssertExecuteUpdateSql( +""" +UPDATE [c] +SET [c].[SugarGrams] = 0 +FROM [Coke] AS [c] +"""); + } + public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool async) { await base.Update_where_keyless_entity_mapped_to_sql_query(async); diff --git a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPTInheritanceBulkUpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPTInheritanceBulkUpdatesSqlServerTest.cs index 768158f916c..3edf38df652 100644 --- a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPTInheritanceBulkUpdatesSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPTInheritanceBulkUpdatesSqlServerTest.cs @@ -144,6 +144,20 @@ public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool AssertExecuteUpdateSql(); } + public override async Task Update_with_interface_in_property_expression(bool async) + { + await base.Update_with_interface_in_property_expression(async); + + AssertExecuteUpdateSql(); + } + + public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async) + { + await base.Update_with_interface_in_EF_Property_in_property_expression(async); + + AssertExecuteUpdateSql(); + } + protected override void ClearLog() => Fixture.TestSqlLoggerFactory.Clear(); diff --git a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/InheritanceBulkUpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/InheritanceBulkUpdatesSqliteTest.cs index d3f45ba0239..98b92ec2dd7 100644 --- a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/InheritanceBulkUpdatesSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/InheritanceBulkUpdatesSqliteTest.cs @@ -5,10 +5,13 @@ namespace Microsoft.EntityFrameworkCore.BulkUpdates; public class InheritanceBulkUpdatesSqliteTest : InheritanceBulkUpdatesTestBase { - public InheritanceBulkUpdatesSqliteTest(InheritanceBulkUpdatesSqliteFixture fixture) + public InheritanceBulkUpdatesSqliteTest( + InheritanceBulkUpdatesSqliteFixture fixture, + ITestOutputHelper testOutputHelper) : base(fixture) { ClearLog(); + // Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper); } [ConditionalFact] @@ -196,6 +199,30 @@ public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool AssertExecuteUpdateSql(); } + public override async Task Update_with_interface_in_property_expression(bool async) + { + await base.Update_with_interface_in_property_expression(async); + + AssertExecuteUpdateSql( +""" +UPDATE "Drinks" AS "d" +SET "SugarGrams" = 0 +WHERE "d"."Discriminator" = 'Coke' +"""); + } + + public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async) + { + await base.Update_with_interface_in_EF_Property_in_property_expression(async); + + AssertExecuteUpdateSql( +""" +UPDATE "Drinks" AS "d" +SET "SugarGrams" = 0 +WHERE "d"."Discriminator" = 'Coke' +"""); + } + protected override void ClearLog() => Fixture.TestSqlLoggerFactory.Clear(); diff --git a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs index 1b58fe6fc8a..df20f2232fd 100644 --- a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs @@ -39,6 +39,17 @@ 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 "Owner" AS "o" +SET "Title" = 'SomeValue' +"""); + } + public override async Task Delete_predicate_based_on_optional_navigation(bool async) { await base.Delete_predicate_based_on_optional_navigation(async); diff --git a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPCInheritanceBulkUpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPCInheritanceBulkUpdatesSqliteTest.cs index 1d4b44ab21d..08f9b13d8bb 100644 --- a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPCInheritanceBulkUpdatesSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPCInheritanceBulkUpdatesSqliteTest.cs @@ -5,10 +5,13 @@ namespace Microsoft.EntityFrameworkCore.BulkUpdates; public class TPCInheritanceBulkUpdatesSqliteTest : TPCInheritanceBulkUpdatesTestBase { - public TPCInheritanceBulkUpdatesSqliteTest(TPCInheritanceBulkUpdatesSqliteFixture fixture) + public TPCInheritanceBulkUpdatesSqliteTest( + TPCInheritanceBulkUpdatesSqliteFixture fixture, + ITestOutputHelper testOutputHelper) : base(fixture) { ClearLog(); + // Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper); } [ConditionalFact] @@ -177,6 +180,28 @@ public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool AssertExecuteUpdateSql(); } + public override async Task Update_with_interface_in_property_expression(bool async) + { + await base.Update_with_interface_in_property_expression(async); + + AssertExecuteUpdateSql( +""" +UPDATE "Coke" AS "c" +SET "SugarGrams" = 0 +"""); + } + + public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async) + { + await base.Update_with_interface_in_EF_Property_in_property_expression(async); + + AssertExecuteUpdateSql( +""" +UPDATE "Coke" AS "c" +SET "SugarGrams" = 0 +"""); + } + protected override void ClearLog() => Fixture.TestSqlLoggerFactory.Clear(); diff --git a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPTInheritanceBulkUpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPTInheritanceBulkUpdatesSqliteTest.cs index 296affc98e6..198a330eca4 100644 --- a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPTInheritanceBulkUpdatesSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPTInheritanceBulkUpdatesSqliteTest.cs @@ -5,10 +5,13 @@ namespace Microsoft.EntityFrameworkCore.BulkUpdates; public class TPTInheritanceBulkUpdatesSqliteTest : TPTInheritanceBulkUpdatesTestBase { - public TPTInheritanceBulkUpdatesSqliteTest(TPTInheritanceBulkUpdatesSqliteFixture fixture) + public TPTInheritanceBulkUpdatesSqliteTest( + TPTInheritanceBulkUpdatesSqliteFixture fixture, + ITestOutputHelper testOutputHelper) : base(fixture) { ClearLog(); + // Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper); } [ConditionalFact] @@ -142,6 +145,20 @@ public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool AssertExecuteUpdateSql(); } + public override async Task Update_with_interface_in_property_expression(bool async) + { + await base.Update_with_interface_in_property_expression(async); + + AssertExecuteUpdateSql(); + } + + public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async) + { + await base.Update_with_interface_in_EF_Property_in_property_expression(async); + + AssertExecuteUpdateSql(); + } + protected override void ClearLog() => Fixture.TestSqlLoggerFactory.Clear();