efcore icon indicating copy to clipboard operation
efcore copied to clipboard

EFCore Delete And Add will merge to Update, and throw NullReferenceException

Open augerpi opened this issue 1 year ago • 5 comments

My entity are bind to StoredProcedure for the CUD operations.

When I Delete an entity and then add a new entity with the same data, EF core will merge the Delete and Insert into one Update command. The problem is when building the update command the GetStoredProcedureMapping return null because the property StoreStoredProcedure is still containing the Insert stored procedure while the entity state suggest an modified state image We can see here that the the GetStoredProcedureMapping will never enter in the if as the StoreStoredProcedure is not the same as the StoredProcedure return by the entityState image

Stack traces

System.NullReferenceException
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.GenerateColumnModifications()
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.<>c.<get_ColumnModifications>b__33_0(ModificationCommand command)
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.get_ColumnModifications()
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.CreateCommandBatches(IEnumerable`1 commandSet, Boolean moreCommandSets, Boolean assertColumnModification, ParameterNameGenerator parameterNameGenerator)+MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.BatchCommands(IList`1 entries, IUpdateAdapter updateAdapter)+MoveNext()
   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.SqlServer.Storage.Internal.SqlServerExecutionStrategy.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()

Include provider and version information

EF Core version: 8.0.4 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET 8.0 Operating system: Windows 11 IDE: Visual Studio 2022 17.9.6

augerpi avatar Jul 23 '24 14:07 augerpi

Duplicate of #30705

ajcvickers avatar Jul 23 '24 14:07 ajcvickers

@AndriySvyryd do you think the fix here is simply #30705, or there's something sproc-specific to be done outside of that?

roji avatar Aug 09 '24 07:08 roji

There will be cases that still need to be merged into a single Update even after https://github.com/dotnet/efcore/issues/30705. So, we need to investigate whether this will continue to be an issue.

AndriySvyryd avatar Aug 09 '24 16:08 AndriySvyryd

There will be cases that still need to be merged into a single Update even after https://github.com/dotnet/efcore/issues/30705.

What do you have in mind for these, wouldn't we systematically stop merging?

In any case, am putting in the backlog as a bug for now.

roji avatar Oct 11 '24 15:10 roji

What do you have in mind for these, wouldn't we systematically stop merging?

Mainly this would be for table-splitting dependents. But, in general, this would be a relatively significant change, so we might need to add a way to configure this behavior.

AndriySvyryd avatar Oct 11 '24 18:10 AndriySvyryd

We're likely encountering the same issue. In our case, we have a navigation property that holds a list of inherited types, mapped using table-per-hierarchy (TPH). One of the derived types includes an owned JSON type (LocaleValueAttribute). When we try to remove an instance of that type and add a different derived type (StringAttribute), we run into a NullReferenceException.

System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.<GenerateColumnModifications>g__FindJsonPartialUpdateInfo|41_2(IUpdateEntry entry, List`1 processedEntries)
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.<GenerateColumnModifications>g__HandleJson|41_4(List`1 columnModifications, <>c__DisplayClass41_0&)
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.GenerateColumnModifications()
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.<>c.<get_ColumnModifications>b__33_0(ModificationCommand command)
   at Microsoft.EntityFrameworkCore.Internal.NonCapturingLazyInitializer.EnsureInitialized[TParam,TValue](TValue& target, TParam param, Func`2 valueFactory)
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.get_ColumnModifications()
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.CreateCommandBatches(IEnumerable`1 commandSet, Boolean moreCommandSets, Boolean assertColumnModification, ParameterNameGenerator parameterNameGenerator)+MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.BatchCommands(IList`1 entries, IUpdateAdapter updateAdapter)+MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabase.SaveChangesAsync(IList`1 entries, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList`1 entriesToSave, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(StateManager stateManager, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Program.<Main>$(String[] args) in C:\Work\Experimental\ConsoleApp2\ConsoleApp2\Program.cs:line 40
   at Program.<Main>$(String[] args) in C:\Work\Experimental\ConsoleApp2\ConsoleApp2\Program.cs:line 40
   at Program.<Main>(String[] args)

Minimal reproduction

using Microsoft.EntityFrameworkCore;

await using var context = new ReproDbContext();

await context.Database.MigrateAsync();

var item = context.Items.Add(new Item
{
    Name = "Product 1",
    Attributes = [
        new LocaleValueAttribute()
        {
            Key = "TextValue",
            Value = new LocaleValue()
            {
                Entries = [
                    new LocaleValueEntry()
                    {
                        Locale = "en-US",
                        Value = "Hello"
                    }
                ]
            }
        }
    ]
});

await context.SaveChangesAsync();

item.Entity.Attributes.RemoveAll(attr => attr.Key == "TextValue");

item.Entity.Attributes.Add(new StringAttribute
{
    Key = "TextValue",
    Value = "World"
});

await context.SaveChangesAsync(); // Throws the NullReferenceException

// Db context
public class ReproDbContext : DbContext
{
    
    public DbSet<Item> Items { get; set; }
    
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer("Server=(localdb)\\mssqllocaldb;Database=efcore-bug-repro;Trusted_Connection=True;MultipleActiveResultSets=true;");
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Item>()
            .HasKey(x => x.Id);

        modelBuilder.Entity<ItemAttribute>(b =>
        {
            b.HasKey(x => new { x.ItemId, x.Key });
            b.HasDiscriminator<string>("Discriminator")
                .HasValue<StringAttribute>("string")
                .HasValue<LocaleValueAttribute>("locale-value");
        });
        
        modelBuilder.Entity<StringAttribute>(b =>
        {
            b.Property(x => x.Value).HasColumnName("StringValue");
        });
        
        modelBuilder.Entity<LocaleValueAttribute>(b =>
        {
            b.OwnsOne(x => x.Value, bc =>
            {
                bc.ToJson("LocaleValue");
                bc.OwnsMany(x => x.Entries, v =>
                {
                    v.Property(x => x.Locale).IsRequired();
                    v.Property(x => x.Value);
                });
            });
        });
    }
}

// Models
public class Item
{
    public int Id { get; set; }
    public required string Name { get; set; }
    public List<ItemAttribute> Attributes { get; set; } = [];
}

public abstract class ItemAttribute
{
    public int ItemId { get; set; }
    public required string Key { get; set; }
}

public class StringAttribute : ItemAttribute
{
    public string Value { get; set; } = null!;
}

public class LocaleValueAttribute : ItemAttribute
{
    public LocaleValue Value { get; set; } = null!;
}

public class LocaleValue
{
    public List<LocaleValueEntry> Entries { get; set; } = [];
}

public class LocaleValueEntry
{
    public required string Locale { get; set; }
    public required string Value { get; set; }
}

@roji @AndriySvyryd Can you let me know if this is related to this issue? Otherwise, I will create a different ticket.

MaikelOrisha avatar Apr 30 '25 14:04 MaikelOrisha