From 59b07b8227845cb0c091e811e58b23d15261adf0 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Thu, 3 Sep 2020 12:42:51 +0300 Subject: [PATCH 1/3] Fix wrong inner join on component key many-to-one property in Criteria --- .../NHSpecificTest/NH3813/FixtureByCode.cs | 193 ++++++++++++++++ .../NHSpecificTest/NH3813/FixtureByCode.cs | 218 ++++++++++++++++++ .../Loader/Criteria/CriteriaJoinWalker.cs | 6 +- 3 files changed, 414 insertions(+), 3 deletions(-) create mode 100644 src/NHibernate.Test/Async/NHSpecificTest/NH3813/FixtureByCode.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/NH3813/FixtureByCode.cs diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH3813/FixtureByCode.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH3813/FixtureByCode.cs new file mode 100644 index 00000000000..79e8cef94df --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH3813/FixtureByCode.cs @@ -0,0 +1,193 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by AsyncGenerator. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + + +using System.Collections.Generic; +using System.Linq; +using NHibernate.Mapping.ByCode; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH3813 +{ + using System.Threading.Tasks; + [TestFixture] + public class KeyManyToOneInnerJoinFetchFixtureAsync : TestCaseMappingByCode + { + protected override Cfg.MappingSchema.HbmMapping GetMappings() + { + var mapper = new ModelMapper(); + + mapper.Class( + m => + { + m.Lazy(false); + + m.Id( + i => i.ID, + id => + { + id.Column("ID"); + id.Generator(Generators.Identity); + }); + + m.Bag( + b => b.AssociationTableCollection, + bm => + { + bm.Inverse(true); + bm.Key(k => k.Column("FirstTableID")); + }, + mp => mp.OneToMany()); + }); + + mapper.Class( + m => + { + m.Lazy(false); + + m.Id( + i => i.ID, + id => + { + id.Column("ID"); + id.Generator(Generators.Identity); + }); + }); + + mapper.Class( + m => + { + m.ComposedId( + i => + { + i.ManyToOne(c => c.FirstTable, p => { p.Column("FirstTableID"); }); + + i.ManyToOne(c => c.OtherTable, p => { p.Column("OtherTableID"); }); + }); + }); + + return mapper.CompileMappingForAllExplicitlyAddedEntities(); + } + + [Test] + public async Task FetchQueryDoesNotContainInnerJoinAsync() + { + using (var logSpy = new SqlLogSpy()) + using (var session = OpenSession()) + { + var q = await (session.QueryOver() + .Fetch(SelectMode.Fetch, f => f.AssociationTableCollection) + .Left.JoinQueryOver(f => f.AssociationTableCollection).ListAsync()); + + var sql = logSpy.GetWholeLog(); + Assert.That(sql, Is.Not.Contains("inner join")); + } + } + + [Test] + public async Task FetchQueryDoesNotContainInnerJoinMultiAsync() + { + using (var logSpy = new SqlLogSpy()) + using (var session = OpenSession()) + { + var q = await (session.QueryOver() + .Fetch(SelectMode.Fetch, f => f.AssociationTableCollection) + .Left.JoinQueryOver(f => f.AssociationTableCollection) + .Left.JoinQueryOver(a => a.OtherTable).ListAsync()); + + var sql = logSpy.GetWholeLog(); + + Assert.That(sql, Does.Not.Contain("inner join")); + Assert.That(sql, Does.Match(@"join\s*AssociationTable").IgnoreCase); + Assert.That(sql, Does.Match(@"join\s*OtherTable")); + } + } + + [Test] + public async Task FetchLoadsAllRecordsAsync() + { + IList result = null; + + using (var session = OpenSession()) + { + // the query should return all records from the table with their collections fetched + result = await (session.QueryOver() + .Fetch(SelectMode.Fetch, f => f.AssociationTableCollection) + .Left.JoinQueryOver(f => f.AssociationTableCollection) + .ListAsync()); + } + + Assert.AreEqual(2, result.Count, "Query returned wrong number of records."); + Assert.IsTrue(result.All(x => NHibernateUtil.IsInitialized(x.AssociationTableCollection)), "Not all collections have been initialized"); + } + + [Test] + public async Task FetchInitializesAllCollectionsAsync() + { + IList result = null; + + using (var session = OpenSession()) + { + // load all records + result = await (session.QueryOver() + .ListAsync()); + + // lazy-load the association collection + await (session.QueryOver() + .Fetch(SelectMode.Fetch, f => f.AssociationTableCollection) + .Left.JoinQueryOver(f => f.AssociationTableCollection) + .ListAsync()); + } + + Assert.IsTrue(result.All(x => NHibernateUtil.IsInitialized(x.AssociationTableCollection)), "Not all collections have been initialized"); + } + + protected override void OnSetUp() + { + base.OnSetUp(); + + using (ISession s = OpenSession()) + using (ITransaction t = s.BeginTransaction()) + { + // a record that has association records will be loaded regularly + var withAssociations = new FirstTable(); + + var other1 = new OtherTable(); + var other2 = new OtherTable(); + + var assoc1 = new AssociationTable() {OtherTable = other1, FirstTable = withAssociations}; + var assoc2 = new AssociationTable() {OtherTable = other2, FirstTable = withAssociations}; + + withAssociations.AssociationTableCollection.Add(assoc1); + withAssociations.AssociationTableCollection.Add(assoc2); + s.Save(withAssociations); + + // a record with no associations will have problems if inner joined to association table + var withoutAssociations = new FirstTable(); + s.Save(withoutAssociations); + + t.Commit(); + } + } + + protected override void OnTearDown() + { + using (ISession s = OpenSession()) + using (ITransaction t = s.BeginTransaction()) + { + s.Delete("from AssociationTable"); + s.Delete("from OtherTable"); + s.Delete("from FirstTable"); + t.Commit(); + } + + base.OnTearDown(); + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH3813/FixtureByCode.cs b/src/NHibernate.Test/NHSpecificTest/NH3813/FixtureByCode.cs new file mode 100644 index 00000000000..308c232eb11 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3813/FixtureByCode.cs @@ -0,0 +1,218 @@ +using System.Collections.Generic; +using System.Linq; +using NHibernate.Mapping.ByCode; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH3813 +{ + [TestFixture] + public class KeyManyToOneInnerJoinFetchFixture : TestCaseMappingByCode + { + protected override Cfg.MappingSchema.HbmMapping GetMappings() + { + var mapper = new ModelMapper(); + + mapper.Class( + m => + { + m.Lazy(false); + + m.Id( + i => i.ID, + id => + { + id.Column("ID"); + id.Generator(Generators.Identity); + }); + + m.Bag( + b => b.AssociationTableCollection, + bm => + { + bm.Inverse(true); + bm.Key(k => k.Column("FirstTableID")); + }, + mp => mp.OneToMany()); + }); + + mapper.Class( + m => + { + m.Lazy(false); + + m.Id( + i => i.ID, + id => + { + id.Column("ID"); + id.Generator(Generators.Identity); + }); + }); + + mapper.Class( + m => + { + m.ComposedId( + i => + { + i.ManyToOne(c => c.FirstTable, p => { p.Column("FirstTableID"); }); + + i.ManyToOne(c => c.OtherTable, p => { p.Column("OtherTableID"); }); + }); + }); + + return mapper.CompileMappingForAllExplicitlyAddedEntities(); + } + + [Test] + public void FetchQueryDoesNotContainInnerJoin() + { + using (var logSpy = new SqlLogSpy()) + using (var session = OpenSession()) + { + var q = session.QueryOver() + .Fetch(SelectMode.Fetch, f => f.AssociationTableCollection) + .Left.JoinQueryOver(f => f.AssociationTableCollection).List(); + + var sql = logSpy.GetWholeLog(); + Assert.That(sql, Is.Not.Contains("inner join")); + } + } + + [Test] + public void FetchQueryDoesNotContainInnerJoinMulti() + { + using (var logSpy = new SqlLogSpy()) + using (var session = OpenSession()) + { + var q = session.QueryOver() + .Fetch(SelectMode.Fetch, f => f.AssociationTableCollection) + .Left.JoinQueryOver(f => f.AssociationTableCollection) + .Left.JoinQueryOver(a => a.OtherTable).List(); + + var sql = logSpy.GetWholeLog(); + + Assert.That(sql, Does.Not.Contain("inner join")); + Assert.That(sql, Does.Match(@"join\s*AssociationTable").IgnoreCase); + Assert.That(sql, Does.Match(@"join\s*OtherTable")); + } + } + + [Test] + public void FetchLoadsAllRecords() + { + IList result = null; + + using (var session = OpenSession()) + { + // the query should return all records from the table with their collections fetched + result = session.QueryOver() + .Fetch(SelectMode.Fetch, f => f.AssociationTableCollection) + .Left.JoinQueryOver(f => f.AssociationTableCollection) + .List(); + } + + Assert.AreEqual(2, result.Count, "Query returned wrong number of records."); + Assert.IsTrue(result.All(x => NHibernateUtil.IsInitialized(x.AssociationTableCollection)), "Not all collections have been initialized"); + } + + [Test] + public void FetchInitializesAllCollections() + { + IList result = null; + + using (var session = OpenSession()) + { + // load all records + result = session.QueryOver() + .List(); + + // lazy-load the association collection + session.QueryOver() + .Fetch(SelectMode.Fetch, f => f.AssociationTableCollection) + .Left.JoinQueryOver(f => f.AssociationTableCollection) + .List(); + } + + Assert.IsTrue(result.All(x => NHibernateUtil.IsInitialized(x.AssociationTableCollection)), "Not all collections have been initialized"); + } + + protected override void OnSetUp() + { + base.OnSetUp(); + + using (ISession s = OpenSession()) + using (ITransaction t = s.BeginTransaction()) + { + // a record that has association records will be loaded regularly + var withAssociations = new FirstTable(); + + var other1 = new OtherTable(); + var other2 = new OtherTable(); + + var assoc1 = new AssociationTable() {OtherTable = other1, FirstTable = withAssociations}; + var assoc2 = new AssociationTable() {OtherTable = other2, FirstTable = withAssociations}; + + withAssociations.AssociationTableCollection.Add(assoc1); + withAssociations.AssociationTableCollection.Add(assoc2); + s.Save(withAssociations); + + // a record with no associations will have problems if inner joined to association table + var withoutAssociations = new FirstTable(); + s.Save(withoutAssociations); + + t.Commit(); + } + } + + protected override void OnTearDown() + { + using (ISession s = OpenSession()) + using (ITransaction t = s.BeginTransaction()) + { + s.Delete("from AssociationTable"); + s.Delete("from OtherTable"); + s.Delete("from FirstTable"); + t.Commit(); + } + + base.OnTearDown(); + } + } + + public class FirstTable + { + public virtual int ID { get; set; } + + public virtual IList AssociationTableCollection { get; set; } + + public FirstTable() + { + this.AssociationTableCollection = new List(); + } + } + + public class OtherTable + { + public virtual int ID { get; set; } + } + + public class AssociationTable + { + public virtual int FirstTableID { get; set; } + public virtual FirstTable FirstTable { get; set; } + + public virtual int OtherTableID { get; set; } + public virtual OtherTable OtherTable { get; set; } + + public override bool Equals(object obj) + { + return base.Equals(obj); + } + + public override int GetHashCode() + { + return base.GetHashCode(); + } + } +} diff --git a/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs b/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs index bc71f0d5e0a..107163b7b68 100644 --- a/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs +++ b/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs @@ -94,7 +94,7 @@ protected override void WalkEntityTree(IOuterJoinLoadable persister, string alia { // NH different behavior (NH-1476, NH-1760, NH-1785) base.WalkEntityTree(persister, alias, path, currentDepth); - WalkCompositeComponentIdTree(persister, alias, path); + WalkCompositeComponentIdTree(persister, alias, path, currentDepth); } protected override OuterJoinableAssociation CreateRootAssociation() @@ -130,14 +130,14 @@ protected override ISet GetEntityFetchLazyProperties(string path) return translator.RootCriteria.GetEntityFetchLazyProperties(path); } - private void WalkCompositeComponentIdTree(IOuterJoinLoadable persister, string alias, string path) + private void WalkCompositeComponentIdTree(IOuterJoinLoadable persister, string alias, string path, int currentDepth) { IType type = persister.IdentifierType; string propertyName = persister.IdentifierPropertyName; if (type != null && type.IsComponentType) { ILhsAssociationTypeSqlInfo associationTypeSQLInfo = JoinHelper.GetIdLhsSqlInfo(alias, persister, Factory); - WalkComponentTree((IAbstractComponentType) type, 0, alias, SubPath(path, propertyName), 0, associationTypeSQLInfo); + WalkComponentTree((IAbstractComponentType) type, 0, alias, SubPath(path, propertyName), currentDepth, associationTypeSQLInfo); } } From f6f21707322de67eb1f315c054d7f58fdafb6818 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Thu, 3 Sep 2020 14:33:29 +0300 Subject: [PATCH 2/3] Clean up and fix SqlServerCe --- .../NHSpecificTest/NH3813/FixtureByCode.cs | 8 ++-- .../NHSpecificTest/NH3813/Entities.cs | 35 +++++++++++++++ .../NHSpecificTest/NH3813/FixtureByCode.cs | 44 +++---------------- 3 files changed, 45 insertions(+), 42 deletions(-) create mode 100644 src/NHibernate.Test/NHSpecificTest/NH3813/Entities.cs diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH3813/FixtureByCode.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH3813/FixtureByCode.cs index 79e8cef94df..cb5d578a8ed 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/NH3813/FixtureByCode.cs +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH3813/FixtureByCode.cs @@ -29,12 +29,13 @@ protected override Cfg.MappingSchema.HbmMapping GetMappings() m.Lazy(false); m.Id( - i => i.ID, + i => i.Id, id => { id.Column("ID"); id.Generator(Generators.Identity); }); + m.Property(x => x.Name); m.Bag( b => b.AssociationTableCollection, @@ -52,12 +53,13 @@ protected override Cfg.MappingSchema.HbmMapping GetMappings() m.Lazy(false); m.Id( - i => i.ID, + i => i.Id, id => { id.Column("ID"); id.Generator(Generators.Identity); }); + m.Property(x => x.Name); }); mapper.Class( @@ -67,9 +69,9 @@ protected override Cfg.MappingSchema.HbmMapping GetMappings() i => { i.ManyToOne(c => c.FirstTable, p => { p.Column("FirstTableID"); }); - i.ManyToOne(c => c.OtherTable, p => { p.Column("OtherTableID"); }); }); + m.Property(x => x.Name); }); return mapper.CompileMappingForAllExplicitlyAddedEntities(); diff --git a/src/NHibernate.Test/NHSpecificTest/NH3813/Entities.cs b/src/NHibernate.Test/NHSpecificTest/NH3813/Entities.cs new file mode 100644 index 00000000000..61bf7fb6039 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3813/Entities.cs @@ -0,0 +1,35 @@ +using System.Collections.Generic; + +namespace NHibernate.Test.NHSpecificTest.NH3813 +{ + public class AssociationTable + { + public virtual FirstTable FirstTable { get; set; } + public virtual OtherTable OtherTable { get; set; } + public virtual string Name { get; set; } + + public override bool Equals(object obj) + { + return base.Equals(obj); + } + + public override int GetHashCode() + { + return base.GetHashCode(); + } + } + + public class FirstTable + { + public virtual int Id { get; set; } + public virtual string Name { get; set; } + + public virtual IList AssociationTableCollection { get; set; } = new List(); + } + + public class OtherTable + { + public virtual int Id { get; set; } + public virtual string Name { get; set; } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH3813/FixtureByCode.cs b/src/NHibernate.Test/NHSpecificTest/NH3813/FixtureByCode.cs index 308c232eb11..907711119db 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH3813/FixtureByCode.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH3813/FixtureByCode.cs @@ -18,12 +18,13 @@ protected override Cfg.MappingSchema.HbmMapping GetMappings() m.Lazy(false); m.Id( - i => i.ID, + i => i.Id, id => { id.Column("ID"); id.Generator(Generators.Identity); }); + m.Property(x => x.Name); m.Bag( b => b.AssociationTableCollection, @@ -41,12 +42,13 @@ protected override Cfg.MappingSchema.HbmMapping GetMappings() m.Lazy(false); m.Id( - i => i.ID, + i => i.Id, id => { id.Column("ID"); id.Generator(Generators.Identity); }); + m.Property(x => x.Name); }); mapper.Class( @@ -56,9 +58,9 @@ protected override Cfg.MappingSchema.HbmMapping GetMappings() i => { i.ManyToOne(c => c.FirstTable, p => { p.Column("FirstTableID"); }); - i.ManyToOne(c => c.OtherTable, p => { p.Column("OtherTableID"); }); }); + m.Property(x => x.Name); }); return mapper.CompileMappingForAllExplicitlyAddedEntities(); @@ -179,40 +181,4 @@ protected override void OnTearDown() base.OnTearDown(); } } - - public class FirstTable - { - public virtual int ID { get; set; } - - public virtual IList AssociationTableCollection { get; set; } - - public FirstTable() - { - this.AssociationTableCollection = new List(); - } - } - - public class OtherTable - { - public virtual int ID { get; set; } - } - - public class AssociationTable - { - public virtual int FirstTableID { get; set; } - public virtual FirstTable FirstTable { get; set; } - - public virtual int OtherTableID { get; set; } - public virtual OtherTable OtherTable { get; set; } - - public override bool Equals(object obj) - { - return base.Equals(obj); - } - - public override int GetHashCode() - { - return base.GetHashCode(); - } - } } From d6823ffd01a53357425c4cf787165aaa0ac74f47 Mon Sep 17 00:00:00 2001 From: Alex Zaytsev Date: Fri, 4 Sep 2020 17:47:34 +1200 Subject: [PATCH 3/3] Cleanup tests --- .../Async/NHSpecificTest/NH3813/FixtureByCode.cs | 16 +++++----------- .../NHSpecificTest/NH3813/FixtureByCode.cs | 16 +++++----------- 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH3813/FixtureByCode.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH3813/FixtureByCode.cs index cb5d578a8ed..366f83da4b6 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/NH3813/FixtureByCode.cs +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH3813/FixtureByCode.cs @@ -152,10 +152,8 @@ public async Task FetchInitializesAllCollectionsAsync() protected override void OnSetUp() { - base.OnSetUp(); - - using (ISession s = OpenSession()) - using (ITransaction t = s.BeginTransaction()) + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) { // a record that has association records will be loaded regularly var withAssociations = new FirstTable(); @@ -180,16 +178,12 @@ protected override void OnSetUp() protected override void OnTearDown() { - using (ISession s = OpenSession()) - using (ITransaction t = s.BeginTransaction()) + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) { - s.Delete("from AssociationTable"); - s.Delete("from OtherTable"); - s.Delete("from FirstTable"); + s.CreateQuery("delete from System.Object").ExecuteUpdate(); t.Commit(); } - - base.OnTearDown(); } } } diff --git a/src/NHibernate.Test/NHSpecificTest/NH3813/FixtureByCode.cs b/src/NHibernate.Test/NHSpecificTest/NH3813/FixtureByCode.cs index 907711119db..cb8c60cb6ed 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH3813/FixtureByCode.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH3813/FixtureByCode.cs @@ -141,10 +141,8 @@ public void FetchInitializesAllCollections() protected override void OnSetUp() { - base.OnSetUp(); - - using (ISession s = OpenSession()) - using (ITransaction t = s.BeginTransaction()) + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) { // a record that has association records will be loaded regularly var withAssociations = new FirstTable(); @@ -169,16 +167,12 @@ protected override void OnSetUp() protected override void OnTearDown() { - using (ISession s = OpenSession()) - using (ITransaction t = s.BeginTransaction()) + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) { - s.Delete("from AssociationTable"); - s.Delete("from OtherTable"); - s.Delete("from FirstTable"); + s.CreateQuery("delete from System.Object").ExecuteUpdate(); t.Commit(); } - - base.OnTearDown(); } } }