Skip to content

Commit 00b36c0

Browse files
kfedorchenkofredericDelaporte
kfedorchenko
authored and
fredericDelaporte
committed
Join-fetching with property-ref results in n+1 select
 * Fixes #1226 (NH-2534)
1 parent 8ffa874 commit 00b36c0

File tree

6 files changed

+243
-0
lines changed

6 files changed

+243
-0
lines changed

‎src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
//------------------------------------------------------------------------------
2+
// <auto-generated>
3+
// This code was generated by AsyncGenerator.
4+
//
5+
// Changes to this file may cause incorrect behavior and will be lost if
6+
// the code is regenerated.
7+
// </auto-generated>
8+
//------------------------------------------------------------------------------
9+
10+
11+
using System;
12+
using System.Collections.Generic;
13+
using System.Linq;
14+
using NUnit.Framework;
15+
16+
namespace NHibernate.Test.NHSpecificTest.GH1226
17+
{
18+
using System.Threading.Tasks;
19+
[TestFixture]
20+
public class FixtureAsync : BugTestCase
21+
{
22+
protected override void OnSetUp()
23+
{
24+
base.OnSetUp();
25+
26+
using (var session = OpenSession())
27+
{
28+
using (var tx = session.BeginTransaction())
29+
{
30+
var bank = new Bank { Code = "01234" };
31+
session.Save(bank);
32+
33+
var account = new Account { Bank = bank };
34+
session.Save(account);
35+
36+
var account2 = new Account { Bank = bank };
37+
session.Save(account2);
38+
39+
tx.Commit();
40+
}
41+
}
42+
}
43+
44+
[Test]
45+
public async Task BankShouldBeJoinFetchedAsync()
46+
{
47+
using (var session = OpenSession())
48+
{
49+
var wasStatisticsEnabled = session.SessionFactory.Statistics.IsStatisticsEnabled;
50+
session.SessionFactory.Statistics.IsStatisticsEnabled = true;
51+
52+
long statementCount;
53+
54+
using (var tx = session.BeginTransaction())
55+
{
56+
// Bug only occurs if the Banks are already in the session cache.
57+
var preloadedBanks = await (session.CreateQuery("from Bank").ListAsync<Bank>());
58+
59+
var countBeforeQuery = session.SessionFactory.Statistics.PrepareStatementCount;
60+
61+
Console.WriteLine("Query: -------------------------------------------------------");
62+
63+
var accounts = await (session.CreateQuery("from Account a left join fetch a.Bank").ListAsync<Account>());
64+
IList<Bank> associatedBanks = accounts.Select(x => x.Bank).ToList();
65+
66+
var countAfterQuery = session.SessionFactory.Statistics.PrepareStatementCount;
67+
statementCount = countAfterQuery - countBeforeQuery;
68+
69+
Console.WriteLine("End ----------------------------------------------------------");
70+
71+
await (tx.CommitAsync());
72+
}
73+
74+
session.SessionFactory.Statistics.IsStatisticsEnabled = wasStatisticsEnabled;
75+
76+
Assert.That(statementCount, Is.EqualTo(1));
77+
}
78+
}
79+
80+
protected override void OnTearDown()
81+
{
82+
base.OnTearDown();
83+
84+
using (var session = OpenSession())
85+
{
86+
using (var tx = session.BeginTransaction())
87+
{
88+
session.Delete("from Account");
89+
session.Delete("from Bank");
90+
tx.Commit();
91+
}
92+
}
93+
}
94+
}
95+
}

‎src/NHibernate.Test/NHSpecificTest/GH1226/Fixture.cs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using NUnit.Framework;
5+
6+
namespace NHibernate.Test.NHSpecificTest.GH1226
7+
{
8+
[TestFixture]
9+
public class Fixture : BugTestCase
10+
{
11+
protected override void OnSetUp()
12+
{
13+
base.OnSetUp();
14+
15+
using (var session = OpenSession())
16+
{
17+
using (var tx = session.BeginTransaction())
18+
{
19+
var bank = new Bank { Code = "01234" };
20+
session.Save(bank);
21+
22+
var account = new Account { Bank = bank };
23+
session.Save(account);
24+
25+
var account2 = new Account { Bank = bank };
26+
session.Save(account2);
27+
28+
tx.Commit();
29+
}
30+
}
31+
}
32+
33+
[Test]
34+
public void BankShouldBeJoinFetched()
35+
{
36+
using (var session = OpenSession())
37+
{
38+
var wasStatisticsEnabled = session.SessionFactory.Statistics.IsStatisticsEnabled;
39+
session.SessionFactory.Statistics.IsStatisticsEnabled = true;
40+
41+
long statementCount;
42+
43+
using (var tx = session.BeginTransaction())
44+
{
45+
// Bug only occurs if the Banks are already in the session cache.
46+
var preloadedBanks = session.CreateQuery("from Bank").List<Bank>();
47+
48+
var countBeforeQuery = session.SessionFactory.Statistics.PrepareStatementCount;
49+
50+
Console.WriteLine("Query: -------------------------------------------------------");
51+
52+
var accounts = session.CreateQuery("from Account a left join fetch a.Bank").List<Account>();
53+
IList<Bank> associatedBanks = accounts.Select(x => x.Bank).ToList();
54+
55+
var countAfterQuery = session.SessionFactory.Statistics.PrepareStatementCount;
56+
statementCount = countAfterQuery - countBeforeQuery;
57+
58+
Console.WriteLine("End ----------------------------------------------------------");
59+
60+
tx.Commit();
61+
}
62+
63+
session.SessionFactory.Statistics.IsStatisticsEnabled = wasStatisticsEnabled;
64+
65+
Assert.That(statementCount, Is.EqualTo(1));
66+
}
67+
}
68+
69+
protected override void OnTearDown()
70+
{
71+
base.OnTearDown();
72+
73+
using (var session = OpenSession())
74+
{
75+
using (var tx = session.BeginTransaction())
76+
{
77+
session.Delete("from Account");
78+
session.Delete("from Bank");
79+
tx.Commit();
80+
}
81+
}
82+
}
83+
}
84+
}

