Add support for table group joins for subclasses in Criteria (#2545)

Fixes #1209, fixes #1215

Author: bahusoid

src/NHibernate.Test/Async/NHSpecificTest/NH2049/Fixture2049.cs

@@ -48,13 +48,12 @@ protected override void OnTearDown()
 		}
 
 		[Test]
-		[KnownBug("Known bug NH-2049.")]
 		public async Task CanCriteriaQueryWithFilterOnJoinClassBaseClassPropertyAsync()
 		{
 			using (ISession session = OpenSession())
 			{
 				session.EnableFilter("DeletedCustomer").SetParameter("deleted", false);
-				IList<Person> persons = await (session.CreateCriteria(typeof (Person)).ListAsync<Person>());
+				IList<Person> persons = await (session.QueryOver<Person>().JoinQueryOver(x => x.IndividualCustomer).ListAsync<Person>());
 
 				Assert.That(persons, Has.Count.EqualTo(1));
 				Assert.That(persons[0].Id, Is.EqualTo(1));

src/NHibernate.Test/Async/NHSpecificTest/NH2208/Filter.cs

@@ -17,13 +17,23 @@ namespace NHibernate.Test.NHSpecificTest.NH2208
 	public class FilterAsync : BugTestCase
 	{
 		[Test]
-		public async Task TestAsync()
+		public async Task TestHqlAsync()
 		{
 			using (ISession session = OpenSession())
 			{
 				session.EnableFilter("myfilter");
 				await (session.CreateQuery("from E1 e join fetch e.BO").ListAsync());
 			}
 		}
+
+		[Test]
+		public async Task TestQueryOverAsync()
+		{
+			using (ISession session = OpenSession())
+			{
+				session.EnableFilter("myfilter");
+				await (session.QueryOver<E1>().JoinQueryOver(x => x.BO).ListAsync());
+			}
+		}
 	}
 }

src/NHibernate.Test/NHSpecificTest/NH2049/Fixture2049.cs

@@ -37,13 +37,12 @@ protected override void OnTearDown()
 		}
 
 		[Test]
-		[KnownBug("Known bug NH-2049.")]
 		public void CanCriteriaQueryWithFilterOnJoinClassBaseClassProperty()
 		{
 			using (ISession session = OpenSession())
 			{
 				session.EnableFilter("DeletedCustomer").SetParameter("deleted", false);
-				IList<Person> persons = session.CreateCriteria(typeof (Person)).List<Person>();
+				IList<Person> persons = session.QueryOver<Person>().JoinQueryOver(x => x.IndividualCustomer).List<Person>();
 
 				Assert.That(persons, Has.Count.EqualTo(1));
 				Assert.That(persons[0].Id, Is.EqualTo(1));

src/NHibernate.Test/NHSpecificTest/NH2208/Filter.cs

@@ -6,13 +6,23 @@ namespace NHibernate.Test.NHSpecificTest.NH2208
 	public class Filter : BugTestCase
 	{
 		[Test]
-		public void Test()
+		public void TestHql()
 		{
 			using (ISession session = OpenSession())
 			{
 				session.EnableFilter("myfilter");
 				session.CreateQuery("from E1 e join fetch e.BO").List();
 			}
 		}
+
+		[Test]
+		public void TestQueryOver()
+		{
+			using (ISession session = OpenSession())
+			{
+				session.EnableFilter("myfilter");
+				session.QueryOver<E1>().JoinQueryOver(x => x.BO).List();
+			}
+		}
 	}
 }

src/NHibernate/Engine/IJoin.cs

@@ -0,0 +1,15 @@
+using NHibernate.Persister.Entity;
+using NHibernate.SqlCommand;
+using NHibernate.Type;
+
+namespace NHibernate.Engine
+{
+	internal interface IJoin
+	{
+		IJoinable Joinable { get;  }
+		string[] LHSColumns { get; }
+		string Alias { get;  }
+		IAssociationType AssociationType { get;  }
+		JoinType JoinType { get; }
+	}
+}

src/NHibernate/Engine/JoinSequence.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.Text;
 using NHibernate.Hql.Ast.ANTLR.Tree;
 using NHibernate.Persister.Collection;
@@ -41,7 +40,7 @@ public override string ToString()
 			return buf.Append('}').ToString();
 		}
 
