NH-2408 - Fix pessimistic locking of union subclasses in Microsoft SQL Server dialects

Author: Dmitry Naumov

src/NHibernate.Test/DialectTest/LockHintAppenderFixture.cs

@@ -0,0 +1,86 @@
+using System.Collections.Generic;
+using NHibernate.Dialect;
+using NHibernate.SqlCommand;
+using NUnit.Framework;
+
+namespace NHibernate.Test.DialectTest
+{
+	[TestFixture]
+	public class LockHintAppenderFixture
+	{
+		private const string MsSql2000LockHint = " with (updlock, rowlock)";
+		private MsSql2000Dialect.LockHintAppender _appender;
+
+		[SetUp]
+		public void SetUp()
+		{
+			_appender = new MsSql2000Dialect.LockHintAppender(new MsSql2000Dialect(), new Dictionary<string, LockMode> { {"person", LockMode.Upgrade} });
+		}
+
+		[Test]
+		public void AppendHintToSingleTableAlias()
+		{
+			const string expectedQuery1 = "select * from Person person with (updlock, rowlock)";
+			const string expectedQuery2 = "select * from Person as person with (updlock, rowlock)";
+
+			var result1 = _appender.AppendLockHint(new SqlString(expectedQuery1.Replace(MsSql2000LockHint, string.Empty)));
+			Assert.That(result1.ToString(), Is.EqualTo(expectedQuery1));
+
+			var result2 = _appender.AppendLockHint(new SqlString(expectedQuery2.Replace(MsSql2000LockHint, string.Empty)));
+			Assert.That(result2.ToString(), Is.EqualTo(expectedQuery2));
+		}
+
+		[Test]
+		public void AppendHintToJoinedTableAlias()
+		{
+			const string expectedQuery =
+				"select * from Person person with (updlock, rowlock) inner join Country country on person.Id = country.Id";
+
+			var result = _appender.AppendLockHint(new SqlString(expectedQuery.Replace(MsSql2000LockHint, string.Empty)));
+			Assert.That(result.ToString(), Is.EqualTo(expectedQuery));
+		}
+
+		[Test]
+		public void AppendHintToUnionTableAlias()
+		{
+			const string expectedQuery =
+				"select Id, Name from (select Id, CONCAT(FirstName, LastName) from Employee with (updlock, rowlock) union all select Id, CONCAT(FirstName, LastName) from Manager with (updlock, rowlock)) as person";
+
+			var result = _appender.AppendLockHint(new SqlString(expectedQuery.Replace(MsSql2000LockHint, string.Empty)));
+			Assert.That(result.ToString(), Is.EqualTo(expectedQuery));
+		}
+
+		[Test]
+		public void ShouldIgnoreCasing()
+		{
+			const string expectedQuery =
+				"select Id, Name FROM (select Id, Name FROM Employee with (updlock, rowlock) union all select Id, Name from Manager with (updlock, rowlock)) as person";
+			
+			var result = _appender.AppendLockHint(new SqlString(expectedQuery.Replace(MsSql2000LockHint, string.Empty)));
+			Assert.That(result.ToString(), Is.EqualTo(expectedQuery));
+		}
+
+		[Test]
+		public void ShouldHandleEscapingInSubselect()
+		{
+			const string expectedQuery =
+				"select Id, Name from (select Id, Name from [Employee] with (updlock, rowlock) union all select Id, Name from [Manager] with (updlock, rowlock)) as person";
+
+			var result = _appender.AppendLockHint(new SqlString(expectedQuery.Replace(MsSql2000LockHint, string.Empty)));
+			Assert.That(result.ToString(), Is.EqualTo(expectedQuery));
+		}
+
+		[Test]
+		public void ShouldHandleMultilineQuery()
+		{
+			const string expectedQuery = @"
+select Id, Name from
+	(select Id, Name from Employee with (updlock, rowlock) union all
+	select Id, Name from Manager with (updlock, rowlock))
+as person";
+
+			var result = _appender.AppendLockHint(new SqlString(expectedQuery.Replace(MsSql2000LockHint, string.Empty)));
+			Assert.That(result.ToString(), Is.EqualTo(expectedQuery));
+		}
+	}
+}

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

