Use inner join instead of implicit join for implied entity joins (#2990)

bahusoid · web-flow · commit 6017fdc73e81 · 2022-07-21T02:50:30.000+02:00
diff --git a/.github/workflows/NetCoreTests.yml b/.github/workflows/NetCoreTests.yml
@@ -15,7 +15,7 @@ jobs:
           - DB: Firebird
             CONNECTION_STRING: "DataSource=localhost;Database=nhibernate;User=SYSDBA;Password=nhibernate;charset=utf8;"
           - DB: MySQL
-            CONNECTION_STRING: "Server=localhost;Uid=root;Password=nhibernate;Database=nhibernate;Old Guids=True;"
+            CONNECTION_STRING: "Server=localhost;Uid=root;Password=nhibernate;Database=nhibernate;Old Guids=True;SslMode=none;"
           - DB: Oracle
             CONNECTION_STRING: "User ID=nhibernate;Password=nhibernate;Metadata Pooling=false;Self Tuning=false;Data Source=(DESCRIPTION = (ADDRESS = (PROTOCOL = TCP)(HOST = localhost)(PORT = 1521)) (CONNECT_DATA = (SERVER = DEDICATED) (SERVICE_NAME = XEPDB1)))"
           - DB: SQLite
diff --git a/build-common/NHibernate.props b/build-common/NHibernate.props
@@ -6,6 +6,7 @@
     <VersionPatch  Condition="'$(VersionPatch)'  == ''">0</VersionPatch>
     <!-- Clear VersionSuffix for making release and set it to dev for making development builds -->
     <VersionSuffix Condition="'$(VersionSuffix)' == ''">dev</VersionSuffix>
+    <LangVersion Condition="'$(MSBuildProjectExtension)' != '.vbproj'">9.0</LangVersion>
 
     <VersionPrefix Condition="'$(VersionPrefix)' == ''">$(NhVersion).$(VersionPatch)</VersionPrefix>
     <VersionSuffix Condition="'$(VersionSuffix)' != '' AND '$(BuildNumber)' != ''">$(VersionSuffix).$(BuildNumber)</VersionSuffix>
diff --git a/src/NHibernate.DomainModel/Container.cs b/src/NHibernate.DomainModel/Container.cs
@@ -92,6 +92,7 @@ public Glarch Glarch
 		private IDictionary<string, Ternary> _ternaryMap;
 		//<set> mapping
 		private ISet<Ternary> _ternarySet;
+		private Simple _manyToOne;
 
 		public virtual IList<Simple> OneToMany
 		{
@@ -129,6 +130,12 @@ public virtual long Id
 			set { _id = value; }
 		}
 
+		public virtual Simple ManyToOne
+		{
+			get => _manyToOne;
+			set => _manyToOne = value;
+		}
+
 		public virtual IList<Contained> Bag
 		{
 			get { return _bag; }
diff --git a/src/NHibernate.DomainModel/Container.hbm.xml b/src/NHibernate.DomainModel/Container.hbm.xml
@@ -22,7 +22,7 @@
 		>
 			<generator class="native" />
 		</id>
-		
+		<many-to-one name="ManyToOne" lazy="proxy" />
 		<list 
 			name="OneToMany" 
 			lazy="true"
diff --git a/src/NHibernate.Test/Async/Legacy/ParentChildTest.cs b/src/NHibernate.Test/Async/Legacy/ParentChildTest.cs
@@ -359,8 +359,8 @@ public async Task CollectionQueryAsync()
 			if (!TestDialect.SupportsEmptyInsertsOrHasNonIdentityNativeGenerator)
 				Assert.Ignore("Support of empty inserts is required");
 
-			ISession s = OpenSession();
-			ITransaction t = s.BeginTransaction();
+			using var s = OpenSession();
+			using var t = s.BeginTransaction();
 
 			Simple s1 = new Simple();
 			s1.Name = "s";
@@ -383,10 +383,15 @@ public async Task CollectionQueryAsync()
 			l.Add(null);
 			l.Add(s2);
 			c.ManyToMany = l;
+			c.ManyToOne = new Simple { Name = "x", Count = 4};
+			await (s.SaveAsync(c.ManyToOne, c.ManyToOne.Count));
 			await (s.SaveAsync(c));
 
 			Assert.AreEqual(1,
 			                (await (s.CreateQuery("select c from c in class ContainerX, s in class Simple where c.OneToMany[2] = s").ListAsync
+			                	())).Count);			
+			Assert.AreEqual(1,
+			                (await (s.CreateQuery("select c from c in class ContainerX, s in class Simple where c.OneToMany[2] = s and c.ManyToOne.Name = 'x'").ListAsync
 			                	())).Count);
 			Assert.AreEqual(1,
 			                (await (s.CreateQuery("select c from c in class ContainerX, s in class Simple where c.ManyToMany[2] = s").
@@ -424,13 +429,13 @@ public async Task CollectionQueryAsync()
 			                	"select c from c in class ContainerX where c.ManyToMany[ c.OneToMany[0].Count ].Name = 's'").ListAsync())).
 			                	Count);
 
+			await (s.DeleteAsync(c.ManyToOne));
 			await (s.DeleteAsync(c));
 			await (s.DeleteAsync(s1));
 			await (s.DeleteAsync(s2));
 			await (s.DeleteAsync(s3));
 
 			await (t.CommitAsync());
-			s.Close();
 		}
 
 		[Test]
diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH1444/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH1444/Fixture.cs
@@ -37,9 +37,9 @@ public async Task BugAsync()
 					var message = ls.GetWholeLog();
 					var paramFormatter = (ISqlParameterFormatter)Sfi.ConnectionProvider.Driver;
 					Assert.That(message, Does.Contain(
-						"xchild0_.ParentId=xparent1_.Id and (" +
-						$"{paramFormatter.GetParameterName(0)}={Dialect.ToBooleanValueString(true)} or " +
-						$"xparent1_.A<{paramFormatter.GetParameterName(1)})"));
+						"on xchild0_.ParentId=xparent1_.Id").And.Contain(
+						$"where {paramFormatter.GetParameterName(0)}={Dialect.ToBooleanValueString(true)} or " +
+						$"xparent1_.A<{paramFormatter.GetParameterName(1)};"));
 				}
 			}
 		}
diff --git a/src/NHibernate.Test/Legacy/ParentChildTest.cs b/src/NHibernate.Test/Legacy/ParentChildTest.cs
@@ -348,8 +348,8 @@ public void CollectionQuery()
 			if (!TestDialect.SupportsEmptyInsertsOrHasNonIdentityNativeGenerator)
 				Assert.Ignore("Support of empty inserts is required");
 
-			ISession s = OpenSession();
-			ITransaction t = s.BeginTransaction();
+			using var s = OpenSession();
+			using var t = s.BeginTransaction();
 
 			Simple s1 = new Simple();
 			s1.Name = "s";
@@ -372,10 +372,15 @@ public void CollectionQuery()
 			l.Add(null);
 			l.Add(s2);
 			c.ManyToMany = l;
+			c.ManyToOne = new Simple { Name = "x", Count = 4};
+			s.Save(c.ManyToOne, c.ManyToOne.Count);
 			s.Save(c);
 
 			Assert.AreEqual(1,
 			                s.CreateQuery("select c from c in class ContainerX, s in class Simple where c.OneToMany[2] = s").List
+			                	().Count);			
+			Assert.AreEqual(1,
+			                s.CreateQuery("select c from c in class ContainerX, s in class Simple where c.OneToMany[2] = s and c.ManyToOne.Name = 'x'").List
 			                	().Count);
 			Assert.AreEqual(1,
 			                s.CreateQuery("select c from c in class ContainerX, s in class Simple where c.ManyToMany[2] = s").
@@ -413,13 +418,13 @@ public void CollectionQuery()
 			                	"select c from c in class ContainerX where c.ManyToMany[ c.OneToMany[0].Count ].Name = 's'").List().
 			                	Count);
 
