dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.63k stars 3.15k forks source link

Issue with 'optimization' of LINQ to SQL #30385

Closed awdorrin closed 1 year ago

awdorrin commented 1 year ago

File a bug

Include your code

Failing code

                var taskList = await _context.SubcontractRtgTasks
                    .AsNoTracking()
                    .Where(x => x.SubcontractRtgId == prev.Subcontract_RTG_ID)
                    .ToListAsync();

                foreach (var task in taskList)
                {
                    task.SubcontractRtgTaskId = 0;
                    task.SubcontractRtgId = subcRtgId;
                    _context.Entry(task).State = EntityState.Added;
                }
                await _context.SaveChangesAsync();

This results in the exception: Microsoft.Data.SqlClient.SqlException (0x80131904): Subquery returned more than 1 value. This is not permitted when the subquery follows =, !=, <, <= , >, >= or when the subquery is used as an expression.

The SQL being generated looks like this (which is one of the funkiest things I've seen in SQL...):

DECLARE @inserted0 TABLE ([Subcontract_RTG_Task_ID] int, [_Position] [int]);
MERGE [dbo].[Subcontract_RTG_Tasks] USING (
VALUES (@p0, @p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8, @p9, @p10, @p11, @p12, 0),
(@p13, @p14, @p15, @p16, @p17, @p18, @p19, @p20, @p21, @p22, @p23, @p24, @p25, 1),
(@p26, @p27, @p28, @p29, @p30, @p31, @p32, @p33, @p34, @p35, @p36, @p37, @p38, 2),
(@p39, @p40, @p41, @p42, @p43, @p44, @p45, @p46, @p47, @p48, @p49, @p50, @p51, 3),
(@p52, @p53, @p54, @p55, @p56, @p57, @p58, @p59, @p60, @p61, @p62, @p63, @p64, 4),
(@p65, @p66, @p67, @p68, @p69, @p70, @p71, @p72, @p73, @p74, @p75, @p76, @p77, 5),
(@p78, @p79, @p80, @p81, @p82, @p83, @p84, @p85, @p86, @p87, @p88, @p89, @p90, 6),
(@p91, @p92, @p93, @p94, @p95, @p96, @p97, @p98, @p99, @p100, @p101, @p102, @p103, 7),
(@p104, @p105, @p106, @p107, @p108, @p109, @p110, @p111, @p112, @p113, @p114, @p115, @p116, 8),
(@p117, @p118, @p119, @p120, @p121, @p122, @p123, @p124, @p125, @p126, @p127, @p128, @p129, 9),
(@p130, @p131, @p132, @p133, @p134, @p135, @p136, @p137, @p138, @p139, @p140, @p141, @p142, 10),
(@p143, @p144, @p145, @p146, @p147, @p148, @p149, @p150, @p151, @p152, @p153, @p154, @p155, 11),
(@p156, @p157, @p158, @p159, @p160, @p161, @p162, @p163, @p164, @p165, @p166, @p167, @p168, 12),
(@p169, @p170, @p171, @p172, @p173, @p174, @p175, @p176, @p177, @p178, @p179, @p180, @p181, 13),
(@p182, @p183, @p184, @p185, @p186, @p187, @p188, @p189, @p190, @p191, @p192, @p193, @p194, 14),
(@p195, @p196, @p197, @p198, @p199, @p200, @p201, @p202, @p203, @p204, @p205, @p206, @p207, 15),
(@p208, @p209, @p210, @p211, @p212, @p213, @p214, @p215, @p216, @p217, @p218, @p219, @p220, 16),
(@p221, @p222, @p223, @p224, @p225, @p226, @p227, @p228, @p229, @p230, @p231, @p232, @p233, 17),
(@p234, @p235, @p236, @p237, @p238, @p239, @p240, @p241, @p242, @p243, @p244, @p245, @p246, 18)) 
AS i
 ([Date_Added], [Date_Modified], [Modified_By], [Subcontract_RTG_CriticalFor_ID], [Subcontract_RTG_ID], [Task_Actual_Complete], [Task_Comments], [Task_Current_ECD], [Task_Description], [Task_Num], [Task_Original_ECD], [Task_Percent], [Task_RI], _Position)
 ON 1=0
WHEN NOT MATCHED THEN
INSERT ([Date_Added], [Date_Modified], [Modified_By], [Subcontract_RTG_CriticalFor_ID], [Subcontract_RTG_ID], 
    [Task_Actual_Complete], [Task_Comments], [Task_Current_ECD], [Task_Description], [Task_Num], [Task_Original_ECD], [Task_Percent], [Task_RI])
VALUES (i.[Date_Added], i.[Date_Modified], i.[Modified_By], i.[Subcontract_RTG_CriticalFor_ID], 
    i.[Subcontract_RTG_ID], i.[Task_Actual_Complete], i.[Task_Comments], i.[Task_Current_ECD], i.[Task_Description], i.[Task_Num], i.[Task_Original_ECD], i.[Task_Percent], i.[Task_RI])
OUTPUT INSERTED.[Subcontract_RTG_Task_ID], i._Position
INTO @inserted0;

SELECT [t].[Subcontract_RTG_Task_ID], [t].[Critical_For_Green] 
FROM [dbo].[Subcontract_RTG_Tasks] t
INNER JOIN @inserted0 i ON ([t].[Subcontract_RTG_Task_ID] = [i].[Subcontract_RTG_Task_ID])
ORDER BY [i].[_Position];

It would seem that some sort of optimization is taking place, once a certain number of items are being updated. I changed the code to save to the database inside the foreach loop

                foreach (var task in taskList)
                {
                    task.SubcontractRtgTaskId = 0;
                    task.SubcontractRtgId = subcRtgId;
                    _context.Entry(task).State = EntityState.Added;
                    await _context.SaveChangesAsync();
                }

And individual inserts are created, which work without exception, but I still do not understand why there is this SELECT query in the output.

INSERT INTO [dbo].[Subcontract_RTG_Tasks] ([Date_Added], [Date_Modified], [Modified_By], [Subcontract_RTG_CriticalFor_ID], [Subcontract_RTG_ID], [Task_Actual_Complete], [Task_Comments], [Task_Current_ECD], [Task_Description], [Task_Num], [Task_Original_ECD], [Task_Percent], [Task_RI])
VALUES (@p0, @p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8, @p9, @p10, @p11, @p12);
SELECT [Subcontract_RTG_Task_ID], [Critical_For_Green]
FROM [dbo].[Subcontract_RTG_Tasks]
WHERE @@ROWCOUNT = 1 AND [Subcontract_RTG_Task_ID] = scope_identity();

For reference, the SuibcontractRtgTask model is:

 public partial class SubcontractRtgTask
    {
        [Key]
        [Column("Subcontract_RTG_Task_ID")]
        public int SubcontractRtgTaskId { get; set; }

        [Column("Subcontract_RTG_ID")]
        public int SubcontractRtgId { get; set; }

        [Column("Task_Num")]
        public int TaskNum { get; set; }

        [Required]
        [Column("Task_Description")]
        [StringLength(2000)]
        [Unicode(false)]
        public string TaskDescription { get; set; }

        [Column("Task_Percent")]
        public int? TaskPercent { get; set; }

        [Column("Task_RI")]
        [StringLength(256)]
        [Unicode(false)]
        public string TaskRi { get; set; }

        [Column("Task_Original_ECD", TypeName = "datetime")]
        public DateTime? TaskOriginalEcd { get; set; }

        [Column("Task_Current_ECD", TypeName = "datetime")]
        public DateTime? TaskCurrentEcd { get; set; }

        [Column("Task_Actual_Complete", TypeName = "datetime")]
        public DateTime? TaskActualComplete { get; set; }

        [Column("Subcontract_RTG_CriticalFor_ID")]
        public int? SubcontractRtgCriticalForId { get; set; }

        [Required]
        [Column("Critical_For_Green")]
        [StringLength(1)]
        [Unicode(false)]
        public string CriticalForGreen { get; set; }

        [Column("Task_Comments")]
        [StringLength(4000)]
        [Unicode(false)]
        public string TaskComments { get; set; }

        [Column("Date_Added", TypeName = "datetime")]
        public DateTime DateAdded { get; set; }

        [Column("Date_Modified", TypeName = "datetime")]
        public DateTime DateModified { get; set; }

        [Column("Modified_By")]
        [StringLength(50)]
        [Unicode(false)]
        public string ModifiedBy { get; set; }

        [ForeignKey("SubcontractRtgId")]
        [InverseProperty("SubcontractRtgTasks")]
        public virtual SubcontractRtg SubcontractRtg { get; set; }

        [ForeignKey("SubcontractRtgCriticalForId")]
        [InverseProperty("SubcontractRtgTasks")]
        public virtual SubcontractRtgCriticalFor SubcontractRtgCriticalFor { get; set; }
    }

Include provider and version information

EF Core version: 6.0.10 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET 6.0 Operating system: Windows/Linux IDE: Visual Studio 2022 17.4

ajcvickers commented 1 year ago

@awdorrin Can you try this with EF Core 7.0.3? The SQL we generate for SaveChanges has significantly changed in EF7.

awdorrin commented 1 year ago

We don't have the ability to deploy apps to .Net 7 in our environment yet, but I may be able to see if I can create a local branch to test. I did just find out from a coworker that the Critical_For_Green column on that table is not a simple 1 character string, but a computed column:

 (case when [Subcontract_RTG_CriticalFor_ID] IS NULL then 'N' else 'Y' end)

So that probably explains the unexpected SELECT and perhaps why the optimized merge approach with the original code was failing?

ajcvickers commented 1 year ago

@awdorrin EF Core 7 does not require .NET 7. It can be used on .NET 6.

awdorrin commented 1 year ago

Oh wow, I didn't realize that! Ok, I'll see what I can do.

awdorrin commented 1 year ago

I had a chance to update to EF Core 7, but due to this breaking change: SQL Server tables with triggers or certain computed columns now require special EF Core configuration the new approach won't work for us.

roji commented 1 year ago

@awdorrin are you saying you can't upgrade to EF Core 7 because of that change? There's a code fragment there which you can drop in, and which makes EF Core 7 behave just like 6 again, in case many of your tables have triggers etc.

awdorrin commented 1 year ago

My understanding from reading about the change, is that I can flag the table as having a trigger and it will use the original ef core 6 logic rather than the new approach, which sort of defeats the purpose of testing to see if the ef core 7 changes work 😀

If I'm wrong, and there is a work around, let me know.

roji commented 1 year ago

Sorry, I was missing context here. So are you saying that the table in question (Subcontract_RTG_Tasks I think) has a trigger, and therefore you can't try the new EF 7.0 update SQL against it?

awdorrin commented 1 year ago

Correct. It has both a trigger and a computed column, therefore I believe that the new EF 7 code would just revert back to EF 6 methods.

ajcvickers commented 1 year ago

@awdorrin 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.

#nullable disable

using (var context = new SomeDbContext())
{
    context.Database.EnsureDeleted();
    context.Database.EnsureCreated();

    context.AddRange(
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask(),
        CreateSubcontractRtgTask());

    context.SaveChanges();
}

using (var context = new SomeDbContext())
{
    var subcontractRtgId = 6;
    var taskList = await context.SubcontractRtgTasks
        .AsNoTracking()
        .Where(x => x.SubcontractRtgId == subcontractRtgId)
        .ToListAsync();

    foreach (var task in taskList)
    {
        task.SubcontractRtgTaskId = 0;
        task.SubcontractRtgId = 7;
        context.Entry(task).State = EntityState.Added;
    }

    await context.SaveChangesAsync();
}

SubcontractRtgTask CreateSubcontractRtgTask()
{
    return new SubcontractRtgTask
    {
        SubcontractRtgId = 6,
        TaskDescription = "X",
        CriticalForGreen = "Y",
        DateAdded = DateTime.UtcNow,
        DateModified = DateTime.UtcNow
    };
}

public partial class SubcontractRtgTask
{
    [Key]
    [Column("Subcontract_RTG_Task_ID")]
    public int SubcontractRtgTaskId { get; set; }

    [Column("Subcontract_RTG_ID")]
    public int SubcontractRtgId { get; set; }

    [Column("Task_Num")]
    public int TaskNum { get; set; }

    [Required]
    [Column("Task_Description")]
    [StringLength(2000)]
    [Unicode(false)]
    public string TaskDescription { get; set; }

    [Column("Task_Percent")]
    public int? TaskPercent { get; set; }

    [Column("Task_RI")]
    [StringLength(256)]
    [Unicode(false)]
    public string TaskRi { get; set; }

    [Column("Task_Original_ECD", TypeName = "datetime")]
    public DateTime? TaskOriginalEcd { get; set; }

    [Column("Task_Current_ECD", TypeName = "datetime")]
    public DateTime? TaskCurrentEcd { get; set; }

    [Column("Task_Actual_Complete", TypeName = "datetime")]
    public DateTime? TaskActualComplete { get; set; }

    [Column("Subcontract_RTG_CriticalFor_ID")]
    public int? SubcontractRtgCriticalForId { get; set; }

    [Required]
    [Column("Critical_For_Green")]
    [StringLength(1)]
    [Unicode(false)]
    public string CriticalForGreen { get; set; }

    [Column("Task_Comments")]
    [StringLength(4000)]
    [Unicode(false)]
    public string TaskComments { get; set; }

    [Column("Date_Added", TypeName = "datetime")]
    public DateTime DateAdded { get; set; }

    [Column("Date_Modified", TypeName = "datetime")]
    public DateTime DateModified { get; set; }

    [Column("Modified_By")]
    [StringLength(50)]
    [Unicode(false)]
    public string ModifiedBy { get; set; }
}

public class SomeDbContext : DbContext
{
    public DbSet<SubcontractRtgTask> SubcontractRtgTasks => Set<SubcontractRtgTask>();

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder
            .UseSqlServer(@"Data Source=(LocalDb)\MSSQLLocalDB;Database=AllTogetherNow")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();
    }
}
awdorrin commented 1 year ago

I will see if I can recreate with a sample program, I just have to find enough time and may not be able to until the end of the week. I did learn that in addition to the computed column on Critical_For_Green that I described above, there is also a trigger configured to update dates in another table, based on values found in this table.

ajcvickers commented 1 year ago

EF Team Triage: Closing this issue as the requested additional details have not been provided and we have been unable to reproduce it.

BTW this is a canned response and may have info or details that do not directly apply to this particular issue. While we'd like to spend the time to uniquely address every incoming issue, we get a lot traffic on the EF projects and that is not practical. To ensure we maximize the time we have to work on fixing bugs, implementing new features, etc. we use canned responses for common triage decisions.