Fix for update with outer join in PostgreSQL

src/NHibernate.Test/Async/NHSpecificTest/GH1565/LockEntityWithOuterJoinTest.cs

@@ -0,0 +1,59 @@
+//------------------------------------------------------------------------------
+// <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 NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.GH1565
+{
+	using System.Threading.Tasks;
+	[TestFixture]
+	public class LockEntityWithOuterJoinTestAsync : BugTestCase
+	{
+		[Test]
+		public async Task LockWithOuterJoin_ShouldBePossibleAsync()
+		{
+			using (var session = OpenSession())
+			{
+				using (var transaction = session.BeginTransaction())
+				{
+					var entity = await (session.GetAsync<MainEntity>(id, LockMode.Upgrade));
+					Assert.That(entity.Id, Is.EqualTo(id));
+					await (transaction.CommitAsync());
+				}
+			}
+		}
+
+		private int id;
+		protected override void OnSetUp()
+		{
+			base.OnSetUp();
+			using (var session = OpenSession())
+			{
+				using (var transaction = session.BeginTransaction())
+				{
+					session.FlushMode = FlushMode.Auto;
+					var entity = new MainEntity();
+					session.Save(entity);
+					transaction.Commit();
+					id = entity.Id;
+				}
+			}
+		}
+
+		protected override void OnTearDown()
+		{
+			base.OnTearDown();
+			using (var session = OpenSession())
+			{
+				session.CreateSQLQuery("delete from MainEntity").ExecuteUpdate();
+			}
+		}
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH1565/LockEntityWithOuterJoinTest.cs

@@ -0,0 +1,55 @@
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.GH1565
+{
+	[TestFixture]
+	public class LockEntityWithOuterJoinTest : BugTestCase
+	{
+		[Test]
+		public void LockWithOuterJoin_ShouldBePossible()
+		{
+			using (var session = OpenSession())
+			{
+				using (var transaction = session.BeginTransaction())
+				{
+					var entity = session.Get<MainEntity>(id, LockMode.Upgrade);
+					Assert.That(entity.Id, Is.EqualTo(id));
+					transaction.Commit();
+				}
+			}
+		}
+
+		private int id;
+		protected override void OnSetUp()
+		{
+			base.OnSetUp();
+			using (var session = OpenSession())
+			{
+				using (var transaction = session.BeginTransaction())
+				{
+					session.FlushMode = FlushMode.Auto;
+					var entity = new MainEntity();
+					session.Save(entity);
+					transaction.Commit();
+					id = entity.Id;
+				}
+			}
+		}
+
+		protected override void OnTearDown()
+		{
+			base.OnTearDown();
+			using (var session = OpenSession())
+			{
+				session.CreateSQLQuery("delete from MainEntity").ExecuteUpdate();
+			}
+		}
+	}
+
+	public class MainEntity
+	{
+		public virtual int Id { get; set; } = 0;
+
+		public virtual string Data { get; set; }
+	}
+}

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

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2" assembly="NHibernate.Test"
+                   namespace="NHibernate.Test.NHSpecificTest.GH1565">
+
+  <class name="MainEntity">
+    <id name="Id" unsaved-value="0">
+      <generator class="native" />
+    </id>
+
+    <join table="EntityData" inverse="true">
+      <key column="EntityId"/>
+      <property name="Data" />
+    </join>
+  </class>
+</hibernate-mapping>

src/NHibernate/Async/Dialect/InformixDialect.cs

@@ -8,14 +8,15 @@
 //------------------------------------------------------------------------------
 
 
+using System;
 using System.Data;
 using System.Data.Common;
 using System.Text;
-using NHibernate.Cfg;
 using NHibernate.Dialect.Function;
 using NHibernate.Exceptions;
 using NHibernate.SqlCommand;
 using NHibernate.Util;
+using Environment = NHibernate.Cfg.Environment;
 
 //using NHibernate.Dialect.Schema;
 

src/NHibernate/Async/Loader/Hql/QueryLoader.cs

@@ -121,4 +121,4 @@ internal async Task<IEnumerable> GetEnumerableAsync(QueryParameters queryParamet
 			return result;
 		}
 	}
-}
+}

src/NHibernate/Dialect/Dialect.cs

