Merge pull request #2305 from fredericDelaporte/Backport2298

Backport Dml Linq Update Produce Wrong Sql
Release 5.1.7

Author: fredericDelaporte

appveyor.yml

@@ -1,4 +1,4 @@
-version: 5.1.6.{build}
+version: 5.1.7.{build}
 image: Visual Studio 2017
 environment:
   matrix:

build-common/NHibernate.props

@@ -2,7 +2,7 @@
   <PropertyGroup>
     <VersionMajor Condition="'$(VersionMajor)' == ''">5</VersionMajor>
     <VersionMinor Condition="'$(VersionMinor)' == ''">1</VersionMinor>
-    <VersionPatch Condition="'$(VersionPatch)' == ''">6</VersionPatch>
+    <VersionPatch Condition="'$(VersionPatch)' == ''">7</VersionPatch>
     <VersionSuffix Condition="'$(VersionSuffix)' == ''"></VersionSuffix>
 
     <VersionPrefix>$(VersionMajor).$(VersionMinor).$(VersionPatch)</VersionPrefix>

build-common/common.xml

@@ -13,8 +13,8 @@
 
 	<!-- This is used only for build folder -->
 	<!-- TODO: Either remove or refactor to use NHibernate.props -->
-	<property name="project.version" value="5.1.6" overwrite="false" />
-	<property name="project.version.numeric" value="5.1.6" overwrite="false" />
+	<property name="project.version" value="5.1.7" overwrite="false" />
+	<property name="project.version.numeric" value="5.1.7" overwrite="false" />
 
 	<!-- properties used to connect to database for testing -->
 	<include buildfile="nhibernate-properties.xml" />

releasenotes.txt

@@ -1,3 +1,11 @@
+Build 5.1.7
+=============================
+
+Release notes - NHibernate - Version 5.1.7
+
+** Bug
+    * #2298 Dml Linq Update Produce Wrong Sql
+
 Build 5.1.6
 =============================
 

src/NHibernate.Test/Async/Linq/ConstantTest.cs

@@ -11,12 +11,13 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Reflection;
+using NHibernate.Criterion;
 using NHibernate.DomainModel.Northwind.Entities;
 using NHibernate.Engine.Query;
+using NHibernate.Linq;
 using NHibernate.Linq.Visitors;
 using NHibernate.Util;
 using NUnit.Framework;
-using NHibernate.Linq;
 
 namespace NHibernate.Test.Linq
 {
@@ -269,5 +270,26 @@ public async Task PlansWithNonParameterizedConstantsAreNotCachedAsync()
 				Has.Count.EqualTo(0),
 				"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.");
+		}
 	}
 }

src/NHibernate.Test/Linq/ConstantTest.cs

@@ -1,8 +1,10 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Reflection;
+using NHibernate.Criterion;
 using NHibernate.DomainModel.Northwind.Entities;
 using NHibernate.Engine.Query;
+using NHibernate.Linq;
 using NHibernate.Linq.Visitors;
 using NHibernate.Util;
 using NUnit.Framework;
@@ -276,5 +278,62 @@ public void PlansWithNonParameterizedConstantsAreNotCached()
 				Has.Count.EqualTo(0),
 				"Query plan should not be cached.");
 		}
+
+		[Test]
+		public void PlansWithNonParameterizedConstantsAreNotCachedForExpandedQuery()
+		{
+			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();
+			db.Customers.Where(x => ids.Contains(x.CustomerId)).Select(
+				c => new {c.CustomerId, c.ContactName, Constant = 1}).First();
+
+			Assert.That(
+				cache,
+				Has.Count.EqualTo(0),
+				"Query plan should not be cached.");
+		}
+
+		//GH-2298 - Different Update queries - same query cache plan
+		[Test]
+		public void DmlPlansForExpandedQuery()
+		{
+			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();
+				db.Customers.Where(x => list.Contains(x.CustomerId)).Update(
+					x => new Customer
+					{
+						CompanyName = "Constant1"
+					});
+
+				db.Customers.Where(x => list.Contains(x.CustomerId))
+				.Update(
+					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.");
+			}
+		}
 	}
 }

