denis-ivanov / EntityFrameworkCore.ClickHouse

ClickHouse provider for Entity Framework Core.
https://clickhouse.tech/
19 stars 4 forks source link

EF SaveChangesAsync with updates throws DbUpdateConcurrencyException every time #53

Open flosca opened 2 months ago

flosca commented 2 months ago

Hello!

I've encountered an issue that every time when I update entity using this provider, SaveChangesAsync throws the following exception:

Unhandled exception. Microsoft.EntityFrameworkCore.DbUpdateConcurrencyException: 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(RelationalDataReader reader, Int32 commandIndex, Int32 expectedRowsAffected, Int32 rowsAffected, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ConsumeResultSetWithRowsAffectedOnlyAsync(Int32 commandIndex, RelationalDataReader reader, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ConsumeAsync(RelationalDataReader reader, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, 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.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)

Below there is a snippet to reproduce this behaviour:

Installed packages: EntityFrameworkCore.ClickHouse 0.0.20 Microsoft.EntityFrameworkCore 8.0.7 Spectre.Console.Cli 0.49.1

using System.Globalization;
using ClickHouse.EntityFrameworkCore.Extensions;
using Microsoft.EntityFrameworkCore;
using Spectre.Console;

class MyFirstTable
{
    public uint UserId { get; set; }

    public string Message { get; set; }

    public DateTime Timestamp { get; set; }

    public float Metric { get; set; }
}

class QuickStartDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        base.OnConfiguring(optionsBuilder);

        optionsBuilder.UseClickHouse("Host=localhost;Protocol=http;Port=8123;Database=QuickStart");
    }

    public DbSet<MyFirstTable> MyFirstTable { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        var entityTypeBuilder = modelBuilder.Entity<MyFirstTable>();

        entityTypeBuilder.Property(e => e.UserId).HasColumnName("user_id");
        entityTypeBuilder.Property(e => e.Message).HasColumnName("message");
        entityTypeBuilder.Property(e => e.Timestamp).HasColumnName("timestamp");
        entityTypeBuilder.Property(e => e.Metric).HasColumnName("metric");

        entityTypeBuilder.HasKey(e => new { e.UserId, e.Timestamp });

        entityTypeBuilder.ToTable("my_first_table", table => table
            .HasMergeTreeEngine()
            .WithPrimaryKey("user_id", "timestamp"));
    }
}

class Program
{
    static async Task Main(string[] args)
    {
        await using var context = new QuickStartDbContext();
        await context.Database.EnsureCreatedAsync();

        var myFirstTable = new MyFirstTable
        {
            UserId = 101,
            Message = "Hello, ClickHouse!",
            Timestamp = DateTime.Now,
            Metric = -1f
        };

        await context.MyFirstTable.AddAsync(
            myFirstTable);

        await context.SaveChangesAsync();

        var dataToUpdate = await context.MyFirstTable.FirstAsync();

        dataToUpdate.Message = "Updated message";
        context.Update(dataToUpdate);
        try
        {
            await context.SaveChangesAsync();
        }
        catch (DbUpdateConcurrencyException ex)
        {
            AnsiConsole.MarkupLine("[red]Concurrency exception occurred[/]");
            AnsiConsole.WriteLine(ex.Message);
        }

        var table = new Table()
            .AddColumns(
                new TableColumn("user_id").RightAligned(),
                new TableColumn("message").LeftAligned(),
                new TableColumn("timestamp").RightAligned(),
                new TableColumn("metric").RightAligned());

        table.AddRow(
            dataToUpdate.UserId.ToString(),
            dataToUpdate.Message,
            dataToUpdate.Timestamp.ToString(CultureInfo.InvariantCulture),
            dataToUpdate.Metric.ToString(CultureInfo.InvariantCulture));

        AnsiConsole.Write(table);
    }
}

The output of the program is:

Concurrency exception occurred
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.
┌─────────┬─────────────────┬─────────────────────┬────────┐
│ user_id │ message         │           timestamp │ metric │
├─────────┼─────────────────┼─────────────────────┼────────┤
│     101 │ Updated message │ 08/08/2024 15:07:39 │     -1 │
└─────────┴─────────────────┴─────────────────────┴────────┘

It's interesting though that the data in ClickHouse did updated. Looks like it's the provider issue: it cannot fetch the actual affected rows while calling SaveChanges. Am I missing anything or is it an actual bug?

Thanks in advance.

flosca commented 2 months ago

I've cloned your library and tried to patch it with one of the overriding of AppendUpdateOperation method in ClickHouseUpdateSqlGenerator.

I just copy-pasted this method from a different provider https://github.com/npgsql/efcore.pg/blob/e6d545ace2294a14140744cd32c33adf68e4e3c7/src/EFCore.PG/Update/Internal/NpgsqlUpdateSqlGenerator.cs#L99 and magically the exception disappeared.

However I don't have a full expertise in EF to propose a pull-request. Maybe that information would be useful for you.