Fix UpdateExpression.VisitChildren to visit the setter column (#33948)

Fixes #33937
dotnet · Jun 11, 2024 · 31a8239 · 31a8239
1 parent e8592c8
commit 31a8239
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 3 deletions.
diff --git a/src/EFCore.Relational/Query/SqlExpressions/UpdateExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/UpdateExpression.cs
@@ -93,20 +93,22 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
         for (var (i, n) = (0, ColumnValueSetters.Count); i < n; i++)
         {
             var columnValueSetter = ColumnValueSetters[i];
+            var newColumn = (ColumnExpression)visitor.Visit(columnValueSetter.Column);
             var newValue = (SqlExpression)visitor.Visit(columnValueSetter.Value);
+
             if (columnValueSetters != null)
             {
-                columnValueSetters.Add(columnValueSetter with { Value = newValue });
+                columnValueSetters.Add(new ColumnValueSetter(newColumn, newValue));
             }
-            else if (!ReferenceEquals(newValue, columnValueSetter.Value))
+            else if (!ReferenceEquals(newColumn, columnValueSetter.Column) || !ReferenceEquals(newValue, columnValueSetter.Value))
             {
                 columnValueSetters = new List<ColumnValueSetter>(n);
                 for (var j = 0; j < i; j++)
                 {
                     columnValueSetters.Add(ColumnValueSetters[j]);
                 }
 
-                columnValueSetters.Add(columnValueSetter with { Value = newValue });
+                columnValueSetters.Add(new ColumnValueSetter(newColumn, newValue));
             }
         }
 

diff --git a/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs
@@ -60,6 +60,20 @@ await AssertTranslationFailedWithDetails(
             RelationalStrings.ExecuteDeleteOnTableSplitting(nameof(Owner)));
     }
 
+    [ConditionalTheory] // #33937, #33946
+    [MemberData(nameof(IsAsyncData))]
+    public virtual async Task Replace_ColumnExpression_in_column_setter(bool async)
+    {
+        var contextFactory = await InitializeAsync<Context28671>();
+
+        await AssertUpdate(
+            async,
+            contextFactory.CreateContext,
+            ss => ss.Set<Owner>().SelectMany(e => e.OwnedCollections),
+            s => s.SetProperty(o => o.Value, "SomeValue"),
+            rowsAffectedCount: 0);
+    }
+
     protected class Context28671(DbContextOptions options) : DbContext(options)
     {
         protected override void OnModelCreating(ModelBuilder modelBuilder)

diff --git a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs
@@ -62,6 +62,19 @@ public override async Task Delete_aggregate_root_when_table_sharing_with_non_own
         AssertSql();
     }
 
+    public override async Task Replace_ColumnExpression_in_column_setter(bool async)
+    {
+        await base.Replace_ColumnExpression_in_column_setter(async);
+
+        AssertSql(
+            """
+UPDATE [o0]
+SET [o0].[Value] = N'SomeValue'
+FROM [Owner] AS [o]
+INNER JOIN [OwnedCollection] AS [o0] ON [o].[Id] = [o0].[OwnerId]
+""");
+    }
+
     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);

diff --git a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs
@@ -1,6 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using Microsoft.Data.Sqlite;
+
 namespace Microsoft.EntityFrameworkCore.BulkUpdates;
 
 #nullable disable
@@ -59,6 +61,20 @@ public override async Task Delete_aggregate_root_when_table_sharing_with_non_own
         AssertSql();
     }
 
+    public override async Task Replace_ColumnExpression_in_column_setter(bool async)
+    {
+        // #33947
+        await Assert.ThrowsAsync<SqliteException>(() => base.Replace_ColumnExpression_in_column_setter(async));
+
+        AssertSql(
+            """
+UPDATE "OwnedCollection" AS "o0"
+SET "Value" = 'SomeValue'
+FROM "Owner" AS "o"
+INNER JOIN "OwnedCollection" AS "o0" ON "o"."Id" = "o0"."OwnerId"
+""");
+    }
+
     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);