@@ -0,0 +1,21 @@
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.NH2408
+{
+	public class Fixture : BugTestCase
+	{
+		[Test]
+		public void ShouldGenerateCorrectSqlStatement()
+		{
+			using (var session = OpenSession())
+			{
+				var query = session.CreateQuery("from Animal a where a.Name = ?");
+				query.SetParameter(0, "Prince");
+
+				query.SetLockMode("a", LockMode.Upgrade);
+
+				Assert.DoesNotThrow(() => query.List());
+			}
+		}
+	}
+}

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

@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2"
+           namespace="NHibernate.Test.NHSpecificTest.NH2408"
+           assembly="NHibernate.Test"
+>
+  <class name="Animal">
+    <id name="Id">
+      <generator class="increment"/>
+    </id>
+    <property name="Name"/>
+
+    <union-subclass name="Dog" table="`Dog`">
+    </union-subclass>
+
+    <union-subclass name="Cat">
+    </union-subclass>
+
+  </class>
+
+</hibernate-mapping>

src/NHibernate.Test/NHSpecificTest/NH2408/Model.cs

@@ -0,0 +1,17 @@
+namespace NHibernate.Test.NHSpecificTest.NH2408
+{
+	public class Animal
+	{
+		public virtual int Id { get; set; }
+
+		public virtual string Name { get; set; }
+	}
+
+	public class Dog : Animal
+	{
+	}
+
+	public class Cat : Animal
+	{
+	}
+}

src/NHibernate.Test/NHibernate.Test.csproj

@@ -218,6 +218,7 @@
     <Compile Include="Criteria\Reptile.cs" />
     <Compile Include="DialectTest\FunctionTests\SubstringSupportFixture.cs" />
     <Compile Include="DialectTest\FunctionTests\SequenceSupportFixture.cs" />
+    <Compile Include="DialectTest\LockHintAppenderFixture.cs" />
     <Compile Include="DialectTest\MsSqlCe40DialectFixture.cs" />
     <Compile Include="DialectTest\SchemaTests\ColumnMetaDataFixture.cs" />
     <Compile Include="DriverTest\DbProviderFactoryDriveConnectionCommandProviderTest.cs" />
@@ -672,6 +673,8 @@
     <Compile Include="NHSpecificTest\NH2297\Entity.cs" />
     <Compile Include="NHSpecificTest\NH2297\Fixture.cs" />
     <Compile Include="NHSpecificTest\NH2297\InvalidCustomCompositeUserTypeBase.cs" />
+    <Compile Include="NHSpecificTest\NH2408\Fixture.cs" />
+    <Compile Include="NHSpecificTest\NH2408\Model.cs" />
     <Compile Include="NHSpecificTest\NH3324\ChildEntity.cs" />
     <Compile Include="NHSpecificTest\NH3324\Entity.cs" />
     <Compile Include="NHSpecificTest\NH3324\FixtureByCode.cs" />
@@ -2879,6 +2882,7 @@
   </ItemGroup>
   <ItemGroup>
     <EmbeddedResource Include="NHSpecificTest\NH3408\Mappings.hbm.xml" />
+    <EmbeddedResource Include="NHSpecificTest\NH2408\Mappings.hbm.xml" />
     <Content Include="NHSpecificTest\NH3324\Mappings.hbm.xml" />
     <EmbeddedResource Include="NHSpecificTest\NH2297\MappingsNames.hbm.xml" />
     <EmbeddedResource Include="NHSpecificTest\NH2297\MappingsTypes.hbm.xml" />

src/NHibernate/Dialect/MsSql2000Dialect.cs

@@ -426,25 +426,6 @@ public override string AppendLockHint(LockMode lockMode, string tableName)
 			return tableName;
 		}
 
