From 49b0bff584e6875be04acddc68913157c05e537b Mon Sep 17 00:00:00 2001 From: tony_ohagan Date: Tue, 1 May 2018 17:26:34 +1000 Subject: [PATCH] Fix ConcurrencyCheck + DatabaseGeneratedOption.Computed (only for non-DateTime fields) --- .../Generators/UpdateGenerator.cs | 36 ++++++++- .../CodeFirstTests.cs | 86 +++++++++++++++++++++- .../MySql.EntityFramework.CodeFirst.Tests/Movie.cs | 22 +++++- .../Properties/SQLSyntax.Designer.cs | 26 ++++++- .../Properties/SQLSyntax.resx | 10 ++- 5 files changed, 170 insertions(+), 10 deletions(-) diff --git a/Source/MySql.Data.EntityFramework5/Generators/UpdateGenerator.cs b/Source/MySql.Data.EntityFramework5/Generators/UpdateGenerator.cs index 6faf3033..0442f1d1 100644 --- a/Source/MySql.Data.EntityFramework5/Generators/UpdateGenerator.cs +++ b/Source/MySql.Data.EntityFramework5/Generators/UpdateGenerator.cs @@ -80,8 +80,9 @@ protected override SelectStatement GenerateReturningSql(DbModificationCommandTre { SelectStatement select = base.GenerateReturningSql(tree, returning); ListFragment where = new ListFragment(); - where.Append(" row_count() > 0 and "); + where.Append(" row_count() = 1 and ("); where.Append( ((DbUpdateCommandTree)tree).Predicate.Accept(this) ); + where.Append(")"); select.Where = where; return select; @@ -89,6 +90,39 @@ protected override SelectStatement GenerateReturningSql(DbModificationCommandTre private Stack _columnsVisited = new Stack(); + public override SqlFragment Visit(DbAndExpression expression) + { + if (_onReturningSelect) + { + if (IsExcludedCondition(expression.Left)) + { + return expression.Right.Accept(this); + } + + if (IsExcludedCondition(expression.Right)) + { + return expression.Left.Accept(this); + } + } + + return base.Visit(expression); + } + + private bool IsExcludedCondition(DbExpression e) + { + var expr = e as DbComparisonExpression; + if (expr == null) return false; + var propExpr = expr.Left as DbPropertyExpression; + if (propExpr == null) return false; + + Facet item = null; + if (propExpr.Property.TypeUsage.Facets.TryGetValue("StoreGeneratedPattern", false, out item)) + { + return (StoreGeneratedPattern)item.Value == StoreGeneratedPattern.Computed; + } + return false; + } + protected override SqlFragment VisitBinaryExpression(DbExpression left, DbExpression right, string op) { BinaryFragment f = new BinaryFragment(); diff --git a/Tests/MySql.EntityFramework.CodeFirst.Tests/CodeFirstTests.cs b/Tests/MySql.EntityFramework.CodeFirst.Tests/CodeFirstTests.cs index fa2821e6..a35d09b8 100644 --- a/Tests/MySql.EntityFramework.CodeFirst.Tests/CodeFirstTests.cs +++ b/Tests/MySql.EntityFramework.CodeFirst.Tests/CodeFirstTests.cs @@ -129,7 +129,7 @@ public void AlterTableTest() /// Fix for "Connector/Net Generates Incorrect SELECT Clause after UPDATE" (MySql bug #62134, Oracle bug #13491689). /// [Fact] - public void ConcurrencyCheck() + public void ConcurrencyCheckWithNonDbGeneratedColumn() { #if DEBUG Debug.WriteLine(new StackTrace().GetFrame(0).GetMethod().Name); @@ -176,7 +176,7 @@ PRIMARY KEY (`Id`) Match m = rx.Match(s); if (m.Success) { - st.CheckSql(m.Groups["item"].Value, SQLSyntax.UpdateWithSelect); + st.CheckSql(m.Groups["item"].Value, SQLSyntax.UpdateWithSelectWithNonDbGeneratedLock); //Assert.Pass(); } } @@ -184,6 +184,88 @@ PRIMARY KEY (`Id`) } } + /// + /// Fix for "Incorrect Sql generated for EF with ConcurrencyCheck and DatabaseGenerated" (MySql bug #73271). + /// Database generated optimistic locking is required to support locking with external (non-EF) systems. + /// + [Fact] + public void ConcurrencyCheckWithDbGeneratedColumn() + { +#if DEBUG + Debug.WriteLine(new StackTrace().GetFrame(0).GetMethod().Name); +#endif + ReInitDb(); + using (MovieDBContext db = new MovieDBContext()) + { + db.Database.Delete(); + db.Database.CreateIfNotExists(); +#if EF6 + db.Database.Log = (e) => Debug.WriteLine(e); +#endif + db.Database.ExecuteSqlCommand(@"DROP TABLE IF EXISTS `MovieReleases2`"); + + db.Database.ExecuteSqlCommand( + @"CREATE TABLE IF NOT EXISTS `MovieRelease2` ( + `Id` int(11) NOT NULL, + `Name` varchar(45) NOT NULL, + `Timestamp` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + `RowVersion` bigint NOT NULL DEFAULT 0, + PRIMARY KEY (`Id`) + ) ENGINE=InnoDB DEFAULT CHARSET=binary"); + + db.Database.ExecuteSqlCommand( + @"CREATE TRIGGER `trg_MovieRelease2_before_update` + BEFORE UPDATE ON `MovieRelease2` + FOR EACH ROW SET NEW.RowVersion = OLD.RowVersion + 1;"); + + MySqlTrace.Listeners.Clear(); + MySqlTrace.Switch.Level = SourceLevels.All; + GenericListener listener = new GenericListener(); + MySqlTrace.Listeners.Add(listener); + + try + { + MovieRelease2 mr = db.MovieReleases2.Create(); + mr.Id = 1; + mr.Name = "Commercial"; + db.MovieReleases2.Add(mr); + Assert.Equal(mr.RowVersion, 0); + db.SaveChanges(); // ADD + Assert.Equal(mr.RowVersion, 0); + + mr.Name = "Director's Cut"; + db.SaveChanges(); // UPDATE #1 + Assert.Equal(mr.RowVersion, 1); + + mr.Name = "Avengers"; + db.SaveChanges(); // UPDATE #2 + Assert.Equal(mr.RowVersion, 2); + } + finally + { + db.Database.ExecuteSqlCommand(@"DROP TABLE IF EXISTS `MovieReleases2`"); + } + // Check sql + Regex rx = new Regex(@"Query Opened: (?UPDATE .*)", RegexOptions.Compiled | RegexOptions.Singleline); + int n = 0; + foreach (string s in listener.Strings) + { + Match m = rx.Match(s); + if (m.Success) + { + if (n++ == 0) + { + st.CheckSql(m.Groups["item"].Value, SQLSyntax.UpdateWithSelectWithDbGeneratedLock1); + } + else + { + st.CheckSql(m.Groups["item"].Value, SQLSyntax.UpdateWithSelectWithDbGeneratedLock2); + } + } + } + } + } + /// /// This tests fix for http://bugs.mysql.com/bug.php?id=64216. /// diff --git a/Tests/MySql.EntityFramework.CodeFirst.Tests/Movie.cs b/Tests/MySql.EntityFramework.CodeFirst.Tests/Movie.cs index 592299fc..6b7c321d 100644 --- a/Tests/MySql.EntityFramework.CodeFirst.Tests/Movie.cs +++ b/Tests/MySql.EntityFramework.CodeFirst.Tests/Movie.cs @@ -75,6 +75,7 @@ public class MovieDBContext : DbContext public DbSet Movies { get; set; } public DbSet MovieFormats { get; set; } public DbSet MovieReleases { get; set; } + public DbSet MovieReleases2 { get; set; } public DbSet EntitySingleColumns { get; set; } public DbSet Medias { get; set; } @@ -96,7 +97,7 @@ protected override void OnModelCreating(DbModelBuilder modelBuilder) modelBuilder.Entity().Property(x => x.Price).HasPrecision(16, 2); modelBuilder.Entity().HasMany(p => p.Formats); modelBuilder.Entity().HasMany( p => p.Medias ); -} + } } public class EntitySingleColumn @@ -112,10 +113,29 @@ public class MovieRelease [DatabaseGenerated(DatabaseGeneratedOption.Computed)] public virtual DateTime Timestamp { get; set; } + // Test: ConcurrencyCheck + Not Computed [ConcurrencyCheck, Required, MaxLength(45)] public virtual string Name { get; set; } } + public class MovieRelease2 + { + [Key, DatabaseGenerated(DatabaseGeneratedOption.None)] + public virtual int Id { get; set; } + + //[DatabaseGenerated(DatabaseGeneratedOption.Computed)] + //public virtual DateTime Timestamp { get; set; } + + // Test: non computed column + [Required, MaxLength(45)] + public virtual string Name { get; set; } + + // Test: ConcurrencyCheck + Computed + [ConcurrencyCheck, DatabaseGenerated(DatabaseGeneratedOption.Computed)] + [Column(TypeName = "bigint")] + public virtual long RowVersion { get; set; } + } + public class MovieDBInitialize : DropCreateDatabaseReallyAlways { public static Movie[] data = new Movie[] { diff --git a/Tests/MySql.EntityFramework.CodeFirst.Tests/Properties/SQLSyntax.Designer.cs b/Tests/MySql.EntityFramework.CodeFirst.Tests/Properties/SQLSyntax.Designer.cs index 1c84e788..9e976440 100644 --- a/Tests/MySql.EntityFramework.CodeFirst.Tests/Properties/SQLSyntax.Designer.cs +++ b/Tests/MySql.EntityFramework.CodeFirst.Tests/Properties/SQLSyntax.Designer.cs @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. -// Runtime Version:4.0.30319.34014 +// Runtime Version:4.0.30319.42000 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. @@ -475,11 +475,29 @@ internal class SQLSyntax { } /// - /// Looks up a localized string similar to UPDATE `MovieReleases` SET `Name`='Director\'s Cut' WHERE (`Id` = 1) AND (`Name` = 'Commercial'); SELECT `Timestamp` FROM `MovieReleases` WHERE row_count() > 0 and (`Id` = 1) AND (`Name` = 'Director\'s Cut'). + /// Looks up a localized string similar to UPDATE `MovieRelease2` SET `Name`='Director\'s Cut' WHERE (`Id` = 1) AND (`RowVersion` = 0); SELECT `RowVersion` FROM `MovieRelease2` WHERE row_count() = 1 and (`Id` = 1). /// - internal static string UpdateWithSelect { + internal static string UpdateWithSelectWithDbGeneratedLock1 { get { - return ResourceManager.GetString("UpdateWithSelect", resourceCulture); + return ResourceManager.GetString("UpdateWithSelectWithDbGeneratedLock1", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to UPDATE `MovieRelease2` SET `Name`='Avengers' WHERE (`Id` = 1) AND (`RowVersion` = 1); SELECT `RowVersion` FROM `MovieRelease2` WHERE row_count() = 1 and (`Id` = 1). + /// + internal static string UpdateWithSelectWithDbGeneratedLock2 { + get { + return ResourceManager.GetString("UpdateWithSelectWithDbGeneratedLock2", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to UPDATE `MovieReleases` SET `Name`='Director\'s Cut' WHERE (`Id` = 1) AND (`Name` = 'Commercial'); SELECT `Timestamp` FROM `MovieReleases` WHERE row_count() = 1 and ((`Id` = 1) AND (`Name` = 'Director\'s Cut')). + /// + internal static string UpdateWithSelectWithNonDbGeneratedLock { + get { + return ResourceManager.GetString("UpdateWithSelectWithNonDbGeneratedLock", resourceCulture); } } } diff --git a/Tests/MySql.EntityFramework.CodeFirst.Tests/Properties/SQLSyntax.resx b/Tests/MySql.EntityFramework.CodeFirst.Tests/Properties/SQLSyntax.resx index d5a25f71..b016f4dc 100644 --- a/Tests/MySql.EntityFramework.CodeFirst.Tests/Properties/SQLSyntax.resx +++ b/Tests/MySql.EntityFramework.CodeFirst.Tests/Properties/SQLSyntax.resx @@ -325,8 +325,14 @@ FROM `CrewMembers` AS `Extent3` INNER JOIN `Ranks` AS `Extent4` ON `Extent3`.`Ra `Project1`.`ShipId` ASC, `Project1`.`C1` ASC - - UPDATE `MovieReleases` SET `Name`='Director\'s Cut' WHERE (`Id` = 1) AND (`Name` = 'Commercial'); SELECT `Timestamp` FROM `MovieReleases` WHERE row_count() > 0 and (`Id` = 1) AND (`Name` = 'Director\'s Cut') + + UPDATE `MovieReleases` SET `Name`='Director\'s Cut' WHERE (`Id` = 1) AND (`Name` = 'Commercial'); SELECT `Timestamp` FROM `MovieReleases` WHERE row_count() = 1 and ((`Id` = 1) AND (`Name` = 'Director\'s Cut')) + + + UPDATE `MovieRelease2` SET `Name`='Director\'s Cut' WHERE (`Id` = 1) AND (`RowVersion` = 0); SELECT `RowVersion` FROM `MovieRelease2` WHERE row_count() = 1 and (`Id` = 1) + + + UPDATE `MovieRelease2` SET `Name`='Avengers' WHERE (`Id` = 1) AND (`RowVersion` = 1); SELECT `RowVersion` FROM `MovieRelease2` WHERE row_count() = 1 and (`Id` = 1) SELECT