Concurrency exception when trying to insert entity with datetime2(0) key

[clement911]

Here is bug in EF6 Core.

Create an entity as follows:

public class QueryRun
    {
        [Column(TypeName = "datetime2(0)")]
        public DateTime StartedAt { get; set; }

        [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
        [NonDefault]
        public int Id { get; private set; }

        //other properties omitted for brevity
     }

Then try insert an entity

ctx.Add(new QueryRun
                {
                    StartedAt = DateTime.UtcNow,
                    //other properties omitted for brevity
                });
 await ctx.SaveChangesAsync();

This fails with the message: Expected 1 affected rows but got 0 rows!
After scratching my head for some time, I found the bug in the generated SQL code.

We can see that EF generated a param @p0 datetime2(7) instead of datetime2(0).
The issue is that the predicate [StartedAt] = @p0 returns FALSE because the parameter has milliseconds whereas the column is datetime2(0) and has no milliseconds.

Expected behaviour: when generating the p0 parameter, EF should use the same type as the column. That is, it should use datetime2(0) instead of datetime2(7). If I change the generated code to use datetime2(0), I can see that the generated ID is returned.

[roji]

@clement911 can you confirm that you've confirmed StartAt as a concurrency token? Assuming so, what technique are you using to update it? In any case, a fully runnable code sample (including the model) is always important for us to investigate, rather than isolated snippets.

[clement911]

@roji on, StartsAt is not a concurrency token.

I've created a fully runnable repro for you here: https://github.com/clement911/EFBug

[roji]

@clement911 thanks for the repro, makes sense - the missing piece above was that StartedAt was part of the key. Seems like when sending parameters in the update pipeline, we don't fully configure the parameter based on the column's type mapping.

Minimal repro

using var ctx = new MyDbContext();

ctx.Database.EnsureDeleted();
ctx.Database.EnsureCreated();

ctx.Add(new MyEntity() { StartedAt = new DateTime(2000, 1, 1, 1, 1, 1, millisecond: 0) });
await ctx.SaveChangesAsync();//WORKS

ctx.Add(new MyEntity() { StartedAt = new DateTime(2000, 1, 1, 1, 1, 1, millisecond: 1) });
await ctx.SaveChangesAsync();//FAILS: The database operation was expected to affect 1 row(s), but actually affected 0 row(s);

public class MyEntity
{
    [Column(TypeName = "datetime2(0)")]
    public DateTime StartedAt { get; set; }

    [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public int Id { get; private set; }
}

public class MyDbContext : DbContext
{
    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);
        modelBuilder.Entity<MyEntity>().HasKey(i => new { i.StartedAt, i.Id });
    }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        base.OnConfiguring(optionsBuilder);
        optionsBuilder.UseSqlServer("Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0;Trust Server Certificate=true");
    }
}

[roji]

Duplicate of dotnet/SqlClient#1380

[roji]

This is because of a SqlClient issue specifically with scale 0 on datetime2, see dotnet/SqlClient#1380. This issue is specifically called out in their docs:

For the DateTime2 type, scale 0 (the default) will be passed as datetime2(7). There is currently no way to send a parameter as datetime2(0). Scales 1-7 work as expected. This problem applies to DateTimeOffset and Time as well.

So while EF Core properly sets the SqlParameter's scale to 0, that gets ignored by SqlClient, which sends the full, untruncated value. Since that doesn't match the database column value (which is truncated), a concurrency exception occurs.

[roji]

In case it's useful for the future, here's an EF Core regression test for this (to be placed in SqlServerValueGenerationScenariosTest):

Regression test

[ConditionalFact] // #27513
public void Insert_with_concurrent_token_with_non_default_store_type()
{
    using var testStore = SqlServerTestStore.CreateInitialized(DatabaseName);
    using (var context = new BlogContextWithConcurrencyTokenWithNonDefaultStoreType(testStore.Name))
    {
        context.Database.EnsureCreatedResiliently();

        var blog = new Blog
        {
            CreatedOn = new DateTime(2000, 1, 1, 1, 1, 1, millisecond: 1),
            Name = "original"
        };
        context.Blogs.Add(blog);
        context.SaveChanges();

        blog.Name = "modified";
        context.SaveChanges();
    }
}

public class BlogContextWithConcurrencyTokenWithNonDefaultStoreType : ContextBase
{
    public BlogContextWithConcurrencyTokenWithNonDefaultStoreType(string databaseName)
        : base(databaseName)
    {
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);

        modelBuilder
            .Entity<Blog>()
            .Property(e => e.CreatedOn)
            .HasColumnType("datetime2(0)")
            .IsConcurrencyToken();
    }
}

[clement911]

@roji , understood about the SqlClient bug.
However in my case there isn't even a concurrency token column but there is an key identity column, which causes EF to query the DB to fetch the generated ID.
So, this issue is not constrained to scenarios with a concurrency token.
Any time you have a composite key with a datetime2(0) column and a identify column, the issue will occur upon insertion.

[clement911]

FYI, our work around for now is to truncate the datetime value in the client c# code. That is, we remove the milliseconds part BEFORE calling savechanges.

[clement911]

I suppose EF could apply that work around internally.
I.e. if the column is of type datetime2(0), it could set the milliseconds part to 0 before saving. I'm not sure if that could cause other problems though...

[roji]

However in my case there isn't even a concurrency token column but there is an key identity column, which causes EF to query the DB to fetch the generated ID.

Yes, that's right. The way inserts are currently implemented when there's an IDENTITY column, EF first sends the INSERT, and then a SELECT to get back generated column values for the newly-inserted row (i.e. the identity value); this is why the SqlClient bug manifests even when there's no concurrency token. Note that #27573 changes how inserts work for 7.0, so that this should no longer affect inserts (except when triggers are defined on the table), but the bug would still manifest if there's a concurrency token.

FYI, our work around for now is to truncate the datetime value in the client c# code. That is, we remove the milliseconds part BEFORE calling savechanges.

Yes, that's the workaround for now. I'd even recommend implementing the truncation in the .NET property setter, e.g.:

        [Column(TypeName = "datetime2(0)")]
        public DateTime StartedAt
        {
            get => _startedAt;
            set => /* truncate and set */ 
        }

        private DateTime _startedAt;

I suppose EF could apply that work around internally.

We generally try to avoid compensating for bugs in lower levels, though we'll discuss this specific case in triage.

[ajcvickers]

Note from triage: closing as SqlClient external issue.

[roji]

Reopening as the bug is not going to be fixed on the SqlClient side (dotnet/SqlClient#1380 (comment)). If we implement client-side parameter transformations (#28028), we can perform truncations in EF to work around this.

[ajcvickers]

Note from triage: we won't be doing anything specifically in EF for this. Workarounds are to manually truncate the type before calling SaveChanges, to truncate in overridden SaveChanges method or in SavingChanges event, or use an interceptor.

[clement911]

Looks like SqlClient now has a fix in the latest preview release:
dotnet/SqlClient#2411

I hope future EF can leverage it to fix this bug.

[ajcvickers]

Re-opening since SqlClient have implemented a fix. We should check it works well with EF.