-		private struct LockHintAppender
-		{
-			private readonly MsSql2000Dialect dialect;
-			private readonly IDictionary<string, LockMode> aliasedLockModes;
-
-			public LockHintAppender(MsSql2000Dialect dialect, IDictionary<string, LockMode> aliasedLockModes)
-			{
-				this.dialect = dialect;
-				this.aliasedLockModes = aliasedLockModes;
-			}
-
-			public string ReplaceMatch(Match match)
-			{
-				string alias = match.Groups[1].Value;
-				string lockHint = dialect.AppendLockHint(aliasedLockModes[alias], alias);
-				return string.Concat(" ", lockHint, match.Groups[2].Value);
-			}
-		}
-
 		public override SqlString ApplyLocksToSql(SqlString sql, IDictionary<string, LockMode> aliasedLockModes, IDictionary<string, string[]> keyColumnNames)
 		{
 			bool doWork = false;
@@ -463,29 +444,7 @@ public override SqlString ApplyLocksToSql(SqlString sql, IDictionary<string, Loc
 				return sql;
 			}
 
-			// Regex matching any alias out of those given. Aliases should contain
-			// no dangerous characters (they are identifiers) so they are not escaped.
-			string aliasesPattern = StringHelper.Join("|", aliasedLockModes.Keys);
-
-			// Match < alias >, < alias,>, or < alias$>, the intent is to capture alias names
-			// in various kinds of "FROM table1 alias1, table2 alias2".
-			Regex matchRegex = new Regex(" (" + aliasesPattern + ")([, ]|$)");
-
-			SqlStringBuilder result = new SqlStringBuilder();
-			MatchEvaluator evaluator = new LockHintAppender(this, aliasedLockModes).ReplaceMatch;
-
-			foreach (object part in sql.Parts)
-			{
-				if (part == Parameter.Placeholder)
-				{
-					result.Add((Parameter)part);
-					continue;
-				}
-
-				result.Add(matchRegex.Replace((string)part, evaluator));
-			}
-
-			return result.ToSqlString();
+			return new LockHintAppender(this, aliasedLockModes).AppendLockHint(sql);
 		}
 
 		public override long TimestampResolutionInTicks
@@ -557,5 +516,72 @@ public override bool IsKnownToken(string currentToken, string nextToken)
 		{
 			return currentToken == "n" && nextToken == "'"; // unicode character
 		}
+
+		public struct LockHintAppender
+		{
+			private static readonly Regex FromClauseTableNameRegex = new Regex(@"from\s+\[?(\w+)\]?", RegexOptions.IgnoreCase | RegexOptions.Multiline);
+
+			private readonly MsSql2000Dialect _dialect;
+			private readonly IDictionary<string, LockMode> _aliasedLockModes;
+
+			private readonly Regex _matchRegex;
+			private readonly Regex _unionSubclassRegex;
+
+			public LockHintAppender(MsSql2000Dialect dialect, IDictionary<string, LockMode> aliasedLockModes)
+			{
+				_dialect = dialect;
+				_aliasedLockModes = aliasedLockModes;
+
+				// Regex matching any alias out of those given. Aliases should contain
+				// no dangerous characters (they are identifiers) so they are not escaped.
+				var aliasesPattern = StringHelper.Join("|", aliasedLockModes.Keys);
+
+				// Match < alias >, < alias,>, or < alias$>, the intent is to capture alias names
+				// in various kinds of "FROM table1 alias1, table2 alias2".
+				_matchRegex = new Regex(" (" + aliasesPattern + ")([, ]|$)");
+				_unionSubclassRegex = new Regex(@"from\s+\(((?:.|\r|\n)*)\)(?:\s+as)?\s+(?<alias>" + aliasesPattern + ")", RegexOptions.IgnoreCase | RegexOptions.Multiline);
+			}
+
+			public SqlString AppendLockHint(SqlString sql)
+			{
+				var result = new SqlStringBuilder();
+
+				foreach (object part in sql.Parts)
+				{
+					if (part == Parameter.Placeholder)
+					{
+						result.Add((Parameter)part);
+						continue;
+					}
+
+					result.Add(ProcessUnionSubclassCase((string)part) ?? _matchRegex.Replace((string)part, ReplaceMatch));
+				}
+
+				return result.ToSqlString();
+			}
+
+			private string ProcessUnionSubclassCase(string part)
+			{
+				var unionMatch = _unionSubclassRegex.Match(part);
+				if (!unionMatch.Success)
+				{
+					return null;
+				}
+
+				var alias = unionMatch.Groups["alias"].Value;
+				var lockMode = _aliasedLockModes[alias];
+				var @this = this;
+				var replacement = FromClauseTableNameRegex.Replace(unionMatch.Value, m => @this._dialect.AppendLockHint(lockMode, m.Value));
+
+				return _unionSubclassRegex.Replace(part, replacement);
+			}
+
+			private string ReplaceMatch(Match match)
+			{
+				string alias = match.Groups[1].Value;
+				string lockHint = _dialect.AppendLockHint(_aliasedLockModes[alias], alias);
+				return string.Concat(" ", lockHint, match.Groups[2].Value); // TODO: seems like this line is redundant
+			}
+		}
 	}
 }