Don't join fkey table when linq comparison with composite id (#3629)

Author: jeff-hagen

.gitattributes

@@ -11,6 +11,7 @@
 *.cmd     text
 *.msbuild text
 *.md      text
+*.sql     text
 
 *.sln text eol=crlf
 *.csproj text eol=crlf

src/NHibernate.DomainModel/NHibernate.DomainModel.csproj

@@ -15,4 +15,7 @@
   <ItemGroup>
     <ProjectReference Include="..\NHibernate\NHibernate.csproj" />
   </ItemGroup>
+  <ItemGroup>
+    <PackageReference Include="Microsoft.Bcl.HashCode" Version="1.1.1" />
+  </ItemGroup>
 </Project>

src/NHibernate.DomainModel/Northwind/Entities/AnotherEntity.cs

@@ -4,6 +4,7 @@ public class AnotherEntity
     {
         public virtual int Id { get; set; }
         public virtual string Output { get; set; }
-		public virtual string Input { get; set; }
+        public virtual string Input { get; set; } 
+        public virtual CompositeIdEntity CompositeIdEntity { get; set; }
     }
-}
+}

src/NHibernate.DomainModel/Northwind/Entities/CompositeIdEntity.cs

@@ -0,0 +1,58 @@
+using System;
+
+namespace NHibernate.DomainModel.Northwind.Entities
+{
+	public class CompositeId : IComparable<CompositeId>
+	{
+		public int ObjectId { get; set; } 
+		public int TenantId { get; set; }
+
+		public CompositeId() { }
+		public CompositeId(int objectId, int tenantId)
+		{
+			ObjectId = objectId;
+			TenantId = tenantId;
+		}
+
+		public override string ToString() => ObjectId + "|" + TenantId;
+		protected bool Equals(CompositeId other) => ObjectId == other.ObjectId && TenantId == other.TenantId;
+		public static bool operator ==(CompositeId left, CompositeId right) => Equals(left, right);
+		public static bool operator !=(CompositeId left, CompositeId right) => !Equals(left, right);
+
+		public override bool Equals(object obj)
+		{
+			if (ReferenceEquals(null, obj) || obj.GetType() != this.GetType())
+			{
+				return false;
+			}
+			return ReferenceEquals(this, obj) || Equals((CompositeId)obj);
+		}
+
+		public override int GetHashCode() => HashCode.Combine(ObjectId, TenantId);
+
+		public int CompareTo(CompositeId other)
+		{
+			if (ReferenceEquals(this, other))
+			{
+				return 0;
+			}
+			else if (ReferenceEquals(other, null))
+			{
+				return 1;
+			}
+			
+			var idComparison = ObjectId.CompareTo(other.ObjectId);
+			if (idComparison != 0)
+			{
+				return idComparison;
+			}
+
+			return TenantId.CompareTo(other);
+		}
+	}
+    public class CompositeIdEntity
+    {
+        public virtual CompositeId Id { get; set; }
+        public virtual string Name { get; set; }
+    }
+}

src/NHibernate.DomainModel/Northwind/Entities/Northwind.cs

@@ -59,10 +59,10 @@ public IQueryable<Timesheet> Timesheets
             get { return _session.Query<Timesheet>(); }
         }
 
-    	public IQueryable<Animal> Animals
-    	{
-			get { return _session.Query<Animal>(); }
-    	}
+        public IQueryable<Animal> Animals
+        {
+            get { return _session.Query<Animal>(); }
+        }
 
         public IQueryable<Mammal> Mammals
         {
@@ -113,5 +113,10 @@ public IQueryable<IUser> IUsers
         {
             get { return _session.Query<IUser>(); }
         }
+        
+        public IQueryable<AnotherEntity> AnotherEntity
+        {
+            get { return _session.Query<AnotherEntity>(); }
+        }
     }
 }

src/NHibernate.DomainModel/Northwind/Mappings/AnotherEntity.hbm.xml

@@ -6,5 +6,9 @@
     </id>
     <property name="Output" />
     <property name="Input" />
+    <many-to-one name="CompositeIdEntity" fetch="select">
+      <column name="CompositeObjectId" />
+      <column name="CompositeTenantId" />
+    </many-to-one>
   </class>
 </hibernate-mapping>

src/NHibernate.DomainModel/Northwind/Mappings/CompositeIdEntity.hbm.xml

@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2" namespace="NHibernate.DomainModel.Northwind.Entities" assembly="NHibernate.DomainModel">
+  <class name="CompositeIdEntity" table="CompositeIdEntity">
+    <composite-id name="Id">
+      <key-property name="ObjectId" column="ObjectId" />
+      <key-property name="TenantId" column="TenantId" />
+    </composite-id>
+    <property name="Name" length="128" />
+  </class>
+</hibernate-mapping>

