Do not move ON condition to Where clause in LINQ (#2539)

bahusoid · web-flow · commit 32a022777c86 · 2020-09-15T20:53:48.000Z
diff --git a/src/NHibernate.Test/Async/Linq/LinqQuerySamples.cs b/src/NHibernate.Test/Async/Linq/LinqQuerySamples.cs
@@ -1078,51 +1078,40 @@ from c in db.Customers
 		}
 
 		[Category("JOIN")]
-		[TestCase(true, Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
-		[TestCase(false, Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
-		public async Task DLinqJoin5dAsync(bool useCrossJoin)
+		[Test(Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
+		public async Task DLinqJoin5dAsync()
 		{
-			if (useCrossJoin && !Dialect.SupportsCrossJoin)
-			{
-				Assert.Ignore("Dialect does not support cross join.");
-			}
-
 			var q =
 				from c in db.Customers
 				join o in db.Orders on
 					new {c.CustomerId, HasContractTitle = c.ContactTitle != null} equals
 					new {o.Customer.CustomerId, HasContractTitle = o.Customer.ContactTitle != null }
 				select new { c.ContactName, o.OrderId };
 
-			using (var substitute = SubstituteDialect())
 			using (var sqlSpy = new SqlLogSpy())
 			{
-				ClearQueryPlanCache();
-				substitute.Value.SupportsCrossJoin.Returns(useCrossJoin);
-
 				await (ObjectDumper.WriteAsync(q));
 
 				var sql = sqlSpy.GetWholeLog();
-				Assert.That(sql, Does.Contain(useCrossJoin ? "cross join" : "inner join"));
 				Assert.That(GetTotalOccurrences(sql, "left outer join"), Is.EqualTo(0));
-				Assert.That(GetTotalOccurrences(sql, "inner join"), Is.EqualTo(useCrossJoin ? 1 : 2));
-				Assert.That(GetTotalOccurrences(sql, "cross join"), Is.EqualTo(useCrossJoin ? 1 : 0));
+				Assert.That(GetTotalOccurrences(sql, "inner join"), Is.EqualTo(2));
+				Assert.That(GetTotalOccurrences(sql, "cross join"), Is.EqualTo(0));
 			}
 		}
 
 		[Category("JOIN")]
 		[Test(Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
-		public void DLinqJoin5dLeftJoinAsync()
+		public async Task DLinqJoin5dLeftJoinAsync()
 		{
 			var q =
 				from c in db.Customers
 				join o in db.Orders on
-					new { c.CustomerId, HasContractTitle = c.ContactTitle != null } equals
-					new { o.Customer.CustomerId, HasContractTitle = o.Customer.ContactTitle != null } into orders
+					new {c.CustomerId, HasContractTitle = c.ContactTitle != null} equals
+					new {o.Customer.CustomerId, HasContractTitle = o.Customer.ContactTitle != null} into orders
 				from o in orders.DefaultIfEmpty()
-				select new { c.ContactName, o.OrderId };
+				select new {c.ContactName, OrderId = (int?) o.OrderId};
 
-			Assert.ThrowsAsync<NotSupportedException>(() => ObjectDumper.WriteAsync(q));
+			await (ObjectDumper.WriteAsync(q));
 		}
 
 		[Category("JOIN")]
diff --git a/src/NHibernate.Test/Linq/LinqQuerySamples.cs b/src/NHibernate.Test/Linq/LinqQuerySamples.cs
@@ -1622,35 +1622,24 @@ from c in db.Customers
 		}
 
 		[Category("JOIN")]
-		[TestCase(true, Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
-		[TestCase(false, Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
-		public void DLinqJoin5d(bool useCrossJoin)
+		[Test(Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
+		public void DLinqJoin5d()
 		{
-			if (useCrossJoin && !Dialect.SupportsCrossJoin)
-			{
-				Assert.Ignore("Dialect does not support cross join.");
-			}
-
 			var q =
 				from c in db.Customers
 				join o in db.Orders on
 					new {c.CustomerId, HasContractTitle = c.ContactTitle != null} equals
 					new {o.Customer.CustomerId, HasContractTitle = o.Customer.ContactTitle != null }
 				select new { c.ContactName, o.OrderId };
 
-			using (var substitute = SubstituteDialect())
 			using (var sqlSpy = new SqlLogSpy())
 			{
-				ClearQueryPlanCache();
-				substitute.Value.SupportsCrossJoin.Returns(useCrossJoin);
-
 				ObjectDumper.Write(q);
 
 				var sql = sqlSpy.GetWholeLog();
-				Assert.That(sql, Does.Contain(useCrossJoin ? "cross join" : "inner join"));
 				Assert.That(GetTotalOccurrences(sql, "left outer join"), Is.EqualTo(0));
-				Assert.That(GetTotalOccurrences(sql, "inner join"), Is.EqualTo(useCrossJoin ? 1 : 2));
-				Assert.That(GetTotalOccurrences(sql, "cross join"), Is.EqualTo(useCrossJoin ? 1 : 0));
+				Assert.That(GetTotalOccurrences(sql, "inner join"), Is.EqualTo(2));
+				Assert.That(GetTotalOccurrences(sql, "cross join"), Is.EqualTo(0));
 			}
 		}
 
@@ -1661,12 +1650,12 @@ public void DLinqJoin5dLeftJoin()
 			var q =
 				from c in db.Customers
 				join o in db.Orders on
-					new { c.CustomerId, HasContractTitle = c.ContactTitle != null } equals
-					new { o.Customer.CustomerId, HasContractTitle = o.Customer.ContactTitle != null } into orders
+					new {c.CustomerId, HasContractTitle = c.ContactTitle != null} equals
+					new {o.Customer.CustomerId, HasContractTitle = o.Customer.ContactTitle != null} into orders
 				from o in orders.DefaultIfEmpty()
-				select new { c.ContactName, o.OrderId };
+				select new {c.ContactName, OrderId = (int?) o.OrderId};
 
-			Assert.Throws<NotSupportedException>(() => ObjectDumper.Write(q));
+			ObjectDumper.Write(q);
 		}
 
 		[Category("JOIN")]
diff --git a/src/NHibernate/Linq/ReWriters/AddJoinsReWriter.cs b/src/NHibernate/Linq/ReWriters/AddJoinsReWriter.cs
@@ -1,6 +1,4 @@
 using System;
-using System.Collections.Generic;
-using System.Collections.Specialized;
 using System.Linq;
 using System.Linq.Expressions;
 using NHibernate.Engine;
@@ -22,13 +20,11 @@ public class AddJoinsReWriter : NhQueryModelVisitorBase, IIsEntityDecider, INhQu
 		private readonly ISessionFactoryImplementor _sessionFactory;
 		private readonly MemberExpressionJoinDetector _memberExpressionJoinDetector;
 		private readonly WhereJoinDetector _whereJoinDetector;
-		private JoinClause _currentJoin;
-		private bool? _innerJoin;
 
 		private AddJoinsReWriter(ISessionFactoryImplementor sessionFactory, QueryModel queryModel)
 		{
 			_sessionFactory = sessionFactory;
-			var joiner = new Joiner(queryModel, AddJoin);
+			var joiner = new Joiner(queryModel);
 			_memberExpressionJoinDetector = new MemberExpressionJoinDetector(this, joiner, _sessionFactory);
 			_whereJoinDetector = new WhereJoinDetector(this, joiner, _sessionFactory);
 		}
@@ -66,30 +62,17 @@ public override void VisitNhHavingClause(NhHavingClause havingClause, QueryModel
 
 		public void VisitNhOuterJoinClause(NhOuterJoinClause nhOuterJoinClause, QueryModel queryModel, int index)
 		{
-			VisitJoinClause(nhOuterJoinClause.JoinClause, false);
+			VisitJoinClause(nhOuterJoinClause.JoinClause);
 		}
 
 		public override void VisitJoinClause(JoinClause joinClause, QueryModel queryModel, int index)
 		{
-			VisitJoinClause(joinClause, true);
+			VisitJoinClause(joinClause);
 		}
 
-		private void VisitJoinClause(JoinClause joinClause, bool innerJoin)
+		private void VisitJoinClause(JoinClause joinClause)
 		{
 			joinClause.InnerSequence = _whereJoinDetector.Transform(joinClause.InnerSequence);
-
-			// When associations are located in the outer key (e.g. from a in A join b in B b on a.C.D.Id equals b.Id),
-			// do nothing and leave them to HQL for adding the missing joins.
-
-			// When associations are located in the inner key (e.g. from a in A join b in B b on a.Id equals b.C.D.Id),
-			// we have to move the condition to the where statement, otherwise the query will be invalid (HQL does not
-			// support them).
-			// Link newly created joins with the current join clause in order to later detect which join type to use.
-			_currentJoin = joinClause;
-			_innerJoin = innerJoin;
-			joinClause.InnerKeySelector = _whereJoinDetector.Transform(joinClause.InnerKeySelector);
-			_currentJoin = null;
-			_innerJoin = null;
 		}
 
 		// Since v5.3
@@ -107,18 +90,6 @@ public bool IsIdentifier(System.Type type, string propertyName)
 			return metadata != null && propertyName.Equals(metadata.IdentifierPropertyName);
 		}
 
-		private void AddJoin(QueryModel queryModel, NhJoinClause joinClause)
-		{
-			joinClause.ParentJoinClause = _currentJoin;
-			if (_innerJoin == true)
-			{
-				// Match the parent join type
-				joinClause.MakeInner();
-			}
-
-			queryModel.BodyClauses.Add(joinClause);
-		}
-
 		bool IIsEntityDecider.IsEntity(MemberExpression expression, out bool isIdentifier)
 		{
 			isIdentifier =
diff --git a/src/NHibernate/Linq/Visitors/JoinBuilder.cs b/src/NHibernate/Linq/Visitors/JoinBuilder.cs
@@ -21,12 +21,6 @@ public class Joiner : IJoiner
 		private readonly NameGenerator _nameGenerator;
 		private readonly QueryModel _queryModel;
 
-		internal Joiner(QueryModel queryModel, System.Action<QueryModel, NhJoinClause> addJoinMethod)
-			: this(queryModel)
-		{
-			AddJoinMethod = addJoinMethod;
-		}
-
 		internal Joiner(QueryModel queryModel)
 		{
 			_nameGenerator = new NameGenerator(queryModel);
