Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
*.cmd text
*.msbuild text
*.md text
*.sql text

*.sln text eol=crlf
*.csproj text eol=crlf
Expand Down
3 changes: 3 additions & 0 deletions src/NHibernate.DomainModel/NHibernate.DomainModel.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,7 @@
<ItemGroup>
<ProjectReference Include="..\NHibernate\NHibernate.csproj" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Bcl.HashCode" Version="1.1.1" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
}
}
}
58 changes: 58 additions & 0 deletions src/NHibernate.DomainModel/Northwind/Entities/CompositeIdEntity.cs
Original file line number Diff line number Diff line change
@@ -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; }
}
}
13 changes: 9 additions & 4 deletions src/NHibernate.DomainModel/Northwind/Entities/Northwind.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -113,5 +113,10 @@ public IQueryable<IUser> IUsers
{
get { return _session.Query<IUser>(); }
}

public IQueryable<AnotherEntity> AnotherEntity
{
get { return _session.Query<AnotherEntity>(); }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Original file line number Diff line number Diff line change
@@ -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>
31 changes: 29 additions & 2 deletions src/NHibernate.Test/Async/CompositeId/CompositeIdFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
namespace NHibernate.Test.CompositeId
{
using System.Threading.Tasks;
using System.Threading;
[TestFixture]
public class CompositeIdFixtureAsync : TestCase
{
Expand All @@ -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"
};
}
}
Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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));
}
}
}
}
11 changes: 11 additions & 0 deletions src/NHibernate.Test/Async/Linq/JoinTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
30 changes: 28 additions & 2 deletions src/NHibernate.Test/CompositeId/CompositeIdFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,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"
};
}
}
Expand Down Expand Up @@ -64,9 +64,13 @@ public void CompositeIds()

Order o = new Order(c);
o.OrderDate = DateTime.Today;
o.Shipper = new Shipper() { Id = new NullableId(null, 13) };
s.Persist(o);

LineItem li = new LineItem(o, p);
li.Quantity = 2;

s.Persist(li);

t.Commit();
}

Expand Down Expand Up @@ -123,6 +127,19 @@ public void CompositeIds()
t.Commit();
}

using (s = OpenSession())
{
t = s.BeginTransaction();
var noShippersForWarehouse = 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)
.ToList();
Assert.AreEqual(1, noShippersForWarehouse.Count);
t.Commit();
}

using (s = OpenSession())
{
t = s.BeginTransaction();
Expand Down Expand Up @@ -291,5 +308,14 @@ public void AnyOnCompositeId()
s.Query<Order>().Select(o => o.LineItems.Any()).ToList();
}
}

public void NullCompositeId()
{
using (var s = OpenSession())
{
s.Query<Order>().Where(o => o.LineItems.Any()).ToList();
s.Query<Order>().Select(o => o.LineItems.Any()).ToList();
}
}
}
}
58 changes: 58 additions & 0 deletions src/NHibernate.Test/CompositeId/NullableId.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
using System;

namespace NHibernate.Test.CompositeId
{
public class NullableId : IComparable<NullableId>
{
public int? Id { get; set; }
public int WarehouseId { get; set; }

public NullableId() { }
public NullableId(int? id, int warehouseId)
{
Id = id;
WarehouseId = warehouseId;
}

public override string ToString() => Id + "|" + WarehouseId;
protected bool Equals(NullableId other) => Id == other.Id && WarehouseId == other.WarehouseId;
public static bool operator ==(NullableId left, NullableId right) => Equals(left, right);
public static bool operator !=(NullableId left, NullableId 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((NullableId)obj);
}

public override int GetHashCode() => HashCode.Combine(Id, WarehouseId);

public int CompareTo(NullableId other)
{
if (ReferenceEquals(this, other))
{
return 0;
}
else if (ReferenceEquals(other, null) || !other.Id.HasValue)
{
return 1;
}
else if (!Id.HasValue)
{
return -1;
}

var idComparison = Id.Value.CompareTo(other.Id);
if (idComparison != 0)
{
return idComparison;
}

return WarehouseId.CompareTo(other.WarehouseId);
}
}
}
9 changes: 8 additions & 1 deletion src/NHibernate.Test/CompositeId/Order.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public override int GetHashCode()
private Customer customer;
private IList<LineItem> lineItems = new List<LineItem>();
private decimal total;
private Shipper shipper;

public Order() {}
public Order(Customer customer)
Expand Down Expand Up @@ -87,6 +88,12 @@ public virtual decimal Total
get { return total; }
set { total = value; }
}

public virtual Shipper Shipper
{
get { return shipper; }
set { shipper = value; }
}

public virtual LineItem GenerateLineItem(Product product, int quantity)
{
Expand All @@ -96,4 +103,4 @@ public virtual LineItem GenerateLineItem(Product product, int quantity)
return li;
}
}
}
}
5 changes: 5 additions & 0 deletions src/NHibernate.Test/CompositeId/Order.hbm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
insert="false"
update="false"
not-null="true"/>

<many-to-one name="Shipper" fetch="select">
<column name="ShipperId" not-null="false"/>
<column name="WarehouseId" />
</many-to-one>

<bag name="LineItems"
fetch="join"
Expand Down
8 changes: 8 additions & 0 deletions src/NHibernate.Test/CompositeId/Shipper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
namespace NHibernate.Test.CompositeId
{
public class Shipper
{
public virtual NullableId Id { get; set; }
public virtual string Name { get; set; }
}
}
Loading