From a82beed9f4f645e3a9f0fd102ae61587d00c362d Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Mon, 9 Aug 2021 11:39:52 +0300 Subject: [PATCH 1/4] Restore 5.2.x behavior for nullable entity null check --- src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs | 6 ++++++ src/NHibernate.Test/Hql/EntityJoinHqlTest.cs | 6 ++++++ src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.cs | 3 +++ src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.g | 2 +- src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs | 9 ++++++--- 5 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs b/src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs index f016139f79b..f79ca262160 100644 --- a/src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs +++ b/src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs @@ -342,8 +342,14 @@ public async Task NullableEntityProjectionAsync() var fullList = await (session.Query().Select(x => new {x.Name, ManyToOneId = (Guid?) x.ManyToOne.Id}).ToListAsync()); var withValidManyToOneList = await (session.Query().Where(x => x.ManyToOne != null).Select(x => new {x.Name, ManyToOneId = (Guid?) x.ManyToOne.Id}).ToListAsync()); + var withValidManyToOneList2 = await (session.CreateQuery("from NullableOwner ex where not ex.ManyToOne is null").ListAsync()); + var withNullManyToOneList = await (session.Query().Where(x => x.ManyToOne == null).ToListAsync()); Assert.That(fullList.Count, Is.EqualTo(2)); Assert.That(withValidManyToOneList.Count, Is.EqualTo(0)); + Assert.That(withValidManyToOneList2.Count, Is.EqualTo(0)); + //NOTE: IS NULL case is broken, it returns 1 instead of 2 records. + //Assert.That(withNullManyToOneList.Count, Is.EqualTo(2)); + Assert.That(withNullManyToOneList.Count, Is.GreaterThan(0)); } } diff --git a/src/NHibernate.Test/Hql/EntityJoinHqlTest.cs b/src/NHibernate.Test/Hql/EntityJoinHqlTest.cs index 2eec6634d65..b166023fca5 100644 --- a/src/NHibernate.Test/Hql/EntityJoinHqlTest.cs +++ b/src/NHibernate.Test/Hql/EntityJoinHqlTest.cs @@ -330,8 +330,14 @@ public void NullableEntityProjection() var fullList = session.Query().Select(x => new {x.Name, ManyToOneId = (Guid?) x.ManyToOne.Id}).ToList(); var withValidManyToOneList = session.Query().Where(x => x.ManyToOne != null).Select(x => new {x.Name, ManyToOneId = (Guid?) x.ManyToOne.Id}).ToList(); + var withValidManyToOneList2 = session.CreateQuery("from NullableOwner ex where not ex.ManyToOne is null").List(); + var withNullManyToOneList = session.Query().Where(x => x.ManyToOne == null).ToList(); Assert.That(fullList.Count, Is.EqualTo(2)); Assert.That(withValidManyToOneList.Count, Is.EqualTo(0)); + Assert.That(withValidManyToOneList2.Count, Is.EqualTo(0)); + //NOTE: IS NULL case is broken, it returns 1 instead of 2 records. + //Assert.That(withNullManyToOneList.Count, Is.EqualTo(2)); + Assert.That(withNullManyToOneList.Count, Is.GreaterThan(0)); } } diff --git a/src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.cs b/src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.cs index 578d9331153..3dc2cd30640 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.cs @@ -42,6 +42,7 @@ public partial class HqlSqlWalker private SelectClause _selectClause; private readonly AliasGenerator _aliasGenerator = new AliasGenerator(); private readonly ASTPrinter _printer = new ASTPrinter(); + private bool _isNullComparison; // //Maps each top-level result variable to its SelectExpression; @@ -1203,6 +1204,8 @@ public IASTFactory ASTFactory } } + internal bool IsNullComparison => _isNullComparison; + public void AddQuerySpaces(string[] spaces) { for (int i = 0; i < spaces.Length; i++) diff --git a/src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.g b/src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.g index fba1010337c..d99340b9726 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.g +++ b/src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.g @@ -380,7 +380,7 @@ comparisonExpr | ^(NOT_BETWEEN exprOrSubquery exprOrSubquery exprOrSubquery) | ^(IN exprOrSubquery inRhs ) | ^(NOT_IN exprOrSubquery inRhs ) - | ^(IS_NULL exprOrSubquery) + | ^(IS_NULL { _isNullComparison = true; } exprOrSubquery { _isNullComparison = false; }) | ^(IS_NOT_NULL exprOrSubquery) // | ^(IS_TRUE expr) // | ^(IS_FALSE expr) diff --git a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs index 9db7baacb2f..50be711aad7 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs @@ -389,9 +389,12 @@ private void DereferenceEntity(EntityType entityType, bool implicitJoin, string bool joinIsNeeded; //For nullable entity comparisons we always need to add join (like not constrained one-to-one or not-found ignore associations) - //NOTE: This fix is not fully correct. It doesn't work for comparisons with null (where e.OneToOneProp is null) - // as by default implicit join is generated and to work propelry left join is required (see GH-2611) - bool comparisonWithNullableEntity = entityType.IsNullable && Walker.IsComparativeExpressionClause; + bool comparisonWithNullableEntity = entityType.IsNullable && Walker.IsComparativeExpressionClause + //NOTE: comparisonWithNullableEntity fix is not fully correct. It doesn't work for comparisons with null (where e.OneToOneProp is null) + // as by default implicit join is generated and to work propelry left join is required (see GH-2611 and GH-2881). + // IsNullComparison check was introduced that disables this fix for null comparison + // (it still generates wrong query but simply returns 5.2.x behavior) + && !Walker.IsNullComparison; if ( IsDotNode( parent ) ) { From 1b16e4fbc57fddc6a400b7371f23593baba989b1 Mon Sep 17 00:00:00 2001 From: Alex Zaytsev Date: Tue, 10 Aug 2021 17:11:52 +1200 Subject: [PATCH 2/4] I maked these --- .../Async/Hql/EntityJoinHqlTest.cs | 4 +--- src/NHibernate.Test/Hql/EntityJoinHqlTest.cs | 4 +--- src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs | 18 ++++++++++-------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs b/src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs index f79ca262160..eebd9d8768c 100644 --- a/src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs +++ b/src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs @@ -347,9 +347,7 @@ public async Task NullableEntityProjectionAsync() Assert.That(fullList.Count, Is.EqualTo(2)); Assert.That(withValidManyToOneList.Count, Is.EqualTo(0)); Assert.That(withValidManyToOneList2.Count, Is.EqualTo(0)); - //NOTE: IS NULL case is broken, it returns 1 instead of 2 records. - //Assert.That(withNullManyToOneList.Count, Is.EqualTo(2)); - Assert.That(withNullManyToOneList.Count, Is.GreaterThan(0)); + Assert.That(withNullManyToOneList.Count, Is.EqualTo(2)); } } diff --git a/src/NHibernate.Test/Hql/EntityJoinHqlTest.cs b/src/NHibernate.Test/Hql/EntityJoinHqlTest.cs index b166023fca5..85164ebb861 100644 --- a/src/NHibernate.Test/Hql/EntityJoinHqlTest.cs +++ b/src/NHibernate.Test/Hql/EntityJoinHqlTest.cs @@ -335,9 +335,7 @@ public void NullableEntityProjection() Assert.That(fullList.Count, Is.EqualTo(2)); Assert.That(withValidManyToOneList.Count, Is.EqualTo(0)); Assert.That(withValidManyToOneList2.Count, Is.EqualTo(0)); - //NOTE: IS NULL case is broken, it returns 1 instead of 2 records. - //Assert.That(withNullManyToOneList.Count, Is.EqualTo(2)); - Assert.That(withNullManyToOneList.Count, Is.GreaterThan(0)); + Assert.That(withNullManyToOneList.Count, Is.EqualTo(2)); } } diff --git a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs index 50be711aad7..0155f71e910 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs @@ -389,12 +389,7 @@ private void DereferenceEntity(EntityType entityType, bool implicitJoin, string 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 = entityType.IsNullable && Walker.IsComparativeExpressionClause - //NOTE: comparisonWithNullableEntity fix is not fully correct. It doesn't work for comparisons with null (where e.OneToOneProp is null) - // as by default implicit join is generated and to work propelry left join is required (see GH-2611 and GH-2881). - // IsNullComparison check was introduced that disables this fix for null comparison - // (it still generates wrong query but simply returns 5.2.x behavior) - && !Walker.IsNullComparison; + bool comparisonWithNullableEntity = entityType.IsNullable && Walker.IsComparativeExpressionClause; if ( IsDotNode( parent ) ) { @@ -420,8 +415,15 @@ private void DereferenceEntity(EntityType entityType, bool implicitJoin, string || comparisonWithNullableEntity; } - if ( joinIsNeeded ) + if ( joinIsNeeded ) { + //TODO: Find correct place for this if. + if (Walker.IsNullComparison) + { + implicitJoin = false; + _joinType = JoinType.LeftOuterJoin; + } + DereferenceEntityJoin( classAlias, entityType, implicitJoin, parent ); if (comparisonWithNullableEntity) { @@ -475,7 +477,7 @@ private void DereferenceEntityJoin(string classAlias, EntityType propertyType, b string[] joinColumns = GetColumns(); string joinPath = Path; - if ( impliedJoin && Walker.IsInFrom ) + if ( impliedJoin && Walker.IsInFrom ) { _joinType = Walker.ImpliedJoinType; } From b96b29958b7e738d50ca0dcc111777531e1a619c Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Tue, 10 Aug 2021 09:10:57 +0300 Subject: [PATCH 3/4] Small adjustement --- src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs index 0155f71e910..f5c5258aaa2 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs @@ -417,8 +417,7 @@ private void DereferenceEntity(EntityType entityType, bool implicitJoin, string if ( joinIsNeeded ) { - //TODO: Find correct place for this if. - if (Walker.IsNullComparison) + if (comparisonWithNullableEntity && Walker.IsNullComparison) { implicitJoin = false; _joinType = JoinType.LeftOuterJoin; From 310e1a96e386d8d3b97ed63aec2ff6e1e995de3a Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Tue, 10 Aug 2021 09:15:58 +0300 Subject: [PATCH 4/4] Remove unrelated changes --- src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs index f5c5258aaa2..b594f4aad44 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs @@ -476,7 +476,7 @@ private void DereferenceEntityJoin(string classAlias, EntityType propertyType, b string[] joinColumns = GetColumns(); string joinPath = Path; - if ( impliedJoin && Walker.IsInFrom ) + if ( impliedJoin && Walker.IsInFrom ) { _joinType = Walker.ImpliedJoinType; }