From d408649f6f17d19b07313b9ea03be849dc2d6db0 Mon Sep 17 00:00:00 2001 From: Caner Olgun Kalyoncu Date: Tue, 27 Sep 2022 16:53:20 +0300 Subject: [PATCH 1/9] NH3069 HQL query plan cache problem resolved --- .../Async/Linq/ConstantTest.cs | 81 +++++++++++++++++++ .../Linq/Visitors/SelectClauseNominator.cs | 31 ++++++- 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/src/NHibernate.Test/Async/Linq/ConstantTest.cs b/src/NHibernate.Test/Async/Linq/ConstantTest.cs index 9c2558a6902..cd06d017a3d 100644 --- a/src/NHibernate.Test/Async/Linq/ConstantTest.cs +++ b/src/NHibernate.Test/Async/Linq/ConstantTest.cs @@ -398,5 +398,86 @@ public async Task DmlPlansForExpandedQueryAsync() "Query plans should either be cached separately or not cached at all."); } } + + [Test] + public async Task PlansWithConstantExpressionsAreNotCachedAsync() + { + var queryPlanCacheType = typeof(QueryPlanCache); + var cache = (SoftLimitMRUCache)queryPlanCacheType.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(Sfi.QueryPlanCache); + cache.Clear(); + + await ((from c in db.Customers + where c.CustomerId == "ALFKI" + select new { c.CustomerId, c.ContactName, DummyBooleanColumn = c.CustomerId == "ALFKI" }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(1), "Query should be cached"); + + await ((from c in db.Customers + where c.CustomerId == "ALFKI" + select new { c.CustomerId, c.ContactName, DummyBooleanColumn = c.CustomerId != "ALFKI" }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(2), "Query should be cached"); + + await ((from p in db.Products + select new { p.Name, DummyBooleanColumn = p.Discontinued && true }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(3), "Query should be cached"); + + await ((from p in db.Products + select new { p.Name, DummyBooleanColumn = p.Discontinued || true }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(4), "Query should be cached"); + + await ((from p in db.Products + select new { p.Name, DummyBooleanColumn = p.ShippingWeight > 10 }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(5), "Query should be cached"); + + await ((from p in db.Products + select new { p.Name, DummyBooleanColumn = p.ShippingWeight < 10 }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(6), "Query should be cached"); + + await ((from p in db.Products + select new { p.Name, DummyBooleanColumn = p.ShippingWeight >= 10 }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(7), "Query should be cached"); + + await ((from p in db.Products + select new { p.Name, DummyBooleanColumn = p.ShippingWeight <= 10 }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(8), "Query should be cached"); + + await ((from p in db.Products + select new { p.Name, DummyColumn = p.ShippingWeight > 0 ? "Test" : null }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(9), "Query should be cached"); + + await ((from p in db.Products + select new { p.Name, DummyColumn = p.ShippingWeight + 10 }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(10), "Query should be cached"); + + await ((from p in db.Products + select new { p.Name, DummyColumn = p.ShippingWeight - 10 }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(11), "Query should be cached"); + + await ((from p in db.Products + select new { p.Name, DummyColumn = p.ShippingWeight * 10 }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(12), "Query should be cached"); + + await ((from p in db.Products + select new { p.Name, DummyColumn = p.ShippingWeight / 10 }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(13), "Query should be cached"); + + await ((from c in db.Customers + where c.CustomerId == "ALFKI" + select new { c.CustomerId, c.ContactName, DummyColumn = c.CustomerId ?? "TEST" }).FirstAsync()); + + Assert.That(cache, Has.Count.EqualTo(14), "Query should be cached"); + } } } diff --git a/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs b/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs index 52bf954101d..74bf0d90270 100644 --- a/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs +++ b/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs @@ -67,7 +67,8 @@ public override Expression Visit(Expression expression) return innerExpression; } - var projectConstantsInHql = _stateStack.Peek() || expression.NodeType == ExpressionType.Equal || IsRegisteredFunction(expression); + //var projectConstantsInHql = _stateStack.Peek() || expression.NodeType == ExpressionType.Equal || IsRegisteredFunction(expression); + var projectConstantsInHql = _stateStack.Peek() || IsConstantExpression(expression) || IsRegisteredFunction(expression); // Set some flags, unless we already have proper values for them: // projectConstantsInHql if they are inside a method call executed server side. @@ -115,6 +116,34 @@ public override Expression Visit(Expression expression) return expression; } + private bool IsConstantExpression(Expression expression) + { + switch (expression.NodeType) + { + case ExpressionType.Equal: + case ExpressionType.NotEqual: + case ExpressionType.LessThan: + case ExpressionType.LessThanOrEqual: + case ExpressionType.GreaterThan: + case ExpressionType.GreaterThanOrEqual: + case ExpressionType.Not: + case ExpressionType.And: + case ExpressionType.AndAlso: + case ExpressionType.Or: + case ExpressionType.OrElse: + case ExpressionType.Add: + case ExpressionType.Subtract: + case ExpressionType.Multiply: + case ExpressionType.Divide: + case ExpressionType.Modulo: + case ExpressionType.Coalesce: + case ExpressionType.Conditional: + return true; + default: + return false; + } + } + private bool IsRegisteredFunction(Expression expression) { if (expression.NodeType == ExpressionType.Call) From caff0be4fe3af113d54932678082969f872888e8 Mon Sep 17 00:00:00 2001 From: Caner Olgun Kalyoncu Date: Tue, 27 Sep 2022 17:13:25 +0300 Subject: [PATCH 2/9] Commented line removed --- src/NHibernate/Linq/Visitors/SelectClauseNominator.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs b/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs index 74bf0d90270..9cda834f34a 100644 --- a/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs +++ b/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs @@ -67,7 +67,6 @@ public override Expression Visit(Expression expression) return innerExpression; } - //var projectConstantsInHql = _stateStack.Peek() || expression.NodeType == ExpressionType.Equal || IsRegisteredFunction(expression); var projectConstantsInHql = _stateStack.Peek() || IsConstantExpression(expression) || IsRegisteredFunction(expression); // Set some flags, unless we already have proper values for them: From 90498b9fd4cf1f1145f554e6f263319bd019466a Mon Sep 17 00:00:00 2001 From: Caner Olgun Kalyoncu Date: Tue, 27 Sep 2022 17:37:12 +0300 Subject: [PATCH 3/9] Sync unit tests added --- src/NHibernate.Test/Linq/ConstantTest.cs | 81 ++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/src/NHibernate.Test/Linq/ConstantTest.cs b/src/NHibernate.Test/Linq/ConstantTest.cs index 74b20f13afb..2c6ee599345 100644 --- a/src/NHibernate.Test/Linq/ConstantTest.cs +++ b/src/NHibernate.Test/Linq/ConstantTest.cs @@ -359,6 +359,87 @@ public void PlansWithNonParameterizedConstantsAreNotCached() "Query plan should not be cached."); } + [Test] + public void PlansWithConstantExpressionsAreNotCached() + { + var queryPlanCacheType = typeof(QueryPlanCache); + var cache = (SoftLimitMRUCache) queryPlanCacheType.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(Sfi.QueryPlanCache); + cache.Clear(); + + (from c in db.Customers + where c.CustomerId == "ALFKI" + select new { c.CustomerId, c.ContactName, DummyBooleanColumn = c.CustomerId == "ALFKI" }).First(); + + Assert.That(cache, Has.Count.EqualTo(1), "Query should be cached"); + + (from c in db.Customers + where c.CustomerId == "ALFKI" + select new { c.CustomerId, c.ContactName, DummyBooleanColumn = c.CustomerId != "ALFKI" }).First(); + + Assert.That(cache, Has.Count.EqualTo(2), "Query should be cached"); + + (from p in db.Products + select new { p.Name, DummyBooleanColumn = p.Discontinued && true }).First(); + + Assert.That(cache, Has.Count.EqualTo(3), "Query should be cached"); + + (from p in db.Products + select new { p.Name, DummyBooleanColumn = p.Discontinued || true }).First(); + + Assert.That(cache, Has.Count.EqualTo(4), "Query should be cached"); + + (from p in db.Products + select new { p.Name, DummyBooleanColumn = p.ShippingWeight > 10 }).First(); + + Assert.That(cache, Has.Count.EqualTo(5), "Query should be cached"); + + (from p in db.Products + select new { p.Name, DummyBooleanColumn = p.ShippingWeight < 10 }).First(); + + Assert.That(cache, Has.Count.EqualTo(6), "Query should be cached"); + + (from p in db.Products + select new { p.Name, DummyBooleanColumn = p.ShippingWeight >= 10 }).First(); + + Assert.That(cache, Has.Count.EqualTo(7), "Query should be cached"); + + (from p in db.Products + select new { p.Name, DummyBooleanColumn = p.ShippingWeight <= 10 }).First(); + + Assert.That(cache, Has.Count.EqualTo(8), "Query should be cached"); + + (from p in db.Products + select new { p.Name, DummyColumn = p.ShippingWeight > 0 ? "Test" : null }).First(); + + Assert.That(cache, Has.Count.EqualTo(9), "Query should be cached"); + + (from p in db.Products + select new { p.Name, DummyColumn = p.ShippingWeight + 10 }).First(); + + Assert.That(cache, Has.Count.EqualTo(10), "Query should be cached"); + + (from p in db.Products + select new { p.Name, DummyColumn = p.ShippingWeight - 10 }).First(); + + Assert.That(cache, Has.Count.EqualTo(11), "Query should be cached"); + + (from p in db.Products + select new { p.Name, DummyColumn = p.ShippingWeight * 10 }).First(); + + Assert.That(cache, Has.Count.EqualTo(12), "Query should be cached"); + + (from p in db.Products + select new { p.Name, DummyColumn = p.ShippingWeight / 10 }).First(); + + Assert.That(cache, Has.Count.EqualTo(13), "Query should be cached"); + + (from c in db.Customers + where c.CustomerId == "ALFKI" + select new { c.CustomerId, c.ContactName, DummyColumn = c.CustomerId ?? "TEST" }).First(); + + Assert.That(cache, Has.Count.EqualTo(14), "Query should be cached"); + } + [Test] public void PlansWithNonParameterizedConstantsAreNotCachedForExpandedQuery() { From 7f22e2430e536f61e01e3ff5a35f05c27b4d6da6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 27 Sep 2022 14:45:33 +0000 Subject: [PATCH 4/9] Generate async files --- .../Async/Linq/ConstantTest.cs | 116 +++++++++--------- 1 file changed, 58 insertions(+), 58 deletions(-) diff --git a/src/NHibernate.Test/Async/Linq/ConstantTest.cs b/src/NHibernate.Test/Async/Linq/ConstantTest.cs index cd06d017a3d..ed3856010c3 100644 --- a/src/NHibernate.Test/Async/Linq/ConstantTest.cs +++ b/src/NHibernate.Test/Async/Linq/ConstantTest.cs @@ -342,68 +342,11 @@ public async Task PlansWithNonParameterizedConstantsAreNotCachedAsync() "Query plan should not be cached."); } - [Test] - public async Task PlansWithNonParameterizedConstantsAreNotCachedForExpandedQueryAsync() - { - var queryPlanCacheType = typeof(QueryPlanCache); - - var cache = (SoftLimitMRUCache) - queryPlanCacheType - .GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic) - .GetValue(Sfi.QueryPlanCache); - cache.Clear(); - - var ids = new[] {"ANATR", "UNKNOWN"}.ToList(); - await (db.Customers.Where(x => ids.Contains(x.CustomerId)).Select( - c => new {c.CustomerId, c.ContactName, Constant = 1}).FirstAsync()); - - Assert.That( - cache, - Has.Count.EqualTo(0), - "Query plan should not be cached."); - } - - //GH-2298 - Different Update queries - same query cache plan - [Test] - public async Task DmlPlansForExpandedQueryAsync() - { - var queryPlanCacheType = typeof(QueryPlanCache); - - var cache = (SoftLimitMRUCache) - queryPlanCacheType - .GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic) - .GetValue(Sfi.QueryPlanCache); - cache.Clear(); - - using (session.BeginTransaction()) - { - var list = new[] {"UNKNOWN", "UNKNOWN2"}.ToList(); - await (db.Customers.Where(x => list.Contains(x.CustomerId)).UpdateAsync( - x => new Customer - { - CompanyName = "Constant1" - })); - - await (db.Customers.Where(x => list.Contains(x.CustomerId)) - .UpdateAsync( - x => new Customer - { - ContactName = "Constant1" - })); - - Assert.That( - cache.Count, - //2 original queries + 2 expanded queries are expected in cache - Is.EqualTo(0).Or.EqualTo(4), - "Query plans should either be cached separately or not cached at all."); - } - } - [Test] public async Task PlansWithConstantExpressionsAreNotCachedAsync() { var queryPlanCacheType = typeof(QueryPlanCache); - var cache = (SoftLimitMRUCache)queryPlanCacheType.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(Sfi.QueryPlanCache); + var cache = (SoftLimitMRUCache) queryPlanCacheType.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(Sfi.QueryPlanCache); cache.Clear(); await ((from c in db.Customers @@ -479,5 +422,62 @@ public async Task PlansWithConstantExpressionsAreNotCachedAsync() Assert.That(cache, Has.Count.EqualTo(14), "Query should be cached"); } + + [Test] + public async Task PlansWithNonParameterizedConstantsAreNotCachedForExpandedQueryAsync() + { + var queryPlanCacheType = typeof(QueryPlanCache); + + var cache = (SoftLimitMRUCache) + queryPlanCacheType + .GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic) + .GetValue(Sfi.QueryPlanCache); + cache.Clear(); + + var ids = new[] {"ANATR", "UNKNOWN"}.ToList(); + await (db.Customers.Where(x => ids.Contains(x.CustomerId)).Select( + c => new {c.CustomerId, c.ContactName, Constant = 1}).FirstAsync()); + + Assert.That( + cache, + Has.Count.EqualTo(0), + "Query plan should not be cached."); + } + + //GH-2298 - Different Update queries - same query cache plan + [Test] + public async Task DmlPlansForExpandedQueryAsync() + { + var queryPlanCacheType = typeof(QueryPlanCache); + + var cache = (SoftLimitMRUCache) + queryPlanCacheType + .GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic) + .GetValue(Sfi.QueryPlanCache); + cache.Clear(); + + using (session.BeginTransaction()) + { + var list = new[] {"UNKNOWN", "UNKNOWN2"}.ToList(); + await (db.Customers.Where(x => list.Contains(x.CustomerId)).UpdateAsync( + x => new Customer + { + CompanyName = "Constant1" + })); + + await (db.Customers.Where(x => list.Contains(x.CustomerId)) + .UpdateAsync( + x => new Customer + { + ContactName = "Constant1" + })); + + Assert.That( + cache.Count, + //2 original queries + 2 expanded queries are expected in cache + Is.EqualTo(0).Or.EqualTo(4), + "Query plans should either be cached separately or not cached at all."); + } + } } } From 7055f7e8a27630af1b81086151fb1b50362ffa06 Mon Sep 17 00:00:00 2001 From: Gokhan Abatay Date: Thu, 29 Sep 2022 02:03:56 +0300 Subject: [PATCH 5/9] SelectClauseNominator constant improvment --- .../Async/Linq/ConstantTest.cs | 9 ++++-- src/NHibernate.Test/Linq/ConstantTest.cs | 10 +++--- .../Linq/Visitors/SelectClauseNominator.cs | 32 ++++++++++++++----- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/NHibernate.Test/Async/Linq/ConstantTest.cs b/src/NHibernate.Test/Async/Linq/ConstantTest.cs index ed3856010c3..4a5b4a3797b 100644 --- a/src/NHibernate.Test/Async/Linq/ConstantTest.cs +++ b/src/NHibernate.Test/Async/Linq/ConstantTest.cs @@ -23,6 +23,8 @@ namespace NHibernate.Test.Linq { using System.Threading.Tasks; using System.Threading; + using System; + // Mainly adapted from tests contributed by Nicola Tuveri on NH-2500 (NH-2500.patch file) [TestFixture] public class ConstantTestAsync : LinqTestCase @@ -349,9 +351,10 @@ public async Task PlansWithConstantExpressionsAreNotCachedAsync() var cache = (SoftLimitMRUCache) queryPlanCacheType.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(Sfi.QueryPlanCache); cache.Clear(); + var input = new { Name = "ALFKI" }; await ((from c in db.Customers where c.CustomerId == "ALFKI" - select new { c.CustomerId, c.ContactName, DummyBooleanColumn = c.CustomerId == "ALFKI" }).FirstAsync()); + select new { c.CustomerId, c.ContactName, DummyBooleanColumn = c.CustomerId == input.Name }).FirstAsync()); Assert.That(cache, Has.Count.EqualTo(1), "Query should be cached"); @@ -392,12 +395,12 @@ public async Task PlansWithConstantExpressionsAreNotCachedAsync() Assert.That(cache, Has.Count.EqualTo(8), "Query should be cached"); await ((from p in db.Products - select new { p.Name, DummyColumn = p.ShippingWeight > 0 ? "Test" : null }).FirstAsync()); + select new { p.Name, DummyColumn = p.ShippingWeight > 0 ? DateTime.Now : default(DateTime?) }).FirstAsync()); Assert.That(cache, Has.Count.EqualTo(9), "Query should be cached"); await ((from p in db.Products - select new { p.Name, DummyColumn = p.ShippingWeight + 10 }).FirstAsync()); + select new { p.Name, DummyColumn = p.UnitPrice + 10 }).FirstAsync()); Assert.That(cache, Has.Count.EqualTo(10), "Query should be cached"); diff --git a/src/NHibernate.Test/Linq/ConstantTest.cs b/src/NHibernate.Test/Linq/ConstantTest.cs index 2c6ee599345..0767e0a5134 100644 --- a/src/NHibernate.Test/Linq/ConstantTest.cs +++ b/src/NHibernate.Test/Linq/ConstantTest.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; using System.Reflection; using NHibernate.Criterion; @@ -366,9 +367,10 @@ public void PlansWithConstantExpressionsAreNotCached() var cache = (SoftLimitMRUCache) queryPlanCacheType.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(Sfi.QueryPlanCache); cache.Clear(); + var input = new { Name = "ALFKI" }; (from c in db.Customers where c.CustomerId == "ALFKI" - select new { c.CustomerId, c.ContactName, DummyBooleanColumn = c.CustomerId == "ALFKI" }).First(); + select new { c.CustomerId, c.ContactName, DummyBooleanColumn = c.CustomerId == input.Name }).First(); Assert.That(cache, Has.Count.EqualTo(1), "Query should be cached"); @@ -409,12 +411,12 @@ public void PlansWithConstantExpressionsAreNotCached() Assert.That(cache, Has.Count.EqualTo(8), "Query should be cached"); (from p in db.Products - select new { p.Name, DummyColumn = p.ShippingWeight > 0 ? "Test" : null }).First(); + select new { p.Name, DummyColumn = p.ShippingWeight > 0 ? DateTime.Now : default(DateTime?) }).First(); Assert.That(cache, Has.Count.EqualTo(9), "Query should be cached"); (from p in db.Products - select new { p.Name, DummyColumn = p.ShippingWeight + 10 }).First(); + select new { p.Name, DummyColumn = p.UnitPrice + 10 }).First(); Assert.That(cache, Has.Count.EqualTo(10), "Query should be cached"); diff --git a/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs b/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs index 9cda834f34a..989bb55da73 100644 --- a/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs +++ b/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs @@ -67,15 +67,14 @@ public override Expression Visit(Expression expression) return innerExpression; } - var projectConstantsInHql = _stateStack.Peek() || IsConstantExpression(expression) || IsRegisteredFunction(expression); + bool isRegisteredFunction = IsRegisteredFunction(expression); + var projectConstantsInHql = _stateStack.Peek() || IsConstantExpression(expression) || isRegisteredFunction; // Set some flags, unless we already have proper values for them: // projectConstantsInHql if they are inside a method call executed server side. // ContainsUntranslatedMethodCalls if a method call must be executed locally. - var isMethodCall = expression.NodeType == ExpressionType.Call; - if (isMethodCall && (!projectConstantsInHql || !ContainsUntranslatedMethodCalls)) + if (expression.NodeType == ExpressionType.Call && (!projectConstantsInHql || !ContainsUntranslatedMethodCalls)) { - var isRegisteredFunction = IsRegisteredFunction(expression); projectConstantsInHql = projectConstantsInHql || isRegisteredFunction; ContainsUntranslatedMethodCalls = ContainsUntranslatedMethodCalls || !isRegisteredFunction; } @@ -115,10 +114,21 @@ public override Expression Visit(Expression expression) return expression; } - private bool IsConstantExpression(Expression expression) + private static bool IsAllowedToProjectInHql(System.Type type) + { + return (type.IsValueType || type == typeof(string)) && typeof(DateTime) != type && typeof(DateTime?) != type && typeof(TimeSpan) != type && typeof(TimeSpan?) != type; + } + + private static bool IsConstantExpression(Expression expression) { switch (expression.NodeType) { + case ExpressionType.Constant: + case ExpressionType.Extension: + return true; + case ExpressionType.MemberAccess: + var member = (MemberExpression)expression; + return IsConstantExpression(member.Expression); case ExpressionType.Equal: case ExpressionType.NotEqual: case ExpressionType.LessThan: @@ -130,14 +140,20 @@ private bool IsConstantExpression(Expression expression) case ExpressionType.AndAlso: case ExpressionType.Or: case ExpressionType.OrElse: + return true; case ExpressionType.Add: case ExpressionType.Subtract: case ExpressionType.Multiply: case ExpressionType.Divide: case ExpressionType.Modulo: + var binary = (BinaryExpression) expression; + return IsAllowedToProjectInHql(binary.Left.Type) && IsConstantExpression(binary.Left) && IsAllowedToProjectInHql(binary.Right.Type) && IsConstantExpression(binary.Right); case ExpressionType.Coalesce: + var coalesce = (BinaryExpression) expression; + return IsConstantExpression(coalesce.Left) && IsConstantExpression(coalesce.Right); case ExpressionType.Conditional: - return true; + var conditional = (ConditionalExpression) expression; + return IsConstantExpression(conditional.IfTrue) && IsConstantExpression(conditional.IfFalse); default: return false; } @@ -207,10 +223,10 @@ private bool CanBeEvaluatedInHqlSelectStatement(Expression expression, bool proj return projectConstantsInHql; } - return !(expression is MemberExpression memberExpression) || // Assume all is good + return projectConstantsInHql && (!(expression is MemberExpression memberExpression) || // Assume all is good // Nominate only expressions that represent a mapped property or a translatable method call ExpressionsHelper.TryGetMappedType(_sessionFactory, expression, out _, out _, out _, out _) || - _functionRegistry.TryGetGenerator(memberExpression.Member, out _); + _functionRegistry.TryGetGenerator(memberExpression.Member, out _)); } private static bool CanBeEvaluatedInHqlStatementShortcut(Expression expression) From 484bb1414e76c25314fd5ee206bc7f2873896802 Mon Sep 17 00:00:00 2001 From: Gokhan Abatay Date: Thu, 29 Sep 2022 19:17:34 +0300 Subject: [PATCH 6/9] queries should be cached --- .../Async/Linq/ConstantTest.cs | 6 ++-- src/NHibernate.Test/Linq/ConstantTest.cs | 6 ++-- .../Linq/Visitors/SelectClauseNominator.cs | 31 +++++++++++++------ 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/NHibernate.Test/Async/Linq/ConstantTest.cs b/src/NHibernate.Test/Async/Linq/ConstantTest.cs index 4a5b4a3797b..7256fb920c3 100644 --- a/src/NHibernate.Test/Async/Linq/ConstantTest.cs +++ b/src/NHibernate.Test/Async/Linq/ConstantTest.cs @@ -325,7 +325,7 @@ public async Task DmlPlansAreCachedAsync() } [Test] - public async Task PlansWithNonParameterizedConstantsAreNotCachedAsync() + public async Task PlansWithNonParameterizedConstantsAreCachedAsync() { var queryPlanCacheType = typeof(QueryPlanCache); @@ -340,8 +340,8 @@ public async Task PlansWithNonParameterizedConstantsAreNotCachedAsync() select new { c.CustomerId, c.ContactName, Constant = 1 }).FirstAsync()); Assert.That( cache, - Has.Count.EqualTo(0), - "Query plan should not be cached."); + Has.Count.EqualTo(1), + "Query should be cached."); } [Test] diff --git a/src/NHibernate.Test/Linq/ConstantTest.cs b/src/NHibernate.Test/Linq/ConstantTest.cs index 0767e0a5134..70120af6963 100644 --- a/src/NHibernate.Test/Linq/ConstantTest.cs +++ b/src/NHibernate.Test/Linq/ConstantTest.cs @@ -341,7 +341,7 @@ public void DmlPlansAreCached() } [Test] - public void PlansWithNonParameterizedConstantsAreNotCached() + public void PlansWithNonParameterizedConstantsAreCached() { var queryPlanCacheType = typeof(QueryPlanCache); @@ -356,8 +356,8 @@ public void PlansWithNonParameterizedConstantsAreNotCached() select new { c.CustomerId, c.ContactName, Constant = 1 }).First(); Assert.That( cache, - Has.Count.EqualTo(0), - "Query plan should not be cached."); + Has.Count.EqualTo(1), + "Query should be cached."); } [Test] diff --git a/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs b/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs index 989bb55da73..830c14febf6 100644 --- a/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs +++ b/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs @@ -67,15 +67,14 @@ public override Expression Visit(Expression expression) return innerExpression; } - bool isRegisteredFunction = IsRegisteredFunction(expression); - var projectConstantsInHql = _stateStack.Peek() || IsConstantExpression(expression) || isRegisteredFunction; + var isRegisteredFunction = IsRegisteredFunction(expression); + var projectConstantsInHql = _stateStack.Peek() || IsConstantExpression(expression) || IsRegisteredFunction(expression); // Set some flags, unless we already have proper values for them: // projectConstantsInHql if they are inside a method call executed server side. // ContainsUntranslatedMethodCalls if a method call must be executed locally. if (expression.NodeType == ExpressionType.Call && (!projectConstantsInHql || !ContainsUntranslatedMethodCalls)) { - projectConstantsInHql = projectConstantsInHql || isRegisteredFunction; ContainsUntranslatedMethodCalls = ContainsUntranslatedMethodCalls || !isRegisteredFunction; } @@ -119,15 +118,26 @@ private static bool IsAllowedToProjectInHql(System.Type type) return (type.IsValueType || type == typeof(string)) && typeof(DateTime) != type && typeof(DateTime?) != type && typeof(TimeSpan) != type && typeof(TimeSpan?) != type; } + private static bool IsValueType(System.Type type) + { + return type.IsValueType || type == typeof(string); + } + private static bool IsConstantExpression(Expression expression) { + //if (expression.NodeType != ExpressionType.Equal) return false; + + if(expression == null) return true; + switch (expression.NodeType) { - case ExpressionType.Constant: case ExpressionType.Extension: + case ExpressionType.Convert: return true; + case ExpressionType.Constant: + return IsValueType(expression.Type); case ExpressionType.MemberAccess: - var member = (MemberExpression)expression; + var member = (MemberExpression) expression; return IsConstantExpression(member.Expression); case ExpressionType.Equal: case ExpressionType.NotEqual: @@ -136,24 +146,25 @@ private static bool IsConstantExpression(Expression expression) case ExpressionType.GreaterThan: case ExpressionType.GreaterThanOrEqual: case ExpressionType.Not: + return true; case ExpressionType.And: case ExpressionType.AndAlso: case ExpressionType.Or: case ExpressionType.OrElse: - return true; case ExpressionType.Add: case ExpressionType.Subtract: case ExpressionType.Multiply: case ExpressionType.Divide: case ExpressionType.Modulo: var binary = (BinaryExpression) expression; - return IsAllowedToProjectInHql(binary.Left.Type) && IsConstantExpression(binary.Left) && IsAllowedToProjectInHql(binary.Right.Type) && IsConstantExpression(binary.Right); + return IsAllowedToProjectInHql(binary.Left.Type) && IsConstantExpression(binary.Left) && + IsAllowedToProjectInHql(binary.Right.Type) && IsConstantExpression(binary.Right); case ExpressionType.Coalesce: var coalesce = (BinaryExpression) expression; return IsConstantExpression(coalesce.Left) && IsConstantExpression(coalesce.Right); case ExpressionType.Conditional: var conditional = (ConditionalExpression) expression; - return IsConstantExpression(conditional.IfTrue) && IsConstantExpression(conditional.IfFalse); + return IsConstantExpression(conditional.Test) && IsConstantExpression(conditional.IfTrue) && IsConstantExpression(conditional.IfFalse); default: return false; } @@ -223,10 +234,10 @@ private bool CanBeEvaluatedInHqlSelectStatement(Expression expression, bool proj return projectConstantsInHql; } - return projectConstantsInHql && (!(expression is MemberExpression memberExpression) || // Assume all is good + return !(expression is MemberExpression memberExpression) || // Assume all is good // Nominate only expressions that represent a mapped property or a translatable method call ExpressionsHelper.TryGetMappedType(_sessionFactory, expression, out _, out _, out _, out _) || - _functionRegistry.TryGetGenerator(memberExpression.Member, out _)); + _functionRegistry.TryGetGenerator(memberExpression.Member, out _); } private static bool CanBeEvaluatedInHqlStatementShortcut(Expression expression) From e447a8515682b404d8341273dc73f4de594eb04c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 29 Sep 2022 16:20:12 +0000 Subject: [PATCH 7/9] Generate async files --- src/NHibernate.Test/Async/Linq/ConstantTest.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/NHibernate.Test/Async/Linq/ConstantTest.cs b/src/NHibernate.Test/Async/Linq/ConstantTest.cs index 7256fb920c3..601c5980f07 100644 --- a/src/NHibernate.Test/Async/Linq/ConstantTest.cs +++ b/src/NHibernate.Test/Async/Linq/ConstantTest.cs @@ -8,6 +8,7 @@ //------------------------------------------------------------------------------ +using System; using System.Collections.Generic; using System.Linq; using System.Reflection; @@ -23,8 +24,6 @@ namespace NHibernate.Test.Linq { using System.Threading.Tasks; using System.Threading; - using System; - // Mainly adapted from tests contributed by Nicola Tuveri on NH-2500 (NH-2500.patch file) [TestFixture] public class ConstantTestAsync : LinqTestCase @@ -138,7 +137,7 @@ public async Task ConstantNonCachedInMemberInitExpressionWithConditionAsync() return db.Shippers.Where(o => o.ShipperId == id) .Select(o => new ShipperDto {Number = id, CompanyName = o.CompanyName}).SingleAsync(cancellationToken); } - catch (System.Exception ex) + catch (Exception ex) { return Task.FromException(ex); } @@ -341,7 +340,7 @@ public async Task PlansWithNonParameterizedConstantsAreCachedAsync() Assert.That( cache, Has.Count.EqualTo(1), - "Query should be cached."); + "Query should be cached."); } [Test] From 17721fec87c5d8e7f26a3d3210f3c87e20df5ae9 Mon Sep 17 00:00:00 2001 From: Gokhan Abatay Date: Fri, 30 Sep 2022 03:07:45 +0300 Subject: [PATCH 8/9] imrovments --- .../Linq/Visitors/SelectClauseNominator.cs | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs b/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs index 830c14febf6..2e8fa61e0b2 100644 --- a/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs +++ b/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs @@ -7,6 +7,9 @@ using NHibernate.Param; using NHibernate.Util; using Remotion.Linq.Parsing; +using Remotion.Linq.Clauses.Expressions; +using Remotion.Linq.Clauses; +using NHibernate.Linq.Clauses; namespace NHibernate.Linq.Visitors { @@ -68,7 +71,7 @@ public override Expression Visit(Expression expression) } var isRegisteredFunction = IsRegisteredFunction(expression); - var projectConstantsInHql = _stateStack.Peek() || IsConstantExpression(expression) || IsRegisteredFunction(expression); + var projectConstantsInHql = _stateStack.Peek() || IsConstantExpression(expression) || isRegisteredFunction; // Set some flags, unless we already have proper values for them: // projectConstantsInHql if they are inside a method call executed server side. @@ -94,7 +97,7 @@ public override Expression Visit(Expression expression) if (_canBeCandidate) { - if (CanBeEvaluatedInHqlSelectStatement(expression, projectConstantsInHql)) + if (CanBeEvaluatedInHqlSelectStatement(expression, projectConstantsInHql, isRegisteredFunction)) { HqlCandidates.Add(expression); } @@ -132,21 +135,32 @@ private static bool IsConstantExpression(Expression expression) switch (expression.NodeType) { case ExpressionType.Extension: + if( expression is QuerySourceReferenceExpression extension && + (extension.ReferencedQuerySource is NhClauseBase || + (extension.ReferencedQuerySource is MainFromClause fromClause && + fromClause.FromExpression.Type.IsGenericType && + fromClause.FromExpression.Type.GetGenericTypeDefinition() == typeof(NhQueryable<>)))) + { + return true; + } + return false; case ExpressionType.Convert: - return true; + var convert = (UnaryExpression) expression; + return convert.Method == null && IsAllowedToProjectInHql(convert.Operand.Type) && IsConstantExpression(convert.Operand); case ExpressionType.Constant: - return IsValueType(expression.Type); + var constant = (ConstantExpression) expression; + return constant.Value == null || IsValueType(expression.Type); case ExpressionType.MemberAccess: var member = (MemberExpression) expression; return IsConstantExpression(member.Expression); case ExpressionType.Equal: case ExpressionType.NotEqual: + case ExpressionType.Not: + return true; case ExpressionType.LessThan: case ExpressionType.LessThanOrEqual: case ExpressionType.GreaterThan: case ExpressionType.GreaterThanOrEqual: - case ExpressionType.Not: - return true; case ExpressionType.And: case ExpressionType.AndAlso: case ExpressionType.Or: @@ -164,7 +178,9 @@ private static bool IsConstantExpression(Expression expression) return IsConstantExpression(coalesce.Left) && IsConstantExpression(coalesce.Right); case ExpressionType.Conditional: var conditional = (ConditionalExpression) expression; - return IsConstantExpression(conditional.Test) && IsConstantExpression(conditional.IfTrue) && IsConstantExpression(conditional.IfFalse); + return IsConstantExpression(conditional.Test) && + IsAllowedToProjectInHql(conditional.IfTrue.Type) && IsConstantExpression(conditional.IfTrue) && + IsAllowedToProjectInHql(conditional.IfFalse.Type) && IsConstantExpression(conditional.IfFalse); default: return false; } @@ -196,7 +212,7 @@ expression is NhMaxExpression || return false; } - private bool CanBeEvaluatedInHqlSelectStatement(Expression expression, bool projectConstantsInHql) + private bool CanBeEvaluatedInHqlSelectStatement(Expression expression, bool projectConstantsInHql, bool isRegisteredFunction) { // HQL can't do New or Member Init if (expression.NodeType == ExpressionType.MemberInit || @@ -221,7 +237,7 @@ private bool CanBeEvaluatedInHqlSelectStatement(Expression expression, bool proj if (expression.NodeType == ExpressionType.Call) { // Depends if it's in the function registry - return IsRegisteredFunction(expression); + return isRegisteredFunction; } if (expression.NodeType == ExpressionType.Conditional) From b474a486dd05dead50b3f4c6291fe245e2386311 Mon Sep 17 00:00:00 2001 From: Gokhan Abatay Date: Fri, 30 Sep 2022 10:34:16 +0300 Subject: [PATCH 9/9] fix for some failed cases --- .../Linq/Visitors/SelectClauseNominator.cs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs b/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs index 2e8fa61e0b2..1bd2a22a87c 100644 --- a/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs +++ b/src/NHibernate/Linq/Visitors/SelectClauseNominator.cs @@ -52,7 +52,7 @@ internal Expression Nominate(Expression expression) ContainsUntranslatedMethodCalls = false; _canBeCandidate = true; _stateStack = new Stack(); - _stateStack.Push(false); + _stateStack.Push(true); return Visit(expression); } @@ -71,7 +71,7 @@ public override Expression Visit(Expression expression) } var isRegisteredFunction = IsRegisteredFunction(expression); - var projectConstantsInHql = _stateStack.Peek() || IsConstantExpression(expression) || isRegisteredFunction; + var projectConstantsInHql = (_stateStack.Peek() && IsConstantExpression(expression)) || isRegisteredFunction; // Set some flags, unless we already have proper values for them: // projectConstantsInHql if they are inside a method call executed server side. @@ -134,6 +134,14 @@ private static bool IsConstantExpression(Expression expression) switch (expression.NodeType) { + case ExpressionType.Equal: + case ExpressionType.NotEqual: + case ExpressionType.Not: + case ExpressionType.MemberInit: + case ExpressionType.New: + case ExpressionType.NewArrayInit: + case ExpressionType.ListInit: + return true; case ExpressionType.Extension: if( expression is QuerySourceReferenceExpression extension && (extension.ReferencedQuerySource is NhClauseBase || @@ -153,10 +161,6 @@ private static bool IsConstantExpression(Expression expression) case ExpressionType.MemberAccess: var member = (MemberExpression) expression; return IsConstantExpression(member.Expression); - case ExpressionType.Equal: - case ExpressionType.NotEqual: - case ExpressionType.Not: - return true; case ExpressionType.LessThan: case ExpressionType.LessThanOrEqual: case ExpressionType.GreaterThan: @@ -179,8 +183,8 @@ private static bool IsConstantExpression(Expression expression) case ExpressionType.Conditional: var conditional = (ConditionalExpression) expression; return IsConstantExpression(conditional.Test) && - IsAllowedToProjectInHql(conditional.IfTrue.Type) && IsConstantExpression(conditional.IfTrue) && - IsAllowedToProjectInHql(conditional.IfFalse.Type) && IsConstantExpression(conditional.IfFalse); + IsValueType(conditional.IfTrue.Type) && IsConstantExpression(conditional.IfTrue) && + IsValueType(conditional.IfFalse.Type) && IsConstantExpression(conditional.IfFalse); default: return false; }