Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL aliases aren't uniquified in ExecuteUpdate setters #31078

Closed
hexianggui opened this issue Jun 14, 2023 · 15 comments · Fixed by #31133 or #31208
Closed

SQL aliases aren't uniquified in ExecuteUpdate setters #31078

hexianggui opened this issue Jun 14, 2023 · 15 comments · Fixed by #31133 or #31208
Assignees
Labels
area-bulkcud closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Milestone

Comments

@hexianggui
Copy link

I use ExecuteUpdateAsync to update data, but it seems that the generated SQL statement has a problem. Here is the code:

var dbContext = await GetDbContextAsync();
var workQuery = dbContext.WorkOrders.Where(x => x.OrderCode == code);
await dbContext.Set<BomItem>()
    .Where(x => x.OrderCode == code)
    .ExecuteUpdateAsync(x =>
        x.SetProperty(s => s.HasWorkOrder, b => workQuery.Where(o => o.BomItemId == b.Id).Any()));

The incorrect SQL statement is:

2023-06-14_111842

The correct SQL statement should be:

UPDATE [m]
SET [m].[HasWorkOrder] = CASE
    WHEN EXISTS (
        SELECT 1
        FROM [MesWorkOrder] AS [m1]
        WHERE [m1].[OrderCode] = @__code_0 AND [m1].[BomItemId] = [m].[Id]) THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END
FROM [MesBomItem] AS [m]
WHERE [m].[OrderCode] = @__code_0

EF Core version:
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: NET 7.0
Operating system:
IDE: Visual Studio 2022 17.6

@ajcvickers
Copy link
Member

I am not able to reproduce this--see my code below. Please attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

using (var context = new SomeDbContext())
{
    await context.Database.EnsureDeletedAsync();
    await context.Database.EnsureCreatedAsync();

    int code = 1;
    var workQuery = context.WorkOrders.Where(x => x.OrderCode == code);
    await context.Set<BomItem>()
        .Where(x => x.OrderCode == code)
        .ExecuteUpdateAsync(x =>
            x.SetProperty(s => s.HasWorkOrder, b => workQuery.Where(o => o.BomItemId == b.Id).Any()));
}

public class SomeDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer(@"Data Source=(LocalDb)\MSSQLLocalDB;Database=AllTogetherNow")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    public DbSet<WorkOrder> WorkOrders => Set<WorkOrder>();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<BomItem>();
    }
}

public class WorkOrder
{
    public int Id { get; set; }
    public int OrderCode { get; set; }
    public int BomItemId { get; set; }
}

public class BomItem
{
    public int Id { get; set; }
    public int OrderCode { get; set; }
    public bool HasWorkOrder { get; set; }
}
info: 6/19/2023 13:56:00.578 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (18ms) [Parameters=[@__code_0='1'], CommandType='Text', CommandTimeout='30']
      UPDATE [b]
      SET [b].[HasWorkOrder] = CASE
          WHEN EXISTS (
              SELECT 1
              FROM [WorkOrders] AS [w]
              WHERE [w].[OrderCode] = @__code_0 AND [w].[BomItemId] = [b].[Id]) THEN CAST(1 AS bit)
          ELSE CAST(0 AS bit)
      END
      FROM [BomItem] AS [b]
      WHERE [b].[OrderCode] = @__code_0

@KernelCrap
Copy link

KernelCrap commented Jun 23, 2023

We are seeing this on both SQL Server and PostgreSQL:

using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;

using (var context = new ExampleDbContext())
{
    await context.Database.EnsureDeletedAsync();
    await context.Database.EnsureCreatedAsync();

    var id = Guid.NewGuid();

    await context.Orders
        .Where(o => o.Id == id)
        .Select(o => new
        {
            Order = o,
            Total = o.OrderProducts.Sum(op => op.Amount * op.TotalCost)
        })
        .ExecuteUpdateAsync(e => e
            .SetProperty(x => x.Order.Total, x => x.Total));
}

public class ExampleDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer(@"Data Source=(LocalDb)\MSSQLLocalDB;Database=test")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    public DbSet<Order> Orders => Set<Order>();
    public DbSet<OrderProduct> OrderProducts => Set<OrderProduct>();
}

public class Order
{
    public Guid Id { get; set; }
    public decimal Total { get; set; }
    public ICollection<OrderProduct> OrderProducts { get; set; } = new List<OrderProduct>();
}

public class OrderProduct
{
    public Guid Id { get; set; }
    public int Amount { get; set; }
    public decimal TotalCost { get; set; }
}

Output for SQL Server:

info: 23/06/2023 13.31.58.648 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (25ms) [Parameters=[@__id_0='4be338c6-7517-4b71-848c-8e2c0ef244fb'], CommandType='Text', CommandTimeout='30']
      UPDATE [o]
      SET [o].[Total] = (
          SELECT COALESCE(SUM(CAST([o].[Amount] AS decimal(18,2)) * [o].[TotalCost]), 0.0)
          FROM [OrderProducts] AS [o]
          WHERE [o].[Id] = [o].[OrderId])
      FROM [Orders] AS [o]
      WHERE [o].[Id] = @__id_0

Both Orders and OrderProducts are given the same alias so the condition will never be true:
WHERE [o].[Id] = [o].[OrderId]

Output for PostgreSQL:

info: 23/06/2023 13.33.29.703 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (27ms) [Parameters=[@__id_0='5e06b091-dc30-440f-abb8-2f0b71081bfb'], CommandType='Text', CommandTimeout='30']
      UPDATE "Orders" AS o
      SET "Total" = (
          SELECT COALESCE(sum(o."Amount"::numeric * o."TotalCost"), 0.0)
          FROM "OrderProducts" AS o
          WHERE o."Id" = o."OrderId")
      WHERE o."Id" = @__id_0

