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.8k stars 3.2k forks source link

Generates Incorrect UPDATE Instead of INSERT SQL for New Entity in One-to-Many Relationship #35090

Closed shyamal890 closed 3 days ago

shyamal890 commented 1 week ago

Include your code

public class View
{
    public Guid view_id { get; set; }

    [MaxLength(50)]
    public string name { get; set; } = null!;

    public DateTime created_at { get; set; } = DateTime.UtcNow;
    public DateTime modified_at { get; set; } = DateTime.UtcNow;

    public virtual ICollection<ViewColumn> Columns { get; set; } = new List<ViewColumn>();
}
public class ViewColumn
{
    public Guid column_id { get; set; }
    public Guid view_id { get; set; }
    public string type { get; set; } = ViewColumnTypeEnum.name;

    public virtual View View { get; set; } = null!;
}

//This is how the relationship is established in DBContext
modelBuilder.Entity<ViewColumn>(entity =>
{
    entity.HasKey(e => e.column_id);

    entity.ToTable("ViewColumn");

    entity.HasOne(d => d.View)
       .WithMany(p => p.Columns)
       .HasForeignKey(d => d.view_id);
});

When I run the following code
var rel_view = db.Views
    .Include(x=> x.Columns)
    .FirstOrDefault(x => x.view_id = view_id);

var new_column= new ViewColumn()
{
    column_id = Guid.NewGuid(),
    type = ViewColumnTypeEnum.name,
};
rel_view.Columns.Add(new_column);
await db.SaveChangesAsync();

EF Core generates the following SQL statement:
UPDATE [ViewColumn] SET [type] = @p1, [view_id] = @p2
      OUTPUT 1
      WHERE [column_id] = @p6;

Where as it should have created an INSERT SQL statement

To solve this I have to specifically add db.Entry(db_sprint_stage_column).State = EntityState.Added;

Include stack traces

Error message:

