From e45c2c2aeba6ed9aee8117fa1a3a84709d61425e Mon Sep 17 00:00:00 2001
From: Shay Rojansky <roji@roji.org>
Date: Mon, 10 Jun 2024 11:49:46 +0200
Subject: [PATCH] Fix UpdateExpression.VisitChildren to visit the setter column

Fixes #33937
---
 .../Query/SqlExpressions/UpdateExpression.cs     |  8 +++++---
 .../NonSharedModelBulkUpdatesTestBase.cs         | 14 ++++++++++++++
 .../NonSharedModelBulkUpdatesSqlServerTest.cs    | 13 +++++++++++++
 .../NonSharedModelBulkUpdatesSqliteTest.cs       | 16 ++++++++++++++++
 4 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/src/EFCore.Relational/Query/SqlExpressions/UpdateExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/UpdateExpression.cs
index 3b0041d2ac5..ec116006b05 100644
--- a/src/EFCore.Relational/Query/SqlExpressions/UpdateExpression.cs
+++ b/src/EFCore.Relational/Query/SqlExpressions/UpdateExpression.cs
@@ -93,12 +93,14 @@ 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++)
@@ -106,7 +108,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
                     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
index 1ff4f844351..3863718e20d 100644
--- 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
index 7629c11c360..5ad9edf7ed3 100644
--- 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
index 49c5e4de35c..57c8c3b6c10 100644
--- 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);