Skip to content

Commit 008760c

Browse files
bahusoidbahusoid
authored
Fix missing joins in subquery in case joins are present in main query (#2494)
Fixes #1326
1 parent 2d3649e commit 008760c

File tree

4 files changed

+222
-7
lines changed

4 files changed

+222
-7
lines changed

‎src/NHibernate.Test/Async/NHSpecificTest/NH3622/FixtureByCode.cs

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
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.Linq;
13+
using System.Text.RegularExpressions;
14+
using NHibernate.Cfg.MappingSchema;
15+
using NHibernate.Mapping.ByCode;
16+
using NUnit.Framework;
17+
using NHibernate.Linq;
18+
19+
namespace NHibernate.Test.NHSpecificTest.NH3622
20+
{
21+
using System.Threading.Tasks;
22+
/// <summary>
23+
/// Fixture using 'by code' mappings
24+
/// </summary>
25+
[TestFixture]
26+
public class ByCodeFixtureAsync : TestCaseMappingByCode
27+
{
28+
protected override HbmMapping GetMappings()
29+
{
30+
var mapper = new ModelMapper();
31+
mapper.AddMapping<TagMap>();
32+
mapper.AddMapping<EquipmentMap>();
33+
mapper.AddMapping<DisciplineMap>();
34+
mapper.AddMapping<DisciplineTypeMap>();
35+
return mapper.CompileMappingForAllExplicitlyAddedEntities();
36+
}
37+
38+
[Test]
39+
public async Task MissingJoinsInSubqueryAsync()
40+
{
41+
var id = Guid.NewGuid();
42+
using( var logSpy = new SqlLogSpy())
43+
using (var s = OpenSession())
44+
{
45+
var x = s.Query<Tag>()
46+
.Where(tag => tag.Equipment.Discipline.DisciplineType.Id == id)
47+
.Select(tag => tag.Id);
48+
49+
var y = await (s.Query<Tag>()
50+
.Where(tag => x.Contains(tag.Id))
51+
.Fetch(tag => tag.Equipment)
52+
.ThenFetch(equipment => equipment.Discipline)
53+
.ThenFetch(discipline => discipline.DisciplineType)
54+
.ToListAsync());
55+
56+
var sql = logSpy.GetWholeLog();
57+
var findSubqyeryIndex = sql.IndexOf(" in (");
58+
var capturesCount = Regex.Matches(sql.Substring(findSubqyeryIndex), "inner join").Count;
59+
//Expected joins for tag.Equipment.Discipline in subquery
60+
Assert.That(capturesCount, Is.EqualTo(2), "Missing inner joins in subquery: " + sql);
61+
}
62+
}
63+
}
64+
}

‎src/NHibernate.Test/NHSpecificTest/NH3622/Entity.cs

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
using System;
2+
using NHibernate.Mapping.ByCode;
3+
using NHibernate.Mapping.ByCode.Conformist;
4+
5+
namespace NHibernate.Test.NHSpecificTest.NH3622
6+
{
7+
public class Tag
8+
{
9+
public virtual Guid Id { get; set; }
10+
public virtual string Name { get; set; }
11+
public virtual Equipment Equipment { get; set; }
12+
}
13+
14+
public class Equipment
15+
{
16+
public virtual Guid Id { get; set; }
17+
public virtual string Name { get; set; }
18+
public virtual Discipline Discipline { get; set; }
19+
}
20+
21+
public class Discipline
22+
{
23+
public virtual Guid Id { get; set; }
24+
public virtual string Name { get; set; }
25+
public virtual DisciplineType DisciplineType { get; set; }
26+
}
27+
28+
public class DisciplineType
29+
{
30+
public virtual Guid Id { get; set; }
31+
public virtual string Name { get; set; }
32+
}
33+
34+
public class TagMap : ClassMapping<Tag>
35+
{
36+
public TagMap()
37+
{
38+
this.Id(x => x.Id, mapper => mapper.Generator(Generators.Assigned));
39+
this.Property(x => x.Name, mapper => mapper.NotNullable(true));
40+
this.ManyToOne(
41+
x => x.Equipment,
42+
mapper =>
43+
{
44+
mapper.NotNullable(true);
45+
mapper.Column("EquipmentId");
46+
});
47+
}
48+
}
49+
50+
public class EquipmentMap : ClassMapping<Equipment>
51+
{
52+
public EquipmentMap()
53+
{
54+
this.Id(x => x.Id, mapper => mapper.Generator(Generators.Assigned));
55+
this.Property(x => x.Name, mapper => mapper.NotNullable(true));
56+
this.ManyToOne(
57+
x => x.Discipline,
58+
mapper =>
59+
{
60+
mapper.Column("DisciplineId");
61+
mapper.NotNullable(false);
62+
});
63+
}
64+
}
65+
66+
public class DisciplineMap : ClassMapping<Discipline>
67+
{
68+
public DisciplineMap()
69+
{
70+
this.Id(x => x.Id, mapper => mapper.Generator(Generators.Assigned));
71+
this.Property(x => x.Name, mapper => mapper.NotNullable(true));
72+
this.ManyToOne(
73+
x => x.DisciplineType,
74+
mapper =>
75+
{
76+
mapper.Column("DisciplineTypeId");
77+
mapper.NotNullable(false);
78+
});
79+
}
80+
}
81+
82+
public class DisciplineTypeMap : ClassMapping<DisciplineType>
83+
{
84+
public DisciplineTypeMap()
85+
{
86+
this.Id(x => x.Id, mapper => mapper.Generator(Generators.Assigned));
87+
this.Property(x => x.Name, mapper => mapper.NotNullable(true));
88+
}
89+
}
90+
}

‎src/NHibernate.Test/NHSpecificTest/NH3622/FixtureByCode.cs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
using System;
2+
using System.Linq;
3+
using System.Text.RegularExpressions;
4+
using NHibernate.Cfg.MappingSchema;
5+
using NHibernate.Mapping.ByCode;
6+
using NUnit.Framework;
7+
using NHibernate.Linq;
8+
9+
namespace NHibernate.Test.NHSpecificTest.NH3622
10+
{
11+
/// <summary>
12+
/// Fixture using 'by code' mappings
13+
/// </summary>
14+
[TestFixture]
15+
public class ByCodeFixture : TestCaseMappingByCode
16+
{
17+
protected override HbmMapping GetMappings()
18+
{
19+
var mapper = new ModelMapper();
20+
mapper.AddMapping<TagMap>();
21+
mapper.AddMapping<EquipmentMap>();
22+
mapper.AddMapping<DisciplineMap>();
23+
mapper.AddMapping<DisciplineTypeMap>();
24+
return mapper.CompileMappingForAllExplicitlyAddedEntities();
25+
}
26+
27+
[Test]
28+
public void MissingJoinsInSubquery()
29+
{
30+
var id = Guid.NewGuid();
31+
using( var logSpy = new SqlLogSpy())
32+
using (var s = OpenSession())
33+
{
34+
var x = s.Query<Tag>()
35+
.Where(tag => tag.Equipment.Discipline.DisciplineType.Id == id)
36+
.Select(tag => tag.Id);
37+
38+
var y = s.Query<Tag>()
39+
.Where(tag => x.Contains(tag.Id))
40+
.Fetch(tag => tag.Equipment)
41+
.ThenFetch(equipment => equipment.Discipline)
42+
.ThenFetch(discipline => discipline.DisciplineType)
43+
.ToList();
44+
45+
var sql = logSpy.GetWholeLog();
46+
var findSubqyeryIndex = sql.IndexOf(" in (");
47+
var capturesCount = Regex.Matches(sql.Substring(findSubqyeryIndex), "inner join").Count;
48+
//Expected joins for tag.Equipment.Discipline in subquery
49+
Assert.That(capturesCount, Is.EqualTo(2), "Missing inner joins in subquery: " + sql);
50+
}
51+
}
52+
}
53+
}

‎src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -504,13 +504,8 @@ private void DereferenceEntityJoin(string classAlias, EntityType propertyType, b
504504
//
505505
///////////////////////////////////////////////////////////////////////////////
506506

507-
bool found = elem != null;
508-
// even though we might find a pre-existing element by join path, for FromElements originating in a from-clause
509-
// we should only ever use the found element if the aliases match (null != null here).
510-
// Implied joins are ok to reuse only if in same from clause (are there any other cases when we should reject implied joins?).
511-
bool useFoundFromElement = found &&
512-
(elem.IsImplied && elem.FromClause == currentFromClause || // NH different behavior (NH-3002)
513-
AreSame(classAlias, elem.ClassAlias));
507+
// even though we might find a pre-existing element by join path, we may not be able to reuse it...
508+
bool useFoundFromElement = elem != null && CanReuse(classAlias, elem);
514509

515510
if ( ! useFoundFromElement )
516511
{
@@ -551,6 +546,19 @@ private bool AreSame(String alias1, String alias2) {
551546
return !StringHelper.IsEmpty( alias1 ) && !StringHelper.IsEmpty( alias2 ) && alias1.Equals( alias2 );
552547
}
553548

549+
private bool CanReuse(string classAlias, FromElement fromElement)
550+
{
551+
// if the from-clauses are the same, we can be a little more aggressive in terms of what we reuse
552+
if (fromElement.FromClause == Walker.CurrentFromClause &&
553+
AreSame(classAlias, fromElement.ClassAlias))
554+
{
555+
return true;
556+
}
557+
558+
// otherwise (subquery case) don't reuse the fromElement if we are processing the from-clause of the subquery
559+
return Walker.CurrentClauseType != HqlSqlWalker.FROM;
560+
}
561+
554562
private void SetImpliedJoin(FromElement elem)
555563
{
556564
_impliedJoin = elem;

0 commit comments

Comments
 (0)