+			s.Delete(c.ManyToOne);
 			s.Delete(c);
 			s.Delete(s1);
 			s.Delete(s2);
 			s.Delete(s3);
 
 			t.Commit();
-			s.Close();
 		}
 
 		[Test]
diff --git a/src/NHibernate.Test/NHSpecificTest/NH1444/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH1444/Fixture.cs
@@ -26,9 +26,9 @@ public void Bug()
 					var message = ls.GetWholeLog();
 					var paramFormatter = (ISqlParameterFormatter)Sfi.ConnectionProvider.Driver;
 					Assert.That(message, Does.Contain(
-						"xchild0_.ParentId=xparent1_.Id and (" +
-						$"{paramFormatter.GetParameterName(0)}={Dialect.ToBooleanValueString(true)} or " +
-						$"xparent1_.A<{paramFormatter.GetParameterName(1)})"));
+						"on xchild0_.ParentId=xparent1_.Id").And.Contain(
+						$"where {paramFormatter.GetParameterName(0)}={Dialect.ToBooleanValueString(true)} or " +
+						$"xparent1_.A<{paramFormatter.GetParameterName(1)};"));
 				}
 			}
 		}
diff --git a/src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.cs b/src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.cs
@@ -782,7 +782,12 @@ void CreateFromJoinElement(
 
 				if (fromElement.Parent == null)
 				{
+					// Most likely means association join is used in invalid context
+					// I.e. in subquery: from EntityA a where exists (from EntityB join a.Assocation)  
+					// Maybe we should throw exception instead
 					fromElement.FromClause.AddChild(fromElement);
+					if (fromElement.IsImplied)
+						fromElement.JoinSequence.SetUseThetaStyle(true);
 				}
 			}
 
