diff --git a/src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs b/src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs index dcca7fc2924..1222dc245f0 100644 --- a/src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs +++ b/src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs @@ -153,6 +153,124 @@ public async Task EntityJoinFoSubqueryAsync() } } + [Test] + public async Task EntityJoinWithNullableOneToOneEntityComparisonInWithClausShouldAddJoinAsync() + { + using (var sqlLog = new SqlLogSpy()) + using (var session = OpenSession()) + { + var entity = + await (session + .CreateQuery( + "select ex " + + "from NullableOwner ex " + + "left join OneToOneEntity st with st = ex.OneToOne " + ).SetMaxResults(1) + .UniqueResultAsync()); + + Assert.That(Regex.Matches(sqlLog.GetWholeLog(), "OneToOneEntity").Count, Is.EqualTo(2)); + Assert.That(sqlLog.Appender.GetEvents().Length, Is.EqualTo(1), "Only one SQL select is expected"); + } + } + + [Test] + public async Task NullableOneToOneWhereEntityIsNotNullAsync() + { + using (var sqlLog = new SqlLogSpy()) + using (var session = OpenSession()) + { + var entity = + await (session + .CreateQuery( + "select ex " + + "from NullableOwner ex " + + "where ex.OneToOne is not null " + ).SetMaxResults(1) + .UniqueResultAsync()); + + Assert.That(Regex.Matches(sqlLog.GetWholeLog(), "OneToOneEntity").Count, Is.EqualTo(1)); + Assert.That(sqlLog.Appender.GetEvents().Length, Is.EqualTo(1), "Only one SQL select is expected"); + } + } + + [Test] + public async Task NullableOneToOneWhereIdIsNotNullAsync() + { + using (var sqlLog = new SqlLogSpy()) + using (var session = OpenSession()) + { + var entity = + await (session + .CreateQuery( + "select ex " + + "from NullableOwner ex " + + "where ex.OneToOne.Id is not null " + ).SetMaxResults(1) + .UniqueResultAsync()); + + Assert.That(Regex.Matches(sqlLog.GetWholeLog(), "OneToOneEntity").Count, Is.EqualTo(1)); + Assert.That(sqlLog.Appender.GetEvents().Length, Is.EqualTo(1), "Only one SQL select is expected"); + } + } + + [Test] + public async Task NullablePropRefWhereIdEntityNotNullShouldAddJoinAsync() + { + using (var sqlLog = new SqlLogSpy()) + using (var session = OpenSession()) + { + var entity = + await (session + .CreateQuery( + "select ex " + + "from NullableOwner ex " + + "where ex.PropRef is not null " + ).SetMaxResults(1) + .UniqueResultAsync()); + + Assert.That(Regex.Matches(sqlLog.GetWholeLog(), "PropRefEntity").Count, Is.EqualTo(1)); + Assert.That(sqlLog.Appender.GetEvents().Length, Is.EqualTo(1), "Only one SQL select is expected"); + } + } + + [Test] + public async Task NullableOneToOneFetchQueryIsNotAffectedAsync() + { + using (var sqlLog = new SqlLogSpy()) + using (var session = OpenSession()) + { + var entity = + await (session + .CreateQuery( + "select ex " + + "from NullableOwner ex left join fetch ex.OneToOne o " + + "where o is null " + ).SetMaxResults(1) + .UniqueResultAsync()); + + Assert.That(Regex.Matches(sqlLog.GetWholeLog(), "OneToOneEntity").Count, Is.EqualTo(1)); + } + } + + [Test] + public async Task NullableOneToOneFetchQueryIsNotAffected2Async() + { + using (var sqlLog = new SqlLogSpy()) + using (var session = OpenSession()) + { + var entity = + await (session + .CreateQuery( + "select ex " + + "from NullableOwner ex left join fetch ex.OneToOne o " + + "where o.Id is null " + ).SetMaxResults(1) + .UniqueResultAsync()); + + Assert.That(Regex.Matches(sqlLog.GetWholeLog(), "OneToOneEntity").Count, Is.EqualTo(1)); + } + } + [Test] public async Task EntityJoinWithEntityComparisonInWithClausShouldNotAddJoinAsync() { @@ -329,7 +447,7 @@ protected override HbmMapping GetMappings() rc.Property(e => e.Name); }); - + mapper.Class( rc => { @@ -367,6 +485,38 @@ protected override HbmMapping GetMappings() rc.Property(e => e.Name); }); + mapper.Class( + rc => + { + rc.Id(e => e.Id, m => m.Generator(Generators.GuidComb)); + rc.Property(e => e.Name); + }); + + mapper.Class( + rc => + { + rc.Id(e => e.Id, m => m.Generator(Generators.GuidComb)); + rc.Property(e => e.Name); + rc.Property(e => e.PropertyRef); + }); + + mapper.Class( + rc => + { + rc.Id(e => e.Id, m => m.Generator(Generators.GuidComb)); + rc.Property(e => e.Name); + rc.OneToOne(e => e.OneToOne, m => m.Constrained(false)); + rc.ManyToOne( + e => e.PropRef, + m => + { + m.PropertyRef(nameof(PropRefEntity.PropertyRef)); + m.ForeignKey("none"); + m.NotFound(NotFoundMode.Ignore); + }); + }); + + return mapper.CompileMappingForAllExplicitlyAddedEntities(); } @@ -431,7 +581,6 @@ protected override void OnSetUp() Composite1Key2 = _entityWithCompositeId.Key.Id2, CustomEntityNameId = _entityWithCustomEntityName.Id }; - session.Save(_noAssociation); session.Flush(); diff --git a/src/NHibernate.Test/Hql/EntityJoinHqlTest.cs b/src/NHibernate.Test/Hql/EntityJoinHqlTest.cs index 0a578bd982a..f01b6303d30 100644 --- a/src/NHibernate.Test/Hql/EntityJoinHqlTest.cs +++ b/src/NHibernate.Test/Hql/EntityJoinHqlTest.cs @@ -142,6 +142,124 @@ public void EntityJoinFoSubquery() } } + [Test] + public void EntityJoinWithNullableOneToOneEntityComparisonInWithClausShouldAddJoin() + { + using (var sqlLog = new SqlLogSpy()) + using (var session = OpenSession()) + { + var entity = + session + .CreateQuery( + "select ex " + + "from NullableOwner ex " + + "left join OneToOneEntity st with st = ex.OneToOne " + ).SetMaxResults(1) + .UniqueResult(); + + Assert.That(Regex.Matches(sqlLog.GetWholeLog(), "OneToOneEntity").Count, Is.EqualTo(2)); + Assert.That(sqlLog.Appender.GetEvents().Length, Is.EqualTo(1), "Only one SQL select is expected"); + } + } + + [Test] + public void NullableOneToOneWhereEntityIsNotNull() + { + using (var sqlLog = new SqlLogSpy()) + using (var session = OpenSession()) + { + var entity = + session + .CreateQuery( + "select ex " + + "from NullableOwner ex " + + "where ex.OneToOne is not null " + ).SetMaxResults(1) + .UniqueResult(); + + Assert.That(Regex.Matches(sqlLog.GetWholeLog(), "OneToOneEntity").Count, Is.EqualTo(1)); + Assert.That(sqlLog.Appender.GetEvents().Length, Is.EqualTo(1), "Only one SQL select is expected"); + } + } + + [Test] + public void NullableOneToOneWhereIdIsNotNull() + { + using (var sqlLog = new SqlLogSpy()) + using (var session = OpenSession()) + { + var entity = + session + .CreateQuery( + "select ex " + + "from NullableOwner ex " + + "where ex.OneToOne.Id is not null " + ).SetMaxResults(1) + .UniqueResult(); + + Assert.That(Regex.Matches(sqlLog.GetWholeLog(), "OneToOneEntity").Count, Is.EqualTo(1)); + Assert.That(sqlLog.Appender.GetEvents().Length, Is.EqualTo(1), "Only one SQL select is expected"); + } + } + + [Test] + public void NullablePropRefWhereIdEntityNotNullShouldAddJoin() + { + using (var sqlLog = new SqlLogSpy()) + using (var session = OpenSession()) + { + var entity = + session + .CreateQuery( + "select ex " + + "from NullableOwner ex " + + "where ex.PropRef is not null " + ).SetMaxResults(1) + .UniqueResult(); + + Assert.That(Regex.Matches(sqlLog.GetWholeLog(), "PropRefEntity").Count, Is.EqualTo(1)); + Assert.That(sqlLog.Appender.GetEvents().Length, Is.EqualTo(1), "Only one SQL select is expected"); + } + } + + [Test] + public void NullableOneToOneFetchQueryIsNotAffected() + { + using (var sqlLog = new SqlLogSpy()) + using (var session = OpenSession()) + { + var entity = + session + .CreateQuery( + "select ex " + + "from NullableOwner ex left join fetch ex.OneToOne o " + + "where o is null " + ).SetMaxResults(1) + .UniqueResult(); + + Assert.That(Regex.Matches(sqlLog.GetWholeLog(), "OneToOneEntity").Count, Is.EqualTo(1)); + } + } + + [Test] + public void NullableOneToOneFetchQueryIsNotAffected2() + { + using (var sqlLog = new SqlLogSpy()) + using (var session = OpenSession()) + { + var entity = + session + .CreateQuery( + "select ex " + + "from NullableOwner ex left join fetch ex.OneToOne o " + + "where o.Id is null " + ).SetMaxResults(1) + .UniqueResult(); + + Assert.That(Regex.Matches(sqlLog.GetWholeLog(), "OneToOneEntity").Count, Is.EqualTo(1)); + } + } + [Test] public void EntityJoinWithEntityComparisonInWithClausShouldNotAddJoin() { @@ -334,7 +452,7 @@ protected override HbmMapping GetMappings() rc.Property(e => e.Name); }); - + mapper.Class( rc => { @@ -372,6 +490,39 @@ protected override HbmMapping GetMappings() rc.Property(e => e.Name); }); + mapper.Class( + rc => + { + rc.Id(e => e.Id, m => m.Generator(Generators.GuidComb)); + rc.Property(e => e.Name); + }); + + mapper.Class( + rc => + { + rc.Id(e => e.Id, m => m.Generator(Generators.GuidComb)); + rc.Property(e => e.Name); + rc.Property(e => e.PropertyRef, m => m.Column("EntityPropertyRef")); + }); + + mapper.Class( + rc => + { + rc.Id(e => e.Id, m => m.Generator(Generators.GuidComb)); + rc.Property(e => e.Name); + rc.OneToOne(e => e.OneToOne, m => m.Constrained(false)); + rc.ManyToOne( + e => e.PropRef, + m => + { + m.Column("OwnerPropertyRef"); + m.PropertyRef(nameof(PropRefEntity.PropertyRef)); + m.ForeignKey("none"); + m.NotFound(NotFoundMode.Ignore); + }); + }); + + return mapper.CompileMappingForAllExplicitlyAddedEntities(); } @@ -436,7 +587,6 @@ protected override void OnSetUp() Composite1Key2 = _entityWithCompositeId.Key.Id2, CustomEntityNameId = _entityWithCustomEntityName.Id }; - session.Save(_noAssociation); session.Flush(); diff --git a/src/NHibernate.Test/Hql/EntityJoinHqlTestEntities.cs b/src/NHibernate.Test/Hql/EntityJoinHqlTestEntities.cs index 2b897d81b6a..277b0ea706b 100644 --- a/src/NHibernate.Test/Hql/EntityJoinHqlTestEntities.cs +++ b/src/NHibernate.Test/Hql/EntityJoinHqlTestEntities.cs @@ -5,22 +5,40 @@ namespace NHibernate.Test.Hql.EntityJoinHqlTestEntities public class EntityComplex { public virtual Guid Id { get; set; } - public virtual int Version { get; set; } - public virtual string Name { get; set; } - public virtual string LazyProp { get; set; } - public virtual EntityComplex SameTypeChild { get; set; } public virtual EntityComplex SameTypeChild2 { get; set; } } + public class OneToOneEntity + { + public virtual Guid Id { get; set; } + public virtual string Name { get; set; } + } + + public class PropRefEntity + { + public virtual Guid Id { get; set; } + public virtual string Name { get; set; } + public virtual string PropertyRef { get; set; } + } + + public class NullableOwner + { + public virtual Guid Id { get; set; } + public virtual string Name { get; set; } + public virtual OneToOneEntity OneToOne { get; set; } + public virtual PropRefEntity PropRef { get; set; } + } + public class EntityWithCompositeId { public virtual CompositeKey Key { get; set; } public virtual string Name { get; set; } } + public class CompositeKey { public int Id1 { get; set; } diff --git a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs index e1ed1bce5e9..bbe0a91b740 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs @@ -386,6 +386,9 @@ private void DereferenceEntity(EntityType entityType, bool implicitJoin, string string property = _propertyName; bool joinIsNeeded; + //For nullable entity comparisons we always need to add join (like not constrained one-to-one or not-found ignore associations) + bool comparisonWithNullableEntity = false; + if ( IsDotNode( parent ) ) { // our parent is another dot node, meaning we are being further dereferenced. @@ -393,7 +396,7 @@ private void DereferenceEntity(EntityType entityType, bool implicitJoin, string // entity's PK (because 'our' table would know the FK). parentAsDotNode = ( DotNode ) parent; property = parentAsDotNode._propertyName; - joinIsNeeded = generateJoin && !IsReferenceToPrimaryKey( parentAsDotNode._propertyName, entityType ); + joinIsNeeded = generateJoin && (entityType.IsNullable || !IsReferenceToPrimaryKey( parentAsDotNode._propertyName, entityType )); } else if ( ! Walker.IsSelectStatement ) { @@ -406,12 +409,18 @@ private void DereferenceEntity(EntityType entityType, bool implicitJoin, string } else { - joinIsNeeded = generateJoin || ((Walker.IsInSelect && !Walker.IsInCase) || (Walker.IsInFrom && !Walker.IsComparativeExpressionClause)); + comparisonWithNullableEntity = (Walker.IsComparativeExpressionClause && entityType.IsNullable); + joinIsNeeded = generateJoin || (Walker.IsInSelect && !Walker.IsInCase) || (Walker.IsInFrom && !Walker.IsComparativeExpressionClause) + || comparisonWithNullableEntity; } if ( joinIsNeeded ) { DereferenceEntityJoin( classAlias, entityType, implicitJoin, parent ); + if (comparisonWithNullableEntity) + { + _columns = FromElement.GetIdentityColumns(); + } } else { diff --git a/src/NHibernate/Hql/Ast/ANTLR/Tree/FromElement.cs b/src/NHibernate/Hql/Ast/ANTLR/Tree/FromElement.cs index b84a43b23e6..fa4796baebc 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/Tree/FromElement.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/Tree/FromElement.cs @@ -482,6 +482,19 @@ public IType GetPropertyType(string propertyName, string propertyPath) } public virtual string GetIdentityColumn() + { + var cols = GetIdentityColumns(); + string result = string.Join(", ", cols); + + if (cols.Length > 1 && Walker.IsComparativeExpressionClause) + { + return "(" + result + ")"; + } + + return result; + } + + internal string[] GetIdentityColumns() { CheckInitialized(); string table = TableAlias; @@ -513,14 +526,8 @@ public virtual string GetIdentityColumn() { cols = GetPropertyMapping(propertyName).ToColumns(propertyName); } - string result = string.Join(", ", cols); - if (cols.Length > 1 && Walker.IsComparativeExpressionClause) - { - return "(" + result + ")"; - } - - return result; + return cols; } public void HandlePropertyBeingDereferenced(IType propertySource, string propertyName)