From 6b9ce7c9f157465b9f5a7d54234a8109b9326aa1 Mon Sep 17 00:00:00 2001 From: Alex Zaytsev Date: Tue, 24 Sep 2024 09:59:01 +1000 Subject: [PATCH 1/3] Add tests for #3609 --- .../NHSpecificTest/GH3609/Entities.cs | 24 ++++++ .../NHSpecificTest/GH3609/Fixture.cs | 74 +++++++++++++++++++ .../NHSpecificTest/GH3609/Mappings.hbm.xml | 21 ++++++ src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs | 2 +- 4 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 src/NHibernate.Test/NHSpecificTest/GH3609/Entities.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/GH3609/Fixture.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/GH3609/Mappings.hbm.xml diff --git a/src/NHibernate.Test/NHSpecificTest/GH3609/Entities.cs b/src/NHibernate.Test/NHSpecificTest/GH3609/Entities.cs new file mode 100644 index 00000000000..28857618b4f --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH3609/Entities.cs @@ -0,0 +1,24 @@ +using System; + +namespace NHibernate.Test.NHSpecificTest.GH3609 +{ + public class Order + { + public virtual long Id { get; set; } + + public virtual string UniqueId { get; set; } = Guid.NewGuid().ToString(); + + public virtual DateTime CreatedDate { get; set; } + } + + public class LineItem + { + public virtual long Id { get; set; } + + public virtual Order Order { get; set; } + + public virtual string ItemName { get; set; } + + public virtual decimal Amount { get; set; } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH3609/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH3609/Fixture.cs new file mode 100644 index 00000000000..956b9de55ec --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH3609/Fixture.cs @@ -0,0 +1,74 @@ +using System; +using System.Linq; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.GH3609 +{ + [TestFixture] + public class Fixture : BugTestCase + { + protected override void OnSetUp() + { + using var session = OpenSession(); + using var transaction = session.BeginTransaction(); + + var order = new Order + { + UniqueId = "0ab92479-8a17-4dbc-9bef-ce4344940cec", + CreatedDate = new DateTime(2024, 09, 24) + }; + session.Save(order); + + session.Save(new LineItem { Order = order, ItemName = "Bananas", Amount = 5 }); + + transaction.Commit(); + } + + protected override void OnTearDown() + { + using var session = OpenSession(); + using var transaction = session.BeginTransaction(); + + session.CreateQuery("delete from System.Object").ExecuteUpdate(); + + transaction.Commit(); + } + + [Test] + public void QueryWithAny() + { + using var session = OpenSession(); + using var transaction = session.BeginTransaction(); + + // This form of query is how we first discovered the issue. This is a simplified reproduction of the + // sort of Linq that we were using in our app. It seems to occur when we force an EXISTS( ... ) subquery. + var validOrders = session.Query().Where(x => x.CreatedDate > new DateTime(2024, 9, 10)); + var orderCount = session.Query().Count(x => validOrders.Any(y => y == x.Order)); + + Assert.That(orderCount, Is.EqualTo(1)); + } + + [Test] + public void QueryWithContains() + { + using var session = OpenSession(); + using var transaction = session.BeginTransaction(); + + var validOrders = session.Query().Where(x => x.CreatedDate > new DateTime(2024, 9, 10)); + var orderCount = session.Query().Count(x => validOrders.Contains(x.Order)); + + Assert.That(orderCount, Is.EqualTo(1)); + } + + [Test] + public void SimpleQueryForDataWhichWasInsertedViaAdoShouldProvideExpectedResults() + { + using var session = OpenSession(); + using var transaction = session.BeginTransaction(); + + // This style of equivalent query does not exhibit the problem. This test passes no matter which NH version. + var lineItem = session.Query().FirstOrDefault(x => x.Order.CreatedDate > new DateTime(2024, 9, 10)); + Assert.That(lineItem, Is.Not.Null); + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH3609/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/GH3609/Mappings.hbm.xml new file mode 100644 index 00000000000..e21dd4871b6 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH3609/Mappings.hbm.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + + + diff --git a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs index 4b0f127a2ca..e05b1600485 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs @@ -161,7 +161,7 @@ public override void ResolveFirstChild() string propName = property.Text; _propertyName = propName; - // If the uresolved property path isn't set yet, just use the property name. + // If the unresolved property path isn't set yet, just use the property name. if (_propertyPath == null) { _propertyPath = propName; From 6bb0a10717eb97ac66d5560e00f3b6d001cb638a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Tue, 12 Aug 2025 17:07:26 +0200 Subject: [PATCH 2/3] Add data required for failing on 5.3.16 --- .../Async/NHSpecificTest/GH3609/Fixture.cs | 93 +++++++++++++++++++ .../NHSpecificTest/GH3609/Fixture.cs | 11 ++- 2 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 src/NHibernate.Test/Async/NHSpecificTest/GH3609/Fixture.cs diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH3609/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH3609/Fixture.cs new file mode 100644 index 00000000000..b84e7663bd0 --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH3609/Fixture.cs @@ -0,0 +1,93 @@ +//------------------------------------------------------------------------------ +// +// 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; +using System.Linq; +using NUnit.Framework; +using NHibernate.Linq; + +namespace NHibernate.Test.NHSpecificTest.GH3609 +{ + using System.Threading.Tasks; + [TestFixture] + public class FixtureAsync : BugTestCase + { + protected override void OnSetUp() + { + using var session = OpenSession(); + using var transaction = session.BeginTransaction(); + + var order = new Order + { + UniqueId = "0ab92479-8a17-4dbc-9bef-ce4344940cec", + CreatedDate = new DateTime(2024, 09, 24) + }; + session.Save(order); + session.Save(new LineItem { Order = order, ItemName = "Bananas", Amount = 5 }); + + order = new Order + { + UniqueId = "4ca17d84-97aa-489f-8701-302a3879a388", + CreatedDate = new DateTime(2021, 09, 19) + }; + session.Save(order); + session.Save(new LineItem { Order = order, ItemName = "Apples", Amount = 10 }); + + transaction.Commit(); + } + + protected override void OnTearDown() + { + using var session = OpenSession(); + using var transaction = session.BeginTransaction(); + + session.CreateQuery("delete from System.Object").ExecuteUpdate(); + + transaction.Commit(); + } + + [Test] + public async Task QueryWithAnyAsync() + { + using var session = OpenSession(); + using var transaction = session.BeginTransaction(); + + // This form of query is how we first discovered the issue. This is a simplified reproduction of the + // sort of Linq that we were using in our app. It seems to occur when we force an EXISTS( ... ) subquery. + var validOrders = session.Query().Where(x => x.CreatedDate > new DateTime(2024, 9, 10)); + var orderCount = await (session.Query().CountAsync(x => validOrders.Any(y => y == x.Order))); + + Assert.That(orderCount, Is.EqualTo(1)); + } + + [Test] + public async Task QueryWithContainsAsync() + { + using var session = OpenSession(); + using var transaction = session.BeginTransaction(); + + var validOrders = session.Query().Where(x => x.CreatedDate > new DateTime(2024, 9, 10)); + var orderCount = await (session.Query().CountAsync(x => validOrders.Contains(x.Order))); + + Assert.That(orderCount, Is.EqualTo(1)); + } + + [Test] + public async Task SimpleQueryForDataWhichWasInsertedViaAdoShouldProvideExpectedResultsAsync() + { + using var session = OpenSession(); + using var transaction = session.BeginTransaction(); + + // This style of equivalent query does not exhibit the problem. This test passes no matter which NH version. + var lineItem = await (session.Query().FirstOrDefaultAsync(x => x.Order.CreatedDate > new DateTime(2024, 9, 10))); + Assert.That(lineItem, Is.Not.Null); + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH3609/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH3609/Fixture.cs index 956b9de55ec..3f3e68dfdc8 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH3609/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/GH3609/Fixture.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Linq; using NUnit.Framework; @@ -18,9 +18,16 @@ protected override void OnSetUp() CreatedDate = new DateTime(2024, 09, 24) }; session.Save(order); - session.Save(new LineItem { Order = order, ItemName = "Bananas", Amount = 5 }); + order = new Order + { + UniqueId = "4ca17d84-97aa-489f-8701-302a3879a388", + CreatedDate = new DateTime(2021, 09, 19) + }; + session.Save(order); + session.Save(new LineItem { Order = order, ItemName = "Apples", Amount = 10 }); + transaction.Commit(); } From 294d207d7eb67b09b45ed3fb51b1f54c76792718 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Thu, 14 Aug 2025 17:25:27 +0200 Subject: [PATCH 3/3] Fix #3609 And fix NH-892 partly broken by the way. --- .../Async/NHSpecificTest/GH3609/Fixture.cs | 29 ++++++++++++++++--- .../NHSpecificTest/GH3609/Entities.cs | 13 ++++++++- .../NHSpecificTest/GH3609/Fixture.cs | 29 ++++++++++++++++--- .../NHSpecificTest/GH3609/Mappings.hbm.xml | 15 ++++++++-- src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs | 19 ++++++++---- 5 files changed, 87 insertions(+), 18 deletions(-) diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH3609/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH3609/Fixture.cs index b84e7663bd0..55554bf75f6 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/GH3609/Fixture.cs +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH3609/Fixture.cs @@ -31,6 +31,7 @@ protected override void OnSetUp() }; session.Save(order); session.Save(new LineItem { Order = order, ItemName = "Bananas", Amount = 5 }); + session.Save(new CleanLineItem { Order = order, ItemName = "Bananas", Amount = 5 }); order = new Order { @@ -39,6 +40,7 @@ protected override void OnSetUp() }; session.Save(order); session.Save(new LineItem { Order = order, ItemName = "Apples", Amount = 10 }); + session.Save(new CleanLineItem { Order = order, ItemName = "Apples", Amount = 10 }); transaction.Commit(); } @@ -48,6 +50,7 @@ protected override void OnTearDown() using var session = OpenSession(); using var transaction = session.BeginTransaction(); + session.CreateQuery("delete from CleanLineItem").ExecuteUpdate(); session.CreateQuery("delete from System.Object").ExecuteUpdate(); transaction.Commit(); @@ -63,10 +66,26 @@ public async Task QueryWithAnyAsync() // sort of Linq that we were using in our app. It seems to occur when we force an EXISTS( ... ) subquery. var validOrders = session.Query().Where(x => x.CreatedDate > new DateTime(2024, 9, 10)); var orderCount = await (session.Query().CountAsync(x => validOrders.Any(y => y == x.Order))); - + + Assert.That(orderCount, Is.EqualTo(1)); + await (transaction.CommitAsync()); + } + + [Test] + public async Task QueryWithAnyOnCleanLinesAsync() + { + using var session = OpenSession(); + using var transaction = session.BeginTransaction(); + + // This form of query is how we first discovered the issue. This is a simplified reproduction of the + // sort of Linq that we were using in our app. It seems to occur when we force an EXISTS( ... ) subquery. + var validOrders = session.Query().Where(x => x.CreatedDate > new DateTime(2024, 9, 10)); + var orderCount = await (session.Query().CountAsync(x => validOrders.Any(y => y == x.Order))); + Assert.That(orderCount, Is.EqualTo(1)); + await (transaction.CommitAsync()); } - + [Test] public async Task QueryWithContainsAsync() { @@ -75,10 +94,11 @@ public async Task QueryWithContainsAsync() var validOrders = session.Query().Where(x => x.CreatedDate > new DateTime(2024, 9, 10)); var orderCount = await (session.Query().CountAsync(x => validOrders.Contains(x.Order))); - + Assert.That(orderCount, Is.EqualTo(1)); + await (transaction.CommitAsync()); } - + [Test] public async Task SimpleQueryForDataWhichWasInsertedViaAdoShouldProvideExpectedResultsAsync() { @@ -88,6 +108,7 @@ public async Task SimpleQueryForDataWhichWasInsertedViaAdoShouldProvideExpectedR // This style of equivalent query does not exhibit the problem. This test passes no matter which NH version. var lineItem = await (session.Query().FirstOrDefaultAsync(x => x.Order.CreatedDate > new DateTime(2024, 9, 10))); Assert.That(lineItem, Is.Not.Null); + await (transaction.CommitAsync()); } } } diff --git a/src/NHibernate.Test/NHSpecificTest/GH3609/Entities.cs b/src/NHibernate.Test/NHSpecificTest/GH3609/Entities.cs index 28857618b4f..79489468534 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH3609/Entities.cs +++ b/src/NHibernate.Test/NHSpecificTest/GH3609/Entities.cs @@ -1,4 +1,4 @@ -using System; +using System; namespace NHibernate.Test.NHSpecificTest.GH3609 { @@ -21,4 +21,15 @@ public class LineItem public virtual decimal Amount { get; set; } } + + public class CleanLineItem + { + public virtual long Id { get; set; } + + public virtual Order Order { get; set; } + + public virtual string ItemName { get; set; } + + public virtual decimal Amount { get; set; } + } } diff --git a/src/NHibernate.Test/NHSpecificTest/GH3609/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH3609/Fixture.cs index 3f3e68dfdc8..57a0a8053dd 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH3609/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/GH3609/Fixture.cs @@ -19,6 +19,7 @@ protected override void OnSetUp() }; session.Save(order); session.Save(new LineItem { Order = order, ItemName = "Bananas", Amount = 5 }); + session.Save(new CleanLineItem { Order = order, ItemName = "Bananas", Amount = 5 }); order = new Order { @@ -27,6 +28,7 @@ protected override void OnSetUp() }; session.Save(order); session.Save(new LineItem { Order = order, ItemName = "Apples", Amount = 10 }); + session.Save(new CleanLineItem { Order = order, ItemName = "Apples", Amount = 10 }); transaction.Commit(); } @@ -36,6 +38,7 @@ protected override void OnTearDown() using var session = OpenSession(); using var transaction = session.BeginTransaction(); + session.CreateQuery("delete from CleanLineItem").ExecuteUpdate(); session.CreateQuery("delete from System.Object").ExecuteUpdate(); transaction.Commit(); @@ -51,10 +54,26 @@ public void QueryWithAny() // sort of Linq that we were using in our app. It seems to occur when we force an EXISTS( ... ) subquery. var validOrders = session.Query().Where(x => x.CreatedDate > new DateTime(2024, 9, 10)); var orderCount = session.Query().Count(x => validOrders.Any(y => y == x.Order)); - + + Assert.That(orderCount, Is.EqualTo(1)); + transaction.Commit(); + } + + [Test] + public void QueryWithAnyOnCleanLines() + { + using var session = OpenSession(); + using var transaction = session.BeginTransaction(); + + // This form of query is how we first discovered the issue. This is a simplified reproduction of the + // sort of Linq that we were using in our app. It seems to occur when we force an EXISTS( ... ) subquery. + var validOrders = session.Query().Where(x => x.CreatedDate > new DateTime(2024, 9, 10)); + var orderCount = session.Query().Count(x => validOrders.Any(y => y == x.Order)); + Assert.That(orderCount, Is.EqualTo(1)); + transaction.Commit(); } - + [Test] public void QueryWithContains() { @@ -63,10 +82,11 @@ public void QueryWithContains() var validOrders = session.Query().Where(x => x.CreatedDate > new DateTime(2024, 9, 10)); var orderCount = session.Query().Count(x => validOrders.Contains(x.Order)); - + Assert.That(orderCount, Is.EqualTo(1)); + transaction.Commit(); } - + [Test] public void SimpleQueryForDataWhichWasInsertedViaAdoShouldProvideExpectedResults() { @@ -76,6 +96,7 @@ public void SimpleQueryForDataWhichWasInsertedViaAdoShouldProvideExpectedResults // This style of equivalent query does not exhibit the problem. This test passes no matter which NH version. var lineItem = session.Query().FirstOrDefault(x => x.Order.CreatedDate > new DateTime(2024, 9, 10)); Assert.That(lineItem, Is.Not.Null); + transaction.Commit(); } } } diff --git a/src/NHibernate.Test/NHSpecificTest/GH3609/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/GH3609/Mappings.hbm.xml index e21dd4871b6..43191f32951 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH3609/Mappings.hbm.xml +++ b/src/NHibernate.Test/NHSpecificTest/GH3609/Mappings.hbm.xml @@ -1,13 +1,13 @@ - + - + - + @@ -18,4 +18,13 @@ column="OrderId" /> + + + + + + + diff --git a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs index e05b1600485..a7d416845dc 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs @@ -397,10 +397,14 @@ 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 = entityType.IsNullable && Walker.IsComparativeExpressionClause && !IsCorrelatedSubselect; + // For nullable entity comparisons we always need to add join (like not constrained one-to-one or not-found ignore associations). + var comparisonWithNullableEntity = Walker.IsComparativeExpressionClause && entityType.IsNullable; + // For property-ref association comparison, we also need to join unless finding a way in the node for the other hand of the comparison + // to detect it should yield the property-ref columns instead of the primary key columns. And if the other hand is an association too, + // it may be a reference to the primary key, so we would need to join anyway. + var comparisonThroughPropertyRef = Walker.IsComparativeExpressionClause && !entityType.IsReferenceToPrimaryKey; - if ( IsDotNode( parent ) ) + if (IsDotNode(parent)) { // our parent is another dot node, meaning we are being further dereferenced. // thus we need to generate a join unless the parent refers to the associated @@ -421,15 +425,18 @@ private void DereferenceEntity(EntityType entityType, bool implicitJoin, string else { joinIsNeeded = generateJoin || (Walker.IsInSelect && !Walker.IsInCase) || (Walker.IsInFrom && !Walker.IsComparativeExpressionClause) - || comparisonWithNullableEntity; + || comparisonWithNullableEntity || comparisonThroughPropertyRef; } if ( joinIsNeeded ) { - DereferenceEntityJoin(classAlias, entityType, implicitJoin, parent, comparisonWithNullableEntity); - if (comparisonWithNullableEntity) + // Subselect queries use theta style joins, which cannot be forced to left outer joins. + var forceLeftJoin = comparisonWithNullableEntity && !IsCorrelatedSubselect; + DereferenceEntityJoin(classAlias, entityType, implicitJoin, parent, forceLeftJoin); + if (comparisonWithNullableEntity || comparisonThroughPropertyRef) { _columns = FromElement.GetIdentityColumns(); + DataType = FromElement.EntityPersister.EntityMetamodel.EntityType; } } else