diff --git a/src/NHibernate/Hql/Ast/ANTLR/SessionFactoryHelperExtensions.cs b/src/NHibernate/Hql/Ast/ANTLR/SessionFactoryHelperExtensions.cs
@@ -319,7 +319,6 @@ public JoinSequence CreateCollectionJoinSequence(IQueryableCollection collPersis
 		{
 			JoinSequence joinSequence = CreateJoinSequence();
 			joinSequence.SetRoot(collPersister, collectionName);
-			joinSequence.SetUseThetaStyle(true);		// TODO: figure out how this should be set.
 
 			///////////////////////////////////////////////////////////////////////////////
 			// This was the reason for failures regarding INDEX_OP and subclass joins on
diff --git a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs
@@ -516,10 +516,8 @@ private void DereferenceEntityJoin(string classAlias, EntityType propertyType, b
 
 			if ( ! useFoundFromElement )
 			{
-				// If this is an implied join in a from element, then use the impled join type which is part of the
-				// tree parser's state (set by the gramamar actions).
 				JoinSequence joinSequence = SessionFactoryHelper
-					.CreateJoinSequence(!forceLeftJoin && impliedJoin, propertyType, tableAlias, _joinType, joinColumns);
+					.CreateJoinSequence(false, propertyType, tableAlias, _joinType, joinColumns);
 
 				var factory = new FromElementFactory(
 						currentFromClause,
diff --git a/src/NHibernate/Hql/Ast/ANTLR/Tree/FromElement.cs b/src/NHibernate/Hql/Ast/ANTLR/Tree/FromElement.cs
@@ -590,13 +590,13 @@ public void SetOrigin(FromElement origin, bool manyToMany)
 				}
 				else
 				{
-					if (!Walker.IsInFrom && !Walker.IsInSelect)
+					if ((IsImplied && !JoinSequence.IsThetaStyle) || Walker.IsInFrom || Walker.IsInSelect)
 					{
-						FromClause.AddChild(this);
+						origin.AddChild(this);
 					}
 					else
 					{
-						origin.AddChild(this);
+						FromClause.AddChild(this);
 					}
 				}
 			}
diff --git a/src/NHibernate/Hql/Ast/ANTLR/Tree/FromElementFactory.cs b/src/NHibernate/Hql/Ast/ANTLR/Tree/FromElementFactory.cs
@@ -122,6 +122,10 @@ private FromElement CreateFromElementInSubselect(
 			
 			string tableAlias = correlatedSubselect ? fromElement.TableAlias : null;
 
+			//To properly generete subselect implicit join is required by SqlGenerator
+			if (fromElement.IsImplied)
+				fromElement.JoinSequence.SetUseThetaStyle(true);
+
 			// If the from element isn't in the same clause, create a new from element.
 			if (fromElement.FromClause != _fromClause)
 			{
@@ -189,18 +193,18 @@ public FromElement CreateCollection(IQueryableCollection queryableCollection,
 			if (elementType.IsEntityType)
 			{
 				// A collection of entities...
-				elem = CreateEntityAssociation(role, roleAlias, joinType);
+				elem = CreateEntityAssociation(role, roleAlias, joinType, indexed);
 			}
 			else if (elementType.IsComponentType)
 			{
 				// A collection of components...
-				JoinSequence joinSequence = CreateJoinSequence(roleAlias, joinType);
+				JoinSequence joinSequence = CreateJoinSequence(roleAlias, joinType, indexed);
 				elem = CreateCollectionJoin(joinSequence, roleAlias);
 			}
 			else
 			{
 				// A collection of scalar elements...
-				JoinSequence joinSequence = CreateJoinSequence(roleAlias, joinType);
+				JoinSequence joinSequence = CreateJoinSequence(roleAlias, joinType, indexed);
 				elem = CreateCollectionJoin(joinSequence, roleAlias);
 			}
 
@@ -332,7 +336,8 @@ public FromElement CreateEntityJoin(
 		private FromElement CreateEntityAssociation(
 						string role,
 						string roleAlias,
-						JoinType joinType)
+						JoinType joinType,
+						bool implicitJoin)
 		{
 			FromElement elem;
 			IQueryable entityPersister = (IQueryable)_queryableCollection.ElementPersister;
@@ -346,7 +351,7 @@ private FromElement CreateEntityAssociation(
 					Log.Debug("createEntityAssociation() : One to many - path = {0} role = {1} associatedEntityName = {2}", _path, role, associatedEntityName);
 				}
 
-				var joinSequence = CreateJoinSequence(roleAlias, joinType);
+				var joinSequence = CreateJoinSequence(roleAlias, joinType, implicitJoin);
 
 				elem = CreateJoin(associatedEntityName, roleAlias, joinSequence, (EntityType) _queryableCollection.ElementType, false);
 				elem.UseFromFragment |= elem.IsImplied && elem.Walker.IsSubQuery;
@@ -358,7 +363,7 @@ private FromElement CreateEntityAssociation(
 					Log.Debug("createManyToMany() : path = {0} role = {1} associatedEntityName = {2}", _path, role, associatedEntityName);
 				}
 
-				elem = CreateManyToMany(role, associatedEntityName, roleAlias, entityPersister, (EntityType)_queryableCollection.ElementType, joinType);
+				elem = CreateManyToMany(role, associatedEntityName, roleAlias, entityPersister, (EntityType)_queryableCollection.ElementType, joinType, implicitJoin);
 				_fromClause.Walker.AddQuerySpaces(_queryableCollection);
 			}
 			elem.CollectionTableAlias = roleAlias;
@@ -401,15 +406,16 @@ private FromElement CreateManyToMany(
 						string roleAlias,
 						IEntityPersister entityPersister,
 						EntityType type,
-						JoinType joinType)
+						JoinType joinType,
+						bool implicitJoin)
 		{
 			FromElement elem;
 			SessionFactoryHelperExtensions sfh = _fromClause.SessionFactoryHelper;
 
 			if (_inElementsFunction /*implied*/ )
 			{
 				// For implied many-to-many, just add the end join.
-				JoinSequence joinSequence = CreateJoinSequence(roleAlias, joinType);
+				JoinSequence joinSequence = CreateJoinSequence(roleAlias, joinType, implicitJoin);
 				elem = CreateJoin(associatedEntityName, roleAlias, joinSequence, type, true);
 			}
 			else
@@ -421,23 +427,24 @@ private FromElement CreateManyToMany(
 				string[] secondJoinColumns = sfh.GetCollectionElementColumns(role, roleAlias);
 
 				// Add the second join, the one that ends in the destination table.
-				JoinSequence joinSequence = CreateJoinSequence(roleAlias, joinType);
+				JoinSequence joinSequence = CreateJoinSequence(roleAlias, joinType, implicitJoin);
 				joinSequence.AddJoin(sfh.GetElementAssociationType(_collectionType), tableAlias, joinType, secondJoinColumns);
 				elem = CreateJoin(associatedEntityName, tableAlias, joinSequence, type, false);
 				elem.UseFromFragment = true;
 			}
 			return elem;
 		}
 
-		private JoinSequence CreateJoinSequence(string roleAlias, JoinType joinType)
+		//TODO: Investigate why not implicit indexed collection join is incorrectly generated and get rid of implicitJoin parameter (check IndexNode.Resolve)
+		private JoinSequence CreateJoinSequence(string roleAlias, JoinType joinType, bool implicitJoin)
 		{
 			SessionFactoryHelperExtensions sessionFactoryHelper = _fromClause.SessionFactoryHelper;
 			string[] joinColumns = Columns;
 			if (_collectionType == null)
 			{
 				throw new InvalidOperationException("collectionType is null!");
 			}
-			return sessionFactoryHelper.CreateJoinSequence(_implied, _collectionType, roleAlias, joinType, joinColumns);
+			return sessionFactoryHelper.CreateJoinSequence(implicitJoin, _collectionType, roleAlias, joinType, joinColumns);
 		}
 
 		private FromElement CreateJoin(
diff --git a/src/NHibernate/Hql/CollectionSubqueryFactory.cs b/src/NHibernate/Hql/CollectionSubqueryFactory.cs
@@ -3,7 +3,6 @@
 using System.Text;
 using NHibernate.Engine;
 using NHibernate.SqlCommand;
-using NHibernate.Util;
 
 namespace NHibernate.Hql
 {
@@ -16,6 +15,7 @@ public static string CreateCollectionSubquery(
 		{
 			try
 			{
+				joinSequence.SetUseThetaStyle(true);
 				JoinFragment join = joinSequence.ToJoinFragment(enabledFilters, true);
 				return new StringBuilder()
 					.Append("select ")
diff --git a/src/NHibernate/NHibernate.csproj b/src/NHibernate/NHibernate.csproj
@@ -12,7 +12,6 @@
 
     <PackageDescription>NHibernate is a mature, open source object-relational mapper for the .NET framework. It is actively developed, fully featured and used in thousands of successful projects.</PackageDescription>
     <PackageTags>ORM; O/RM; DataBase; DAL; ObjectRelationalMapping; NHibernate; ADO.Net; Core</PackageTags>
-    <LangVersion>7.3</LangVersion>
   </PropertyGroup>
   
   <ItemGroup>

Original file line number	Diff line number	Diff line change
`@@ -92,6 +92,7 @@ public Glarch Glarch`
`92`	`92`	`private IDictionary<string, Ternary> _ternaryMap;`
`93`	`93`	`//<set> mapping`
`94`	`94`	`private ISet<Ternary> _ternarySet;`
	`95`	`+ private Simple _manyToOne;`
`95`	`96`
`96`	`97`	`public virtual IList<Simple> OneToMany`
`97`	`98`	`{`
`@@ -129,6 +130,12 @@ public virtual long Id`
`129`	`130`	`set { _id = value; }`
`130`	`131`	`}`
`131`	`132`
	`133`	`+ public virtual Simple ManyToOne`
	`134`	`+ {`
	`135`	`+ get => _manyToOne;`
	`136`	`+ set => _manyToOne = value;`
	`137`	`+ }`
	`138`	`+`
`132`	`139`	`public virtual IList<Contained> Bag`
`133`	`140`	`{`
`134`	`141`	`get { return _bag; }`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@`
`22`	`22`	`>`
`23`	`23`	`<generator class="native" />`
`24`	`24`	`</id>`
`25`		`-`
	`25`	`+ <many-to-one name="ManyToOne" lazy="proxy" />`
`26`	`26`	`<list`
`27`	`27`	`name="OneToMany"`
`28`	`28`	`lazy="true"`
Original file line number	Diff line number	Diff line change
`@@ -37,9 +37,9 @@ public async Task BugAsync()`
`37`	`37`	`var message = ls.GetWholeLog();`
`38`	`38`	`var paramFormatter = (ISqlParameterFormatter)Sfi.ConnectionProvider.Driver;`
`39`	`39`	`Assert.That(message, Does.Contain(`
`40`		`- "xchild0_.ParentId=xparent1_.Id and (" +`
`41`		`- $"{paramFormatter.GetParameterName(0)}={Dialect.ToBooleanValueString(true)} or " +`
`42`		`- $"xparent1_.A<{paramFormatter.GetParameterName(1)})"));`
	`40`	`+ "on xchild0_.ParentId=xparent1_.Id").And.Contain(`
	`41`	`+ $"where {paramFormatter.GetParameterName(0)}={Dialect.ToBooleanValueString(true)} or " +`
	`42`	`+ $"xparent1_.A<{paramFormatter.GetParameterName(1)};"));`
`43`	`43`	`}`
`44`	`44`	`}`
`45`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,9 +26,9 @@ public void Bug()`
`26`	`26`	`var message = ls.GetWholeLog();`
`27`	`27`	`var paramFormatter = (ISqlParameterFormatter)Sfi.ConnectionProvider.Driver;`
`28`	`28`	`Assert.That(message, Does.Contain(`
`29`		`- "xchild0_.ParentId=xparent1_.Id and (" +`
`30`		`- $"{paramFormatter.GetParameterName(0)}={Dialect.ToBooleanValueString(true)} or " +`
`31`		`- $"xparent1_.A<{paramFormatter.GetParameterName(1)})"));`
	`29`	`+ "on xchild0_.ParentId=xparent1_.Id").And.Contain(`
	`30`	`+ $"where {paramFormatter.GetParameterName(0)}={Dialect.ToBooleanValueString(true)} or " +`
	`31`	`+ $"xparent1_.A<{paramFormatter.GetParameterName(1)};"));`
`32`	`32`	`}`
`33`	`33`	`}`
`34`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -782,7 +782,12 @@ void CreateFromJoinElement(`
`782`	`782`
`783`	`783`	`if (fromElement.Parent == null)`
`784`	`784`	`{`
	`785`	`+ // Most likely means association join is used in invalid context`
	`786`	`+ // I.e. in subquery: from EntityA a where exists (from EntityB join a.Assocation)`
	`787`	`+ // Maybe we should throw exception instead`
`785`	`788`	`fromElement.FromClause.AddChild(fromElement);`
	`789`	`+ if (fromElement.IsImplied)`
	`790`	`+ fromElement.JoinSequence.SetUseThetaStyle(true);`
`786`	`791`	`}`
`787`	`792`	`}`
`788`	`793`
Original file line number	Diff line number	Diff line change
`@@ -319,7 +319,6 @@ public JoinSequence CreateCollectionJoinSequence(IQueryableCollection collPersis`
`319`	`319`	`{`
`320`	`320`	`JoinSequence joinSequence = CreateJoinSequence();`
`321`	`321`	`joinSequence.SetRoot(collPersister, collectionName);`
`322`		`- joinSequence.SetUseThetaStyle(true); // TODO: figure out how this should be set.`
`323`	`322`
`324`	`323`	`///////////////////////////////////////////////////////////////////////////////`
`325`	`324`	`// This was the reason for failures regarding INDEX_OP and subclass joins on`