-		private sealed class Join
+		private sealed class Join : IJoin
 		{
 			private readonly IAssociationType associationType;
 			private readonly IJoinable joinable;
@@ -50,7 +49,7 @@ private sealed class Join
 			private readonly string[] lhsColumns;
 
 			public Join(ISessionFactoryImplementor factory, IAssociationType associationType, string alias, JoinType joinType,
-			            string[] lhsColumns)
+						string[] lhsColumns)
 			{
 				this.associationType = associationType;
 				this.joinable = associationType.GetAssociatedJoinable(factory);
@@ -182,7 +181,7 @@ internal JoinFragment ToJoinFragment(
 				last = join.Joinable;
 			}
 
-			if (rootJoinable == null && ProcessAsTableGroupJoin(includeAllSubclassJoins, withClauses, joinFragment))
+			if (rootJoinable == null && TableGroupJoinHelper.ProcessAsTableGroupJoin(joins, withClauses, includeAllSubclassJoins, joinFragment, alias => IsIncluded(alias), factory))
 			{
 				return joinFragment;
 			}
@@ -253,117 +252,6 @@ private SqlString GetWithClause(IDictionary<string, IFilter> enabledFilters, ref
 			return SqlStringHelper.JoinParts(" and ", withConditions);
 		}
 
-		private bool ProcessAsTableGroupJoin(bool includeAllSubclassJoins, SqlString[] withClauseFragments, JoinFragment joinFragment)
-		{
-			if (!NeedsTableGroupJoin(joins, withClauseFragments, includeAllSubclassJoins))
-				return false;
-
-			var first = joins[0];
-			string joinString = ANSIJoinFragment.GetJoinString(first.JoinType);
-			joinFragment.AddFromFragmentString(
-				new SqlString(
-					joinString,
-					" (",
-					first.Joinable.TableName,
-					" ",
-					first.Alias
-				));
-
-			foreach (var join in joins)
-			{
-				if (join != first)
-					joinFragment.AddJoin(
-						join.Joinable.TableName,
-						join.Alias,
-						join.LHSColumns,
-						JoinHelper.GetRHSColumnNames(join.AssociationType, factory),
-						join.JoinType,
-						SqlString.Empty);
-
-				AddSubclassJoins(
-					joinFragment,
-					join.Alias,
-					join.Joinable,
-					// TODO (from hibernate): Think about if this could be made always true 
-					// NH Specific: made always true (original check: join.JoinType == JoinType.InnerJoin)
-					true,
-					includeAllSubclassJoins
-				);
-			}
-
-			var tableGroupWithClause = GetTableGroupJoinWithClause(withClauseFragments, first);
-			joinFragment.AddFromFragmentString(tableGroupWithClause);
-			return true;
-		}
-
-		private SqlString GetTableGroupJoinWithClause(SqlString[] withClauseFragments, Join first)
-		{
-			SqlStringBuilder fromFragment = new SqlStringBuilder();
-			fromFragment.Add(")").Add(" on ");
-
-			String[] lhsColumns = first.LHSColumns;
-			var isAssociationJoin = lhsColumns.Length > 0;
-			if (isAssociationJoin)
-			{
-				String rhsAlias = first.Alias;
-				String[] rhsColumns = JoinHelper.GetRHSColumnNames(first.AssociationType, factory);
-				for (int j = 0; j < lhsColumns.Length; j++)
-				{
-					fromFragment.Add(lhsColumns[j]);
-					fromFragment.Add("=");
-					fromFragment.Add(rhsAlias);
-					fromFragment.Add(".");
-					fromFragment.Add(rhsColumns[j]);
-					if (j < lhsColumns.Length - 1)
-					{
-						fromFragment.Add(" and ");
-					}
-				}
-			}
-
-			for (var i= 0; i < withClauseFragments.Length; i++)
-			{
-				var withClause = withClauseFragments[i];
-				if (SqlStringHelper.IsEmpty(withClause))
-					continue;
-
-				if (withClause.StartsWithCaseInsensitive(" and "))
-				{
-					if (!isAssociationJoin)
-					{
-						withClause = withClause.Substring(4);
-					}
-				}
-				else if (isAssociationJoin)
-				{
-					fromFragment.Add(" and ");
-				}
-
-				fromFragment.Add(withClause);
-			}
-
-			return fromFragment.ToSqlString();
-		}
-
-		private bool NeedsTableGroupJoin(List<Join> joins, SqlString[] withClauseFragments, bool includeSubclasses)
-		{
-			// If the rewrite is disabled or we don't have a with clause, we don't need a table group join
-			if ( /*!collectionJoinSubquery ||*/ withClauseFragments.All(x => SqlStringHelper.IsEmpty(x)))
-			{
-				return false;
-			}
-			// If we only have one join, a table group join is only necessary if subclass columns are used in the with clause
-			if (joins.Count == 1)
-			{
-				return joins[0].Joinable is AbstractEntityPersister persister && persister.HasSubclassJoins(includeSubclasses);
-				//NH Specific: No alias processing
-				//return isSubclassAliasDereferenced( joins[ 0], withClauseFragment );
-			}
-
-			//NH Specific: No alias processing (see hibernate JoinSequence.NeedsTableGroupJoin)
-			return true;
-		}
-
 		private bool IsManyToManyRoot(IJoinable joinable)
 		{
 			if (joinable != null && joinable.IsCollection)

src/NHibernate/Engine/TableGroupJoinHelper.cs

@@ -0,0 +1,127 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using NHibernate.Persister.Entity;
+using NHibernate.SqlCommand;
+
+namespace NHibernate.Engine
+{
+	//Generates table group join if neccessary. Example of generated query with table group join:
+	// SELECT *
+	// FROM Person person0_
+	// INNER JOIN (
+	// IndividualCustomer individual1_ 
+	// INNER JOIN Customer individual1_1_ ON individual1_.IndividualCustomerID = individual1_1_.Id
+	// ) ON person0_.Id = individual1_.PersonID AND individual1_1_.Deleted = @p0
+	internal class TableGroupJoinHelper
+	{
+		internal static bool ProcessAsTableGroupJoin(IReadOnlyList<IJoin> tableGroupJoinables, SqlString[] withClauseFragments, bool includeAllSubclassJoins, JoinFragment joinFragment, Func<string, bool> isSubclassIncluded, ISessionFactoryImplementor sessionFactoryImplementor)
+		{
+			if (!NeedsTableGroupJoin(tableGroupJoinables, withClauseFragments, includeAllSubclassJoins))
+				return false;
+
+			var first = tableGroupJoinables[0];
+			string joinString = ANSIJoinFragment.GetJoinString(first.JoinType);
+			joinFragment.AddFromFragmentString(
+				new SqlString(
+					joinString,
+					" (",
+					first.Joinable.TableName,
+					" ",
+					first.Alias
+				));
+
+			foreach (var join in tableGroupJoinables)
+			{
+				if (join != first)
+					joinFragment.AddJoin(
+						join.Joinable.TableName,
+						join.Alias,
+						join.LHSColumns,
+						JoinHelper.GetRHSColumnNames(join.AssociationType, sessionFactoryImplementor),
+						join.JoinType,
+						SqlString.Empty);
+
+				bool include = includeAllSubclassJoins && isSubclassIncluded(join.Alias);
+				// TODO (from hibernate): Think about if this could be made always true 
+				// NH Specific: made always true (original check: join.JoinType == JoinType.InnerJoin)
+				const bool innerJoin = true;
+				joinFragment.AddJoins(
+					join.Joinable.FromJoinFragment(join.Alias, innerJoin, include),
+					join.Joinable.WhereJoinFragment(join.Alias, innerJoin, include));
+			}
+
+			var withClause = GetTableGroupJoinWithClause(withClauseFragments, first, sessionFactoryImplementor);
+			joinFragment.AddFromFragmentString(withClause);
+			return true;
+		}
+
+		private static bool NeedsTableGroupJoin(IReadOnlyList<IJoin> joins, SqlString[] withClauseFragments, bool includeSubclasses)
+		{
+			// If we don't have a with clause, we don't need a table group join
+			if (withClauseFragments.All(x => SqlStringHelper.IsEmpty(x)))
+			{
+				return false;
+			}
+
+			// If we only have one join, a table group join is only necessary if subclass columns are used in the with clause
+			if (joins.Count == 1)
+			{
+				return joins[0].Joinable is AbstractEntityPersister persister && persister.HasSubclassJoins(includeSubclasses);
+				//NH Specific: No alias processing
+				//return isSubclassAliasDereferenced( joins[ 0], withClauseFragment );
+			}
+
+			//NH Specific: No alias processing (see hibernate JoinSequence.NeedsTableGroupJoin)
+			return true;
+		}
+
+		private static SqlString GetTableGroupJoinWithClause(SqlString[] withClauseFragments, IJoin first, ISessionFactoryImplementor factory)
+		{
+			SqlStringBuilder fromFragment = new SqlStringBuilder();
+			fromFragment.Add(")").Add(" on ");
+
+			string[] lhsColumns = first.LHSColumns;
+			var isAssociationJoin = lhsColumns.Length > 0;
+			if (isAssociationJoin)
+			{
+				string rhsAlias = first.Alias;
+				string[] rhsColumns = JoinHelper.GetRHSColumnNames(first.AssociationType, factory);
+				fromFragment.Add(lhsColumns[0]).Add("=").Add(rhsAlias).Add(".").Add(rhsColumns[0]);
+				for (int j = 1; j < lhsColumns.Length; j++)
+				{
+					fromFragment.Add(" and ").Add(lhsColumns[j]).Add("=").Add(rhsAlias).Add(".").Add(rhsColumns[j]);
+				}
+			}
+
+			AppendWithClause(fromFragment, isAssociationJoin, withClauseFragments);
+
+			return fromFragment.ToSqlString();
+		}
+
+		private static void AppendWithClause(SqlStringBuilder fromFragment, bool hasConditions, SqlString[] withClauseFragments)
+		{
+			for (var i = 0; i < withClauseFragments.Length; i++)
+			{
+				var withClause = withClauseFragments[i];
+				if (SqlStringHelper.IsEmpty(withClause))
+					continue;
+
+				if (withClause.StartsWithCaseInsensitive(" and "))
+				{
+					if (!hasConditions)
+					{
+						withClause = withClause.Substring(4);
+					}
+				}
+				else if (hasConditions)
+				{
+					fromFragment.Add(" and ");
+				}
+
+				fromFragment.Add(withClause);
+				hasConditions = true;
+			}
+		}
+	}
+}