Fix implied commits for Oracle and MySQL (#3485)

Author: fredericDelaporte

src/NHibernate.Test/Async/NHSpecificTest/GH3474/Fixture.cs

@@ -0,0 +1,81 @@
+//------------------------------------------------------------------------------
+// <auto-generated>
+//     This code was generated by AsyncGenerator.
+//
+//     Changes to this file may cause incorrect behavior and will be lost if
+//     the code is regenerated.
+// </auto-generated>
+//------------------------------------------------------------------------------
+
+
+using System.Linq;
+using NUnit.Framework;
+using NHibernate.Linq;
+
+namespace NHibernate.Test.NHSpecificTest.GH3474
+{
+	using System.Threading.Tasks;
+	[TestFixture]
+	public class FixtureAsync : BugTestCase
+	{
+		protected override void OnSetUp()
+		{
+			using var session = OpenSession();
+			using var transaction = session.BeginTransaction();
+
+			var e1 = new CreditCardPayment { CreditCardType = "Visa", Amount = 50 };
+			session.Save(e1);
+
+			var e2 = new ChequePayment { Bank = "CA", Amount = 32 };
+			session.Save(e2);
+
+			var e3 = new CashPayment { Amount = 18.5m };
+			session.Save(e3);
+
+			transaction.Commit();
+		}
+
+		protected override void OnTearDown()
+		{
+			using var session = OpenSession();
+			using var transaction = session.BeginTransaction();
+
+			// The HQL delete does all the job inside the database without loading the entities, but it does
+			// not handle delete order for avoiding violating constraints if any. Use
+			// session.Delete("from System.Object");
+			// instead if in need of having NHibernate ordering the deletes, but this will cause
+			// loading the entities in the session.
+			session.CreateQuery("delete from System.Object").ExecuteUpdate();
+
+			transaction.Commit();
+		}
+
+		[Test]
+		public async Task PolymorphicUpdateShouldNotCommitAsync()
+		{
+			using (var session = OpenSession())
+			using (var transaction = session.BeginTransaction())
+			{
+				var payment = await (session.Query<CreditCardPayment>().FirstAsync());
+				payment.Amount = 100;
+				await (session.FlushAsync());
+
+				await (session.CreateQuery("update ChequePayment set Amount = 64").ExecuteUpdateAsync());
+
+				await (transaction.RollbackAsync());
+			}
+
+			using (var session = OpenSession())
+			using (var transaction = session.BeginTransaction())
+			{
+				IPayment payment = await (session.Query<CreditCardPayment>().FirstAsync());
+				Assert.That(payment.Amount, Is.EqualTo(50m));
+
+				payment = await (session.Query<ChequePayment>().FirstAsync());
+				Assert.That(payment.Amount, Is.EqualTo(32m));
+
+				await (transaction.CommitAsync());
+			}
+		}
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH3474/Entities.cs

@@ -0,0 +1,30 @@
+using System;
+
+namespace NHibernate.Test.NHSpecificTest.GH3474
+{
+	public interface IPayment
+	{
+		public Guid Id { get; set; }
+		public decimal Amount { get; set; }
+	}
+
+	public class CreditCardPayment : IPayment
+	{
+		public virtual Guid Id { get; set; }
+		public virtual decimal Amount { get; set; }
+		public virtual string CreditCardType { get; set; }
+	}
+
+	public class CashPayment : IPayment
+	{
+		public virtual Guid Id { get; set; }
+		public virtual decimal Amount { get; set; }
+	}
+
+	public class ChequePayment : IPayment
+	{
+		public virtual Guid Id { get; set; }
+		public virtual decimal Amount { get; set; }
+		public virtual string Bank { get; set; }
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH3474/Fixture.cs

@@ -0,0 +1,69 @@
+using System.Linq;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.GH3474
+{
+	[TestFixture]
+	public class Fixture : BugTestCase
+	{
+		protected override void OnSetUp()
+		{
+			using var session = OpenSession();
+			using var transaction = session.BeginTransaction();
+
+			var e1 = new CreditCardPayment { CreditCardType = "Visa", Amount = 50 };
+			session.Save(e1);
+
+			var e2 = new ChequePayment { Bank = "CA", Amount = 32 };
+			session.Save(e2);
+
+			var e3 = new CashPayment { Amount = 18.5m };
+			session.Save(e3);
+
+			transaction.Commit();
+		}
+
+		protected override void OnTearDown()
+		{
+			using var session = OpenSession();
+			using var transaction = session.BeginTransaction();
+
+			// The HQL delete does all the job inside the database without loading the entities, but it does
+			// not handle delete order for avoiding violating constraints if any. Use
+			// session.Delete("from System.Object");
+			// instead if in need of having NHibernate ordering the deletes, but this will cause
+			// loading the entities in the session.
+			session.CreateQuery("delete from System.Object").ExecuteUpdate();
+
+			transaction.Commit();
+		}
+
+		[Test]
+		public void PolymorphicUpdateShouldNotCommit()
+		{
+			using (var session = OpenSession())
+			using (var transaction = session.BeginTransaction())
+			{
+				var payment = session.Query<CreditCardPayment>().First();
+				payment.Amount = 100;
+				session.Flush();
+
+				session.CreateQuery("update ChequePayment set Amount = 64").ExecuteUpdate();
+
+				transaction.Rollback();
+			}
+
+			using (var session = OpenSession())
+			using (var transaction = session.BeginTransaction())
+			{
+				IPayment payment = session.Query<CreditCardPayment>().First();
+				Assert.That(payment.Amount, Is.EqualTo(50m));
+
+				payment = session.Query<ChequePayment>().First();
+				Assert.That(payment.Amount, Is.EqualTo(32m));
+
+				transaction.Commit();
+			}
+		}
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH3474/Mappings.hbm.xml

@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2" assembly="NHibernate.Test"
+                   namespace="NHibernate.Test.NHSpecificTest.GH3474">
+
+  <class name="IPayment" table="Payment">
+    <id name="Id" generator="guid.comb" />
+    <property name="Amount" />
+    <joined-subclass name="CreditCardPayment">
+      <key column="Id" />
+      <property name="CreditCardType" />
+    </joined-subclass>
+    <joined-subclass name="CashPayment">
+      <key column="Id"/>
+    </joined-subclass>
+    <joined-subclass name="ChequePayment">
+      <key column="Id"/>
+      <property name="Bank" />
+    </joined-subclass>
+  </class>
+
+</hibernate-mapping>

src/NHibernate/Async/Hql/Ast/ANTLR/Exec/AbstractStatementExecutor.cs

@@ -162,7 +162,7 @@ public async Task DoWorkAsync(DbConnection connection, DbTransaction transaction
 				{
 					stmnt = connection.CreateCommand();
 					stmnt.Transaction = transaction;
-					stmnt.CommandText = "drop table " + persister.TemporaryIdTableName;
+					stmnt.CommandText = $"{session.Factory.Dialect.DropTemporaryTableString} {persister.TemporaryIdTableName}";
 					await (stmnt.ExecuteNonQueryAsync(cancellationToken)).ConfigureAwait(false);
 					session.Factory.Settings.SqlStatementLogger.LogCommand(stmnt, FormatStyle.Ddl);
 				}

src/NHibernate/Dialect/Dialect.cs

@@ -717,28 +717,28 @@ public virtual string GenerateTemporaryTableName(string baseTableName)
 
 		/// <summary> 
 		/// Does the dialect require that temporary table DDL statements occur in
-		/// isolation from other statements?  This would be the case if the creation
+		/// isolation from other statements? This would be the case if the creation
 		/// would cause any current transaction to get committed implicitly.
-		///  </summary>
-		/// <returns> see the result matrix above. </returns>
+		/// </summary>
+		/// <returns>See the result matrix in the remarks.</returns>
 		/// <remarks>
-		/// JDBC defines a standard way to query for this information via the
-		/// {@link java.sql.DatabaseMetaData#dataDefinitionCausesTransactionCommit()}
-		/// method.  However, that does not distinguish between temporary table
-		/// DDL and other forms of DDL; MySQL, for example, reports DDL causing a
-		/// transaction commit via its driver, even though that is not the case for
-		/// temporary table DDL.
-		/// <p/>
-		/// Possible return values and their meanings:<ul>
-		/// <li>{@link Boolean#TRUE} - Unequivocally, perform the temporary table DDL in isolation.</li>
-		/// <li>{@link Boolean#FALSE} - Unequivocally, do <b>not</b> perform the temporary table DDL in isolation.</li>
-		/// <li><i>null</i> - defer to the JDBC driver response in regards to {@link java.sql.DatabaseMetaData#dataDefinitionCausesTransactionCommit()}</li>
-		/// </ul>
+		/// Possible return values and their meanings:
+		/// <list type="bullet">
+		/// <item>
+		/// <term><see langword="true" /></term>
+		/// <description>Unequivocally, perform the temporary table DDL in isolation.</description>
+		/// </item>
+		/// <item>
+		/// <term><see langword="false" /></term>
+		/// <description>Unequivocally, do <b>not</b> perform the temporary table DDL in isolation.</description>
+		/// </item>
+		/// <item>
+		/// <term><see langword="null" /></term>
+		/// <description>Defer to <see cref="Cfg.Settings.IsDataDefinitionImplicitCommit" />.</description>
+		/// </item>
+		/// </list>
 		/// </remarks>
-		public virtual bool? PerformTemporaryTableDDLInIsolation()
-		{
-			return null;
-		}
+		public virtual bool? PerformTemporaryTableDDLInIsolation() => null;
 
 		/// <summary> Do we need to drop the temporary table after use? </summary>
 		public virtual bool DropTemporaryTableAfterUse()
@@ -2471,6 +2471,9 @@ public virtual string CreateTemporaryTableString
 			get { return "create table"; }
 		}
 
+		/// <summary>Command used to drop a temporary table.</summary>
+		public virtual string DropTemporaryTableString => "drop table";
+
 		/// <summary> 
 		/// Get any fragments needing to be postfixed to the command for
 		/// temporary table creation. 

src/NHibernate/Dialect/MySQLDialect.cs

@@ -451,6 +451,9 @@ public override string CreateTemporaryTableString
 			get { return "create temporary table if not exists"; }
 		}
 
+		/// <inheritdoc />
+		public override string DropTemporaryTableString => "drop temporary table";
+
 		protected virtual void RegisterCastTypes()
 		{
 			// According to the MySql documentation (http://dev.mysql.com/doc/refman/4.1/en/cast-functions.html)

src/NHibernate/Dialect/Oracle8iDialect.cs

@@ -532,6 +532,11 @@ public override string GenerateTemporaryTableName(String baseTableName)
 			return name.Length > 30 ? name.Substring(1, (30) - (1)) : name;
 		}
 
+		/// <inheritdoc />
+		/// <remarks>Oracle does commit any pending transaction prior to executing any DDL,
+		/// included for temporary tables.</remarks>
+		public override bool? PerformTemporaryTableDDLInIsolation() => true;
+
 		public override bool DropTemporaryTableAfterUse()
 		{
 			return false;

src/NHibernate/Hql/Ast/ANTLR/Exec/AbstractStatementExecutor.cs

@@ -298,7 +298,7 @@ public void DoWork(DbConnection connection, DbTransaction transaction)
 				{
 					stmnt = connection.CreateCommand();
 					stmnt.Transaction = transaction;
-					stmnt.CommandText = "drop table " + persister.TemporaryIdTableName;
+					stmnt.CommandText = $"{session.Factory.Dialect.DropTemporaryTableString} {persister.TemporaryIdTableName}";
 					stmnt.ExecuteNonQuery();
 					session.Factory.Settings.SqlStatementLogger.LogCommand(stmnt, FormatStyle.Ddl);
 				}