@@ -501,15 +501,15 @@ public virtual ILockingStrategy GetLockingStrategy(ILockable lockable, LockMode
 		/// <returns> The appropriate for update fragment. </returns>
 		public virtual string GetForUpdateString(LockMode lockMode)
 		{
-			if (lockMode == LockMode.Upgrade)
+			if (Equals(lockMode, LockMode.Upgrade))
 			{
 				return ForUpdateString;
 			}
-			if (lockMode == LockMode.UpgradeNoWait)
+			if (Equals(lockMode, LockMode.UpgradeNoWait))
 			{
 				return ForUpdateNowaitString;
 			}
-			if (lockMode == LockMode.Force)
+			if (Equals(lockMode, LockMode.Force))
 			{
 				return ForUpdateNowaitString;
 			}
@@ -526,14 +526,30 @@ public virtual string ForUpdateString
 			get { return " for update"; }
 		}
 
-		/// <summary> Is <tt>FOR UPDATE OF</tt> syntax supported? </summary>
-		/// <value> True if the database supports <tt>FOR UPDATE OF</tt> syntax; false otherwise. </value>
+		/// <summary>Is <c>FOR UPDATE OF</c> syntax supported?</summary>
+		/// <value><see langword="true"/> if the database supports <c>FOR UPDATE OF</c> syntax; <see langword="false"/> otherwise. </value>
+		public virtual bool SupportsForUpdateOf
+			// By default, just check UsesColumnsWithForUpdateOf. ForUpdateOf needs to be overriden only for dialects supporting
+			// "For Update Of" on table aliases.
+			=> UsesColumnsWithForUpdateOf;
+
+		/// <summary>Is <c>FOR UPDATE OF</c> syntax expecting columns?</summary>
+		/// <value><see langword="true"/> if the database expects a column list with <c>FOR UPDATE OF</c> syntax,
+		/// <see langword="false"/> if it expects table alias instead or do not support <c>FOR UPDATE OF</c> syntax.</value>
+		// Since v5.1
+		[Obsolete("Use UsesColumnsWithForUpdateOf instead")]
 		public virtual bool ForUpdateOfColumns
 		{
 			// by default we report no support
 			get { return false; }
 		}
 
+		public virtual bool UsesColumnsWithForUpdateOf
+#pragma warning disable 618
+			// For avoiding a breaking change, we need to call the old name by default.
+			=> ForUpdateOfColumns;
+#pragma warning restore 618
+
 		/// <summary> 
 		/// Does this dialect support <tt>FOR UPDATE</tt> in conjunction with outer joined rows?
 		/// </summary>
@@ -567,11 +583,11 @@ public virtual string ForUpdateNowaitString
 		}
 
 		/// <summary> 
-		/// Get the <tt>FOR UPDATE OF column_list NOWAIT</tt> fragment appropriate
-		/// for this dialect given the aliases of the columns to be write locked.
+		/// Get the <c>FOR UPDATE OF column_list NOWAIT</c> fragment appropriate
+		/// for this dialect given the aliases of the columns or tables to be write locked.
 		/// </summary>
-		/// <param name="aliases">The columns to be write locked. </param>
-		/// <returns> The appropriate <tt>FOR UPDATE colunm_list NOWAIT</tt> clause string. </returns>
+		/// <param name="aliases">The columns or tables to be write locked.</param>
+		/// <returns>The appropriate <c>FOR UPDATE colunm_or_table_list NOWAIT</c> clause string.</returns>
 		public virtual string GetForUpdateNowaitString(string aliases)
 		{
 			return GetForUpdateString(aliases);

src/NHibernate/Dialect/InformixDialect.cs

@@ -1,11 +1,12 @@
+using System;
 using System.Data;
 using System.Data.Common;
 using System.Text;
-using NHibernate.Cfg;
 using NHibernate.Dialect.Function;
 using NHibernate.Exceptions;
 using NHibernate.SqlCommand;
 using NHibernate.Util;
+using Environment = NHibernate.Cfg.Environment;
 
 //using NHibernate.Dialect.Schema;
 
@@ -175,13 +176,19 @@ public override string AddColumnString
 		//    throw new NotSupportedException();
 		//}
 
-		/// <summary> Is <tt>FOR UPDATE OF</tt> syntax supported? </summary>
-		/// <value> True if the database supports <tt>FOR UPDATE OF</tt> syntax; false otherwise. </value>
+		/// <inheritdoc />
+		// Since v5.1
+		[Obsolete("Use UsesColumnsWithForUpdateOf instead")]
 		public override bool ForUpdateOfColumns
 		{
 			get { return true; }
 		}
 
+		/* 6.0 TODO: uncomment once ForUpdateOfColumns is removed.
+		/// <inheritdoc />
+		public override bool UsesColumnsWithForUpdateOf => true;
+		*/
+
 		/// <summary> 
 		/// Does this dialect support <tt>FOR UPDATE</tt> in conjunction with outer joined rows?
 		/// </summary>

src/NHibernate/Dialect/Oracle8iDialect.cs

@@ -485,11 +485,18 @@ public override bool UseMaxForLimit
 			get { return true; }
 		}
 
+		// Since v5.1
+		[Obsolete("Use UsesColumnsWithForUpdateOf instead")]
 		public override bool ForUpdateOfColumns
 		{
 			get { return true; }
 		}
 