‎src/NHibernate.Test/NHSpecificTest/GH1226/Mappings.hbm.xml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2"
3+
assembly="NHibernate.Test"
4+
namespace="NHibernate.Test.NHSpecificTest.GH1226">
5+
<class name="Account">
6+
<id name="Id">
7+
<generator class="guid"/>
8+
</id>
9+
<many-to-one fetch="join" name="Bank" property-ref="Code"/>
10+
</class>
11+
12+
<class name="Bank">
13+
<id name="Id">
14+
<generator class="guid"/>
15+
</id>
16+
<property name="Code" unique="true"/>
17+
</class>
18+
</hibernate-mapping>

‎src/NHibernate.Test/NHSpecificTest/GH1226/Model.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
using System;
2+
3+
namespace NHibernate.Test.NHSpecificTest.GH1226
4+
{
5+
public class Account
6+
{
7+
public virtual Guid Id { get; set; }
8+
public virtual Bank Bank { get; set; }
9+
}
10+
11+
public class Bank
12+
{
13+
public virtual Guid Id { get; set; }
14+
public virtual string Code { get; set; }
15+
}
16+
}

‎src/NHibernate/Async/Loader/Loader.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,8 @@ private async Task InstanceAlreadyLoadedAsync(DbDataReader rs, int i, IEntityPer
650650
entry.LockMode = lockMode;
651651
}
652652
}
653+
654+
CacheByUniqueKey(i, persister, obj, session);
653655
}
654656

655657
/// <summary>

‎src/NHibernate/Loader/Loader.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -972,6 +972,34 @@ private void InstanceAlreadyLoaded(DbDataReader rs, int i, IEntityPersister pers
972972
entry.LockMode = lockMode;
973973
}
974974
}
975+
976+
CacheByUniqueKey(i, persister, obj, session);
977+
}
978+
979+
private void CacheByUniqueKey(int i, IEntityPersister persister, object obj, ISessionImplementor session)
980+
{
981+
// #1226: If it is already loaded and can be loaded from an association with a property ref, make
982+
// sure it is also cached by its unique key.
983+
var ukName = OwnerAssociationTypes?[i]?.RHSUniqueKeyPropertyName;
984+
if (ukName == null)
985+
return;
986+
var index = ((IUniqueKeyLoadable)persister).GetPropertyIndex(ukName);
987+
var ukValue = persister.GetPropertyValue(obj, index);
988+
// ukValue can be null for two reasons:
989+
// - Entity currently loading and not yet fully hydrated. In such case, it has already been handled by
990+
// InstanceNotYetLoaded on a previous row, there is nothing more to do. This case could also be
991+
// detected with "session.PersistenceContext.GetEntry(obj).Status == Status.Loading", but since there
992+
// is a second case, just test for ukValue null.
993+
// - Entity association is unset in session but not yet persisted, autoflush disabled: ignore. We are
994+
// already in an error case: querying entities changed in session without flushing them before querying.
995+
// So here it gets loaded as if it were still associated, but we do not have the key anymore in session:
996+
// we cannot cache it, so long for the additionnal round-trip this will cause. (Do not fallback on
997+
// reading the key in rs, this is stale data in regard to the session state.)
998+
if (ukValue == null)
999+
return;
1000+
var type = persister.PropertyTypes[index];
1001+
var euk = new EntityUniqueKey(persister.EntityName, ukName, ukValue, type, session.Factory);
1002+
session.PersistenceContext.AddEntity(euk, obj);
9751003
}
9761004

9771005
/// <summary>

0 commit comments

Comments
 (0)