EF Core version: 7.0.8
Database provider: Microsoft.EntityFrameworkCore.SqlServer (7.0.8)
Database provider: Npgsql.EntityFrameworkCore.PostgreSQL (7.0.4)
Target framework: NET 7.0
Operating system: Windows 11 22H2
IDE: Visual Studio 2022 17.4

If I change the name of the table, it works:

public DbSet<Order> AOrders => Set<Order>();
info: 23/06/2023 13.38.41.842 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (29ms) [Parameters=[@__id_0='a016b51c-fa31-4b71-b061-7e73057677a5'], CommandType='Text', CommandTimeout='30']
      UPDATE [a]
      SET [a].[Total] = (
          SELECT COALESCE(SUM(CAST([o].[Amount] AS decimal(18,2)) * [o].[TotalCost]), 0.0)
          FROM [OrderProducts] AS [o]
          WHERE [a].[Id] = [o].[OrderId])
      FROM [AOrders] AS [a]
      WHERE [a].[Id] = @__id_0

@roji
Copy link
Member

roji commented Jun 23, 2023

@KernelCrap referencing navigations within the ExecuteUpdate currently supported; see these docs which include the workaround.

@KernelCrap
Copy link

@roji The example is using the method described in your link.

@roji
Copy link
Member

roji commented Jun 23, 2023

@KernelCrap I'm seeing the following in your code sample:

        .ExecuteUpdateAsync(e => e
            .SetProperty(x => x.Order.Total, x => x.Total));

x.Order.Total is using a navigation.

@KernelCrap
Copy link

@roji Here is the example from the documentation:

context.Blogs
    .Select(b => new { Blog = b, NewRating = b.Posts.Average(p => p.Rating) })
    .ExecuteUpdate(setters => setters.SetProperty(b => b.Blog.Rating, b => b.NewRating));

It is supported when using Select to project it to an anonymous type first (which is what my example does).

@hexianggui
Copy link
Author

hexianggui commented Jun 24, 2023

@ajcvickers
Thank you for your response, and I apologize for taking so long to reply. I created a completely new project based on my project environment, and I obtained the same results as you did. The problem did not reproduce. It's possible that this is unrelated to EF Core, but it does indeed exist in my original project.
The address of the new project is efcoretest ,but the problem could not be reproduced!
2023-06-24_102915
Next, I will attempt to reproduce the issue in the new project, and I will inform you of the results. Thank you once again for your attention!

@KernelCrap
Copy link

KernelCrap commented Jun 24, 2023

@hexianggui In your new project, your tables are named BomItem and WorkOrder, but in your original comment the tables were MesWorkOrder and MesBomItem.

From my testing, it breaks when two tables use the same prefix. In my example, before my comment was hidden, I experienced the same thing with two tables named Order and OrderProduct, but it worked fine when renaming one of the tables to something that didn't start with Order.

@hexianggui
Copy link
Author

hexianggui commented Jun 25, 2023

@hexianggui In your new project, your tables are named BomItem and WorkOrder, but in your original comment the tables were MesWorkOrder and MesBomItem.

From my testing, it breaks when two tables use the same prefix. In my example, before my comment was hidden, I experienced the same thing with two tables named Order and OrderProduct, but it worked fine when renaming one of the tables to something that didn't start with Order.

@KernelCrap Yes, you are right. Thank you very much! I added a prefix and now I have successfully reproduced the previous issue! exampleProject
Your previous comment was probably hidden, and I didn't see it. Now I can see it!

2023-06-25_143339
@ajcvickers After running the project (before running, make sure to execute the database migration), you can execute the APIs in Swagger, and you will get the results in the console. Thank you, everyone!

Furthermore, to maintain consistency with the original project environment, I built the project using ABPvNext. I have not conducted any testing to determine if the same results can be achieved without using any additional frameworks.

@hexianggui
Copy link
Author

hexianggui commented Jun 25, 2023

I haven't read the source code, but based on the above observations, it appears that table aliases are derived from the initial letter of the table name. For example, the aliases for "BomItem" and "WorkOrder" are "b" and "w" respectively, while the aliases for "MesBomItem" and "MesWorkOrder" are both "m". If that's the case, having two or more table names with the same initial letter in the same SQL statement can lead to confusion.
However, from what I can see currently, this situation only occurs in subquery statements. When I perform a join query using LINQ, the generated SQL statement includes correct aliases. For example, when joining "MesBomItem" and "MesBom", the generated aliases are "m" and "m0" respectively.

@roji
Copy link
Member

roji commented Jun 25, 2023

@KernelCrap apologies, I read your code sample too quickly. I can see the error happening, we'll investigate this.

@roji roji changed the title I use ExecuteUpdateAsync to update data, but it seems that the generated SQL statement has a problem SQL aliases aren't uniquified in ExecuteUpdate setters Jun 25, 2023
@roji roji self-assigned this Jun 25, 2023
roji added a commit to roji/efcore that referenced this issue Jun 25, 2023
@roji roji added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. area-bulkcud and removed area-query labels Jun 25, 2023
@roji
Copy link
Member

roji commented Jun 26, 2023

Reopening to consider patching.

@roji roji reopened this Jun 26, 2023
@ajcvickers ajcvickers added this to the 7.0.x milestone Jul 8, 2023
@ajcvickers
Copy link
Member

@roji Note from triage: patch.

roji added a commit to roji/efcore that referenced this issue Jul 8, 2023
@roji
Copy link
Member

roji commented Jul 8, 2023

Patch PR: #31208

@anoordover
Copy link

Thanks for this fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-bulkcud closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Projects
None yet
5 participants