Perform alias uniquification in ExecuteUpdate setters (dotnet#31133)

Fixes dotnet#31078
roji · Jun 26, 2023 · 8e4acf4 · 8e4acf4
1 parent c526345
commit 8e4acf4
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 2 deletions.
diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs
@@ -1544,7 +1544,10 @@ static bool AreOtherNonOwnedEntityTypesInTheTable(IEntityType rootType, ITableBa
                         OperatorType: ExpressionType.Equal, Left: ColumnExpression column
                     } sqlBinaryExpression)
                 {
-                    columnValueSetters.Add(new ColumnValueSetter(column, sqlBinaryExpression.Right));
+                    columnValueSetters.Add(
+                        new ColumnValueSetter(
+                            column,
+                            selectExpression.AssignUniqueAliases(sqlBinaryExpression.Right)));
                 }
                 else
                 {

diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
@@ -4188,7 +4188,14 @@ private void AddTable(TableExpressionBase tableExpressionBase, TableReferenceExp
         _tableReferences.Add(tableReferenceExpression);
     }
 
-    private SqlExpression AssignUniqueAliases(SqlExpression expression)
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    [EntityFrameworkInternal]
+    public SqlExpression AssignUniqueAliases(SqlExpression expression)
         => (SqlExpression)new AliasUniquifier(_usedAliases).Visit(expression);
 
     private static string GenerateUniqueAlias(HashSet<string> usedAliases, string currentAlias)

diff --git a/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs
@@ -197,6 +197,66 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
         }
     }
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual async Task Update_with_alias_uniquification_in_setter_subquery(bool async)
+    {
+        var contextFactory = await InitializeAsync<Context31078>();
+        await AssertUpdate(
+            async,
+            contextFactory.CreateContext,
+            ss => ss.Orders.Where(o => o.Id == 1)
+                .Select(o => new
+                {
+                    Order = o,
+                    Total = o.OrderProducts.Sum(op => op.Amount)
+                }),
+            s => s.SetProperty(x => x.Order.Total, x => x.Total),
+            rowsAffectedCount: 1);
+    }
+
+    protected class Context31078 : DbContext
+    {
+        public Context31078(DbContextOptions options)
+            : base(options)
+        {
+        }
+
+        public DbSet<Order> Orders
+            => Set<Order>();
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+        {
+            modelBuilder.Entity<Order>(
+                b =>
+                {
+                    b.Property(o => o.Id).ValueGeneratedNever();
+                    b.HasData(new Order { Id = 1 });
+                });
+
+            modelBuilder.Entity<OrderProduct>(b =>
+            {
+                b.Property(op => op.Id).ValueGeneratedNever();
+                b.HasData(
+                    new OrderProduct { Id = 1, Amount = 8 },
+                    new OrderProduct { Id = 2, Amount = 9 });
+            });
+        }
+    }
+
+    public class Order
+    {
+        public int Id { get; set; }
+        public int Total { get; set; }
+        public ICollection<OrderProduct> OrderProducts { get; set; } = new List<OrderProduct>();
+    }
+
+    public class OrderProduct
+    {
+        public int Id { get; set; }
+        public int Amount { get; set; }
+    }
+
     public class Blog
     {
         public int Id { get; set; }

diff --git a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs
@@ -90,6 +90,22 @@ WHERE [b].[Title] IS NOT NULL AND [b].[Title] LIKE N'Arthur%'
 """);
     }
 
+    public override async Task Update_with_alias_uniquification_in_setter_subquery(bool async)
+    {
+        await base.Update_with_alias_uniquification_in_setter_subquery(async);
+
+        AssertSql(
+"""
+UPDATE [o]
+SET [o].[Total] = (
+    SELECT COALESCE(SUM([o0].[Amount]), 0)
+    FROM [OrderProduct] AS [o0]
+    WHERE [o].[Id] = [o0].[OrderId])
+FROM [Orders] AS [o]
+WHERE [o].[Id] = 1
+""");
+    }
+
     private void AssertSql(params string[] expected)
         => TestSqlLoggerFactory.AssertBaseline(expected);
 

diff --git a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs
@@ -79,6 +79,21 @@ SELECT 1
 """);
     }
 
+    public override async Task Update_with_alias_uniquification_in_setter_subquery(bool async)
+    {
+        await base.Update_with_alias_uniquification_in_setter_subquery(async);
+
+        AssertSql(
+"""
+UPDATE "Orders" AS "o"
+SET "Total" = (
+    SELECT COALESCE(SUM("o0"."Amount"), 0)
+    FROM "OrderProduct" AS "o0"
+    WHERE "o"."Id" = "o0"."OrderId")
+WHERE "o"."Id" = 1
+""");
+    }
+
     protected override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
         => base.AddOptions(builder).ConfigureWarnings(wcb => wcb.Log(SqliteEventId.CompositeKeyWithValueGeneration));