MySQL Bugs: #81477: Concurrency control for c# connector and EntityFramework does not work

Bug #81477	Concurrency control for c# connector and EntityFramework does not work
Submitted:	18 May 2016 7:55	Modified:	2 Aug 2016 6:29
Reporter:	jkjkjk gfgfgf	Email Updates:
Status:	Duplicate	Impact on me:	None
Category:	Connector / NET	Severity:	S2 (Serious)
Version:	7.0.3-DMR	OS:	Windows (EntityFramework 6)
Assigned to:		CPU Architecture:	Any

Description:
EntityFramework 6

If you use [ConcurrencyCheck] attribute on any column in the entity, none of the updates will ever be successfull.

Same bug existed in version 6.8.9 of connector and was never fixed. It is high time to fix it when new major version of connector is released as concurrency control is impossible in current state

How to repeat:
Use entity :

    public class User
    {
        [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
        [Key]
        public int UserId { get; set; }

        public int SomeValue {get;set;}
   
        [ConcurrencyCheck]
        [DatabaseGenerated(DatabaseGeneratedOption.Computed)]
        [Column(TypeName = "timestamp")]
        public DateTime RowVersion { get; set; }
}

Context

public class CouponsMysqlContext : DbContext
    {
                 public virtual DbSet<User> Users { get; set; }
}

On Update 
Following query will be generated :

UPDATE `Users` SET `IdentityId`=@gp1, `DisplayName`=@gp2 WHERE (`UserId` = 55619) AND (`RowVersion` = @gp3);
SELECT
`RowVersion`
FROM `Users`
 WHERE  row_count() > 0 and (`UserId` = 55619) AND (`RowVersion` = @gp4)

Which does not make any sense and will always fail as RowVersion is beeing regenerated by database after update and the library gives exactly same value for @gp3 and @gp4.

Suggested fix:
Remove 
AND (`RowVersion` = @gp4)

From select : 

SELECT
`RowVersion`
FROM `Users`
 WHERE  row_count() > 0 and (`UserId` = 55619)

Hello !,

Thank you for the bug report.
I could not repeat the issue at our end using with Visual Studio 2013, Connector/NET 7.0.3.
Could you please provide repeatable test case (create table statements, steps, sample code etc. - please make it as private if you prefer) to confirm this issue at our end?

Thanks,
Chiranjeevi.

No problem - bug is easy reproducable. It is necessary that the column is modified by database on update.

Table:
CREATE TABLE `entities` (
	`EntityId` INT(11) NOT NULL AUTO_INCREMENT,
	`Concurrent` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
	`Val` VARCHAR(50) NOT NULL COLLATE 'utf8_polish_ci',
	PRIMARY KEY (`EntityId`)
)
COLLATE='utf8_polish_ci'
ENGINE=InnoDB
AUTO_INCREMENT=3
;

Context:
    public class Context : DbContext
    {
        public Context() : base ("CS")
        {
            System.Data.Entity.Database.SetInitializer<Context>(null);
        }

        public DbSet<Entity> Entities { get; set; }
    }

Entity :

    public class Entity
    {
        [Key]
        public int EntityId { get; set; }

        [ConcurrencyCheck]
        [DatabaseGenerated(DatabaseGeneratedOption.Computed)]
        [Column(TypeName = "timestamp")]
        public DateTime Concurrent { get; set; }

        public string Val { get; set; }
    }

Run:

        static void Main(string[] args)
        {
            using (var c = new Context())
            {
                c.Database.Log = (s => Console.WriteLine(s));
                var obj = new Entity() {Val = "val1"};
                c.Entities.Add(obj);
                c.SaveChanges();

                obj.Val = "val2";
                c.SaveChanges();
            }
        }

Exception is on second save.

I have reproduced this fault in a unit test.  

From code inspection I will argue that the generated coded cannot possibly be correct ...

Statement #1: 

UPDATE `Users` SET `IdentityId`=@gp1, `DisplayName`=@gp2 WHERE (`UserId` = 55619) AND (`RowVersion` = @gp3);

- Correctly apply the optimistic lock condition `RowVersion` = @gp3

Statement #2 perform in the same transaction ...

SELECT `RowVersion` FROM `Users`
 WHERE  row_count() > 0 and (`UserId` = 55619) AND (`RowVersion` = @gp4)

BUG#1: Incorrectly applies the condition `RowVersion` = @gp3  since now the timestamp field RowVersion has been updated. This is due to the scheme defining  `RowVersion` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP  .  The attribute [DatabaseGenerated(DatabaseGeneratedOption.Computed)] indicates this. 

EF cannot know the value of RowVersion or apply any valid condition to it in the SELECT statement since it is changed by the UPDATE statement!

So the previously suggested fix of removing AND (`RowVersion` = @gp4) from the SELECT is correct.  

        [ConcurrencyCheck]
        [DatabaseGenerated(DatabaseGeneratedOption.Computed)]
        [Column(TypeName = "timestamp")]
        public DateTime RowVersion { get; set; }

This also indicates that there cannot be a Unit Test covering this important test case since any update on a record configured to use this method of concurrency checking will always fail.

Possible BUG#2: If we're only loading the updated timestamp value, we need to also take care that there are no updates possible between the UPDATE and SELECT statements.  If this is not the case, then the entire record (all fields) must be loaded, not just the timestamp field. So we may need to inspect the active isolation level to determine the correct SELECT statement.

Here's the code for UpdateGenerator.cs ...

https://github.com/mysql/mysql-connector-net/blob/7.0/Source/MySql.Data.EntityFramework5/G...

- Line 70 assigns the UPDATE statement predicate.
- Line 74 assigns the SELECT statement predicate.

I suspect that the VisitBinaryExpression() method needs 
to filter out the concurrency clause when _onReturningSelect == true

The existing Concurrency unit test uses this test case (Movie.cs):

```
public class MovieRelease
  {
    [Key, DatabaseGenerated(DatabaseGeneratedOption.None)]
    public virtual int Id { get; set; }

    [DatabaseGenerated(DatabaseGeneratedOption.Computed)]
    public virtual DateTime Timestamp { get; set; }

    [ConcurrencyCheck, Required, MaxLength(45)]
    public virtual string Name { get; set; }
  }
```

Which uses a timestamp but places the ConcurrencyCheck on another property.

- http://bugs.mysql.com/bug.php?id=73271 - DUPLICATE 2 years old - includes a patch.
  - http://bugs.mysql.com/bug.php?id=80033 - DUPLICATE 6 months old

Can you please explain why this 2 year old critical issue is not yet fixed!

This basically prevents implementation of optimistic locking with timestamps. This is critical for use of MySQL with EF6.

If you would kindly document how to setup the test environment then I may submit a tested patch. I see that some submitted a fix 2 years ago but they could not test it.

Hello !,

Thank you for your feedback.
This is most likely duplicate of Bug #73271, please see Bug #73271.

Thanks,
Chiranjeevi.