+		/* 6.0 TODO: uncomment once ForUpdateOfColumns is removed.
+		/// <inheritdoc />
+		public override bool UsesColumnsWithForUpdateOf => true;
+		*/
+
 		public override bool SupportsUnionAll
 		{
 			get { return true; }

src/NHibernate/Dialect/PostgreSQLDialect.cs

@@ -227,6 +227,12 @@ public override SqlString GetLimitString(SqlString queryString, SqlString offset
 			return pagingBuilder.ToSqlString();
 		}
 
+		/// <inheritdoc />
+		public override bool SupportsForUpdateOf => true;
+
+		/// <inheritdoc />
+		public override bool SupportsOuterJoinForUpdate => false;
+
 		public override string GetForUpdateString(string aliases)
 		{
 			return ForUpdateString + " of " + aliases;

src/NHibernate/Dialect/SybaseSQLAnywhere10Dialect.cs

@@ -643,11 +643,29 @@ public override string GetForUpdateString(LockMode lockMode)
 		/// In this dialect, we avoid this issue by supporting only
 		/// <tt>FOR UPDATE BY LOCK</tt>.
 		/// </summary>
+		// Since v5.1
+		[Obsolete("Use UsesColumnsWithForUpdateOf instead")]
 		public override bool ForUpdateOfColumns
 		{
 			get { return false; }
 		}
 
+		/* 6.0 TODO: uncomment once ForUpdateOfColumns is removed.
+		/// <summary>
+		/// SQL Anywhere does support <c>FOR UPDATE OF</c> syntax. However,
+		/// in SQL Anywhere one cannot specify both <c>FOR UPDATE OF</c> syntax
+		/// and <c>FOR UPDATE BY LOCK</c> in the same statement. To achieve INTENT
+		/// locking when using <c>FOR UPDATE OF</c> syntax one must use a table hint
+		/// in the query's FROM clause, ie.
+		/// <code>
+		/// SELECT * FROM FOO WITH( UPDLOCK ) FOR UPDATE OF ( column-list ).
+		/// </code>
+		/// In this dialect, we avoid this issue by supporting only
+		/// <c>FOR UPDATE BY LOCK</c>.
+		/// </summary>
+		public override bool UsesColumnsWithForUpdateOf => false;
+		*/
+
 		/// <summary>
 		/// SQL Anywhere supports <tt>FOR UPDATE</tt> over cursors containing
 		/// outer joins.

src/NHibernate/Loader/AbstractEntityJoinWalker.cs

@@ -118,7 +118,7 @@ private void InitStatementString(SqlString projection, SqlString condition, SqlS
 			JoinFragment ojf = MergeOuterJoins(associations);
 
 			SqlSelectBuilder select = new SqlSelectBuilder(Factory)
-				.SetLockMode(lockMode)
+				.SetLockMode(lockMode, alias)
 				.SetSelectClause(selectClause)
 				.SetFromClause(Dialect.AppendLockHint(lockMode, persister.FromTableFragment(alias)) +persister.FromJoinFragment(alias, true, true))
 				.SetWhereClause(condition)

src/NHibernate/Loader/Criteria/CriteriaLoader.cs

@@ -171,7 +171,7 @@ protected override SqlString ApplyLocks(SqlString sqlSelectString, IDictionary<s
 			}
 
 			Dictionary<string, LockMode> aliasedLockModes = new Dictionary<string, LockMode>();
-			Dictionary<string, string[]> keyColumnNames = dialect.ForUpdateOfColumns ? new Dictionary<string, string[]>() : null;
+			Dictionary<string, string[]> keyColumnNames = dialect.UsesColumnsWithForUpdateOf ? new Dictionary<string, string[]>() : null;
 			string[] drivingSqlAliases = Aliases;
 
 			//NH-3710: if we are issuing an aggregation function, Aliases will be null

src/NHibernate/Loader/Hql/QueryLoader.cs

@@ -71,7 +71,7 @@ protected override SqlString ApplyLocks(SqlString sql, IDictionary<string, LockM
 			// we are given a map of user-alias -> lock mode
 			// create a new map of sql-alias -> lock mode
 			var aliasedLockModes = new Dictionary<string, LockMode>();
-			Dictionary<string, string[]> keyColumnNames = dialect.ForUpdateOfColumns ? new Dictionary<string, string[]>() : null;
+			Dictionary<string, string[]> keyColumnNames = dialect.UsesColumnsWithForUpdateOf ? new Dictionary<string, string[]>() : null;
 
 			foreach (var entry in lockModes)
 			{
@@ -456,4 +456,4 @@ protected override IEnumerable<IParameterSpecification> GetParameterSpecificatio
 			return _queryTranslator.CollectedParameterSpecifications;
 		}
 	}
-}
+}

src/NHibernate/SqlCommand/ForUpdateFragment.cs

@@ -30,7 +30,7 @@ public ForUpdateFragment(Dialect.Dialect dialect, IDictionary<string, LockMode>
 				if (LockMode.Read.LessThan(lockMode))
 				{
 					string tableAlias = me.Key;
-					if (dialect.ForUpdateOfColumns)
+					if (dialect.UsesColumnsWithForUpdateOf)
 					{
 						string[] keyColumns = keyColumnNames[tableAlias];
 						if (keyColumns == null)
@@ -89,4 +89,4 @@ public string ToSqlStringFragment()
 					: dialect.GetForUpdateString(aliases.ToString());
 		}
 	}
-}
+}