src/NHibernate/Engine/Query/QueryPlanCache.cs

@@ -60,7 +60,7 @@ public IQueryExpressionPlan GetHQLQueryPlan(IQueryExpression queryExpression, bo
 				}
 				plan = new QueryExpressionPlan(queryExpression, shallow, enabledFilters, factory);
 				// 6.0 TODO: add "CanCachePlan { get; }" to IQueryExpression interface
-				if (!(queryExpression is NhLinqExpression linqExpression) || linqExpression.CanCachePlan)
+				if (!(queryExpression is ICacheableQueryExpression linqExpression) || linqExpression.CanCachePlan)
 					planCache.Put(key, plan);
 				else
 					log.Debug("Query plan not cacheable");
@@ -115,7 +115,7 @@ public IQueryExpressionPlan GetFilterQueryPlan(IQueryExpression queryExpression,
 				log.Debug("unable to locate collection-filter query plan in cache; generating ({0} : {1})", collectionRole, queryExpression.Key);
 				plan = new FilterQueryPlan(queryExpression, collectionRole, shallow, enabledFilters, factory);
 				// 6.0 TODO: add "CanCachePlan { get; }" to IQueryExpression interface
-				if (!(queryExpression is NhLinqExpression linqExpression) || linqExpression.CanCachePlan)
+				if (!(queryExpression is ICacheableQueryExpression linqExpression) || linqExpression.CanCachePlan)
 					planCache.Put(key, plan);
 				else
 					log.Debug("Query plan not cacheable");

src/NHibernate/IQueryExpression.cs

@@ -5,11 +5,17 @@
 
 namespace NHibernate
 {
+	//TODO 6.0: Merge into IQueryExpression
+	internal interface ICacheableQueryExpression
+	{
+		bool CanCachePlan { get; }
+	}
+
 	public interface IQueryExpression
 	{
 		IASTNode Translate(ISessionFactoryImplementor sessionFactory, bool filter);
 		string Key { get; }
 		System.Type Type { get; }
 		IList<NamedParameterDescriptor> ParameterDescriptors { get; }
 	}
-}
+}

src/NHibernate/Impl/ExpressionQueryImpl.cs

@@ -150,16 +150,18 @@ public override object[] ValueArray()
 		}
 	}
 
-	internal class ExpandedQueryExpression : IQueryExpression
+	internal class ExpandedQueryExpression : IQueryExpression, ICacheableQueryExpression
 	{
 		private readonly IASTNode _tree;
+		private ICacheableQueryExpression _cacheableExpression;
 
 		public ExpandedQueryExpression(IQueryExpression queryExpression, IASTNode tree, string key)
 		{
 			_tree = tree;
 			Key = key;
 			Type = queryExpression.Type;
 			ParameterDescriptors = queryExpression.ParameterDescriptors;
+			 _cacheableExpression = queryExpression as ICacheableQueryExpression;
 		}
 
 		#region IQueryExpression Members
@@ -176,6 +178,8 @@ public IASTNode Translate(ISessionFactoryImplementor sessionFactory, bool filter
 		public IList<NamedParameterDescriptor> ParameterDescriptors { get; private set; }
 
 		#endregion
+
+		public bool CanCachePlan => _cacheableExpression?.CanCachePlan ?? true;
 	}
 
 	internal class ParameterExpander

src/NHibernate/Linq/NhLinqExpression.cs

@@ -11,7 +11,7 @@
 
 namespace NHibernate.Linq
 {
-	public class NhLinqExpression : IQueryExpression
+	public class NhLinqExpression : IQueryExpression, ICacheableQueryExpression
 	{
 		public string Key { get; protected set; }