The database operation was expected to affect 1 row(s), but actually affected 0 row(s); data may have been modified or deleted since entities were loaded. See https://go.microsoft.com/fwlink/?LinkId=527962 for information on understanding and handling optimistic concurrency exceptions.
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.<ThrowAggregateUpdateConcurrencyExceptionAsync>d__10.MoveNext()
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.<ConsumeResultSetWithRowsAffectedOnlyAsync>d__6.MoveNext()
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.<ConsumeAsync>d__2.MoveNext()
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.<ExecuteAsync>d__50.MoveNext()
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.<ExecuteAsync>d__50.MoveNext()
   at Microsoft.EntityFrameworkCore.SqlServer.Update.Internal.SqlServerModificationCommandBatch.<ExecuteAsync>d__15.MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.<ExecuteAsync>d__9.MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.<ExecuteAsync>d__9.MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.<ExecuteAsync>d__9.MoveNext()
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<SaveChangesAsync>d__111.MoveNext()
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<SaveChangesAsync>d__115.MoveNext()
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.<ExecuteAsync>d__7`2.MoveNext()
   at Microsoft.EntityFrameworkCore.DbContext.<SaveChangesAsync>d__63.MoveNext()
   at SmartTask.Data.Services.SomoneService.<CreateRelevantViews>d__3.MoveNext() in ....

Include verbose output

NA

Include provider and version information

EF Core version: 9.0 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET 9.0 (Also faced the same issue in 8.0) Operating system: Windows 10 IDE: Visual Studio 2022 17.12

roji commented 1 week ago

You probably need to read this section of the docs, which contains the following:

As mentioned above, integer and GUID key properties are configured to use automatically generated key values by default. This means that the application must not set any key value explicitly.

If you remove column_id = Guid.NewGuid() from your ViewColumn initialization, everything should work (EF by default already generates a Guid for you).

/cc @ajcvickers

shyamal890 commented 1 week ago

Thanks @roji for pointing me to the reference doc.

Have one doubt though. Incase of PK being generated in the frontend and then passed on to the backend, would we have to explicitly mark the entity as EntityState.Added?

Am asking this because incase of direct insert into db.ViewColumns.Add we don't get this error. So there seems to be a discrepancy between direct insert in to the table and insert through reference.

ajcvickers commented 1 week ago

@shyamal890 In general, if you are tracking disconnected entities for which key value generation is not being used, then you will need to explicitly set the state to Added or Modified as appropriate, since EF won't know for sure. Add will assume Added and Update will assume Modified. But the important thing to remember is to switch off value generation if you're not going to use it so that EF won't infer Update in an Add because the key value is set.

kevcoffey commented 1 week ago

@ajcvickers There seems to be a behaviour change between version 8.0.11 and 9.0.0 in this regard? We have issues with 9.0.0 that we didn't have with 8.0.11.

ajcvickers commented 1 week ago

@kevcoffey Please attach a small, runnable project or post a small, runnable code listing that works on 8.0.11 and fails on 9.0.0 so that we can investigate.

kevcoffey commented 6 days ago
using Microsoft.EntityFrameworkCore;

var optionsBuilder = new DbContextOptionsBuilder<DatabaseContext>()
    .UseSqlServer(
        "...",
        options => options
            .EnableRetryOnFailure()
            .UseQuerySplittingBehavior(QuerySplittingBehavior.SplitQuery));

var databaseContext = new DatabaseContext(optionsBuilder.Options);

var parentId = new Guid("8784dc85-e749-4c66-baae-694f2225c478");
var someEntityId = new Guid("0434e3dc-9202-443a-bedc-3025294fa6c1");

var parent = databaseContext
    .Parents
    .Where(_ => _.Id == parentId)
    .Include(_ => _.Child)
    .SingleOrDefault() ?? throw new Exception("...");

var someEntity = databaseContext
    .SomeEntities
    .Where(_ => _.Id == someEntityId)
    .SingleOrDefault() ?? throw new Exception("...");

parent.Child = new Child1
{
    SomeEntity = someEntity,
};

databaseContext.SaveChanges();

public abstract class Base
{
    public Guid Id { get; set; } = Guid.NewGuid();
} 

public class Parent : Base
{
    public Child? Child { get; set; }
}

public abstract class Child : Base
{
    public SomeEntity? SomeEntity { get; set; }
}

public class Child1 : Child { }

public abstract class SomeEntity : Base { }

public abstract class SomeEntity1 : SomeEntity { }

public class SomeEntity2 : SomeEntity1 { }

public class SomeEntity3 : SomeEntity2 { }

public class DatabaseContext(DbContextOptions<DatabaseContext> options) : DbContext(options)
{
    public DbSet<Child> Children { get; set; }

    public DbSet<Parent> Parents { get; set; }

    public DbSet<SomeEntity> SomeEntities { get; set; }

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

        modelBuilder.Entity<SomeEntity1>();
        modelBuilder.Entity<SomeEntity2>();
        modelBuilder.Entity<SomeEntity3>();
    }
}

@ajcvickers, sorry it took me a while to boil it down to an example. This code works with 8.0.11 but not with 9.0.0, 9.0.0 gives:

Microsoft.EntityFrameworkCore.DbUpdateConcurrencyException
  HResult=0x80131500
  Message=The database operation was expected to affect 1 row(s), but actually affected 0 row(s); data may have been modified or deleted since entities were loaded. See https://go.microsoft.com/fwlink/?LinkId=527962 for information on understanding and handling optimistic concurrency exceptions.
  Source=Microsoft.EntityFrameworkCore.Relational
  StackTrace:
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ThrowAggregateUpdateConcurrencyException(RelationalDataReader reader, Int32 commandIndex, Int32 expectedRowsAffected, Int32 rowsAffected)
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ConsumeResultSetWithRowsAffectedOnly(Int32 commandIndex, RelationalDataReader reader)
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.Consume(RelationalDataReader reader)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.SqlServer.Update.Internal.SqlServerModificationCommandBatch.Execute(IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabase.SaveChanges(IList`1 entries)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IList`1 entriesToSave)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(StateManager stateManager, Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<>c.<SaveChanges>b__112_0(DbContext _, ValueTuple`2 t)
   at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.<>c__DisplayClass28_0`2.<Execute>b__0(DbContext context, TState state)
   at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.ExecuteImplementation[TState,TResult](Func`3 operation, Func`3 verifySucceeded, TState state)
   at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges()
   at Program.<Main>$(String[] args) in E:\kcoffey\EntityFramework8_0_11_v9_0_0\EntityFramework8_0_11_v9_0_0\Program.cs:line 33
ajcvickers commented 3 days ago

I am able to reproduce this change between EF8 and EF9. However, the new behavior is correct, indicating that there was a bug in EF8 that got fixed as part of some other work.

@shyamal890 As discussed above, this code:

parent.Child = new Child1
{
    SomeEntity = someEntity,
};

Sets a new Child object with the Id property already set. Because Id has value generation turned on, the fact that the Id exists means that it must have already been generated, which means that this entity already exists in the database. If you're not making use of generated key values, then configure them appropriately. For example:

modelBuilder.Entity<Child1>().Property(e => e.Id).ValueGeneratedNever();
modelBuilder.Entity<Parent>().Property(e => e.Id).ValueGeneratedNever();

Alternately, if you must have a mix of generated values and pre-set valuyes, then you will need to be explicit about the state of such entities. For example, in this case, the following code will ensure an insert: by calling DbContext.Add directly on the child.

var child = new Child1
{
    SomeEntity = someEntity,
};

databaseContext.Add(child);
parent.Child = child;