src/NHibernate.Test/Async/CompositeId/CompositeIdFixture.cs

@@ -18,6 +18,7 @@
 namespace NHibernate.Test.CompositeId
 {
 	using System.Threading.Tasks;
+	using System.Threading;
 	[TestFixture]
 	public class CompositeIdFixtureAsync : TestCase
 	{
@@ -33,7 +34,7 @@ protected override string[] Mappings
 				return new string[]
 				       	{
 				       		"CompositeId.Customer.hbm.xml", "CompositeId.Order.hbm.xml", "CompositeId.LineItem.hbm.xml",
-				       		"CompositeId.Product.hbm.xml"
+				       		"CompositeId.Product.hbm.xml", "CompositeId.Shipper.hbm.xml"
 				       	};
 			}
 		}
@@ -76,9 +77,13 @@ public async Task CompositeIdsAsync()
 
 				Order o = new Order(c);
 				o.OrderDate = DateTime.Today;
+				o.Shipper = new Shipper() { Id = new NullableId(null, 13) };
+				await (s.PersistAsync(o));
+				
 				LineItem li = new LineItem(o, p);
 				li.Quantity = 2;
-
+				await (s.PersistAsync(li));
+				
 				await (t.CommitAsync());
 			}
 
@@ -135,6 +140,19 @@ public async Task CompositeIdsAsync()
 				await (t.CommitAsync());
 			}
 
+			using (s = OpenSession())
+			{
+				t = s.BeginTransaction();
+				var noShippersForWarehouse = await (s.Query<Order>()
+					// NOTE: .Where(x => x.Shipper.Id == new NullableId(null, 13)) improperly renders
+					// "where (ShipperId = @p1 and WarehouseId = @p2)" with @p1 = NULL (needs to be is null)
+					// But the effort to fix is pretty high due to how component tuples are managed in linq / hql.
+					.Where(x => x.Shipper.Id.WarehouseId == 13 && x.Shipper.Id.Id == null)
+					.ToListAsync());
+				Assert.AreEqual(1, noShippersForWarehouse.Count);
+				await (t.CommitAsync());
+			}
+
 			using (s = OpenSession())
 			{
 				t = s.BeginTransaction();
@@ -303,5 +321,14 @@ public async Task AnyOnCompositeIdAsync()
 				await (s.Query<Order>().Select(o => o.LineItems.Any()).ToListAsync());
 			}
 		}
+
+		public async Task NullCompositeIdAsync(CancellationToken cancellationToken = default(CancellationToken))
+		{
+			using (var s = OpenSession())
+			{
+				await (s.Query<Order>().Where(o => o.LineItems.Any()).ToListAsync(cancellationToken));
+				await (s.Query<Order>().Select(o => o.LineItems.Any()).ToListAsync(cancellationToken));
+			}
+		}
 	}
 }

src/NHibernate.Test/Async/Linq/JoinTests.cs

@@ -297,6 +297,17 @@ public async Task OrderLinesWithSelectingCustomerNameInCaseShouldProduceTwoJoins
 				Assert.That(countJoins, Is.EqualTo(2));
 			}
 		}
+		
+		[Test]
+		public async Task ShouldConstipateJoinsWhenOnlyComparingCompositeIdPropertiesAsync()
+		{
+			using (var spy = new SqlLogSpy())
+			{
+				await (db.AnotherEntity.Where(x => x.CompositeIdEntity.Id.TenantId == 3).ToListAsync());
+				var countJoins = CountJoins(spy);
+				Assert.That(countJoins, Is.EqualTo(0));
+			}
+		}
 
 		private static int CountJoins(LogSpy sqlLog)
 		{

src/NHibernate.Test/Async/QueryTranslator/CustomQueryLoaderFixture.cs

@@ -42,7 +42,8 @@ internal sealed class CustomQueryLoaderFixtureAsync : TestCase
 				"Northwind.Mappings.TimeSheet.hbm.xml",
 				"Northwind.Mappings.Animal.hbm.xml",
 				"Northwind.Mappings.Patient.hbm.xml",
-				"Northwind.Mappings.NumericEntity.hbm.xml"
+				"Northwind.Mappings.NumericEntity.hbm.xml",
+				"Northwind.Mappings.CompositeIdEntity.hbm.xml"
 			};
 
 		protected override string MappingsAssembly => "NHibernate.DomainModel";