From 1097114d87e088be8d17acc658eebeddff90f67d Mon Sep 17 00:00:00 2001 From: Ayende Rahien Date: Tue, 24 Jan 2012 18:31:56 +0200 Subject: [PATCH 1/4] Adding failing test --- .../NHSpecificTest/NH2500/Fixture.cs | 96 +++++++++++++++++++ src/NHibernate.Test/NHibernate.Test.csproj | 1 + 2 files changed, 97 insertions(+) create mode 100644 src/NHibernate.Test/NHSpecificTest/NH2500/Fixture.cs diff --git a/src/NHibernate.Test/NHSpecificTest/NH2500/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH2500/Fixture.cs new file mode 100644 index 00000000000..61454912471 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH2500/Fixture.cs @@ -0,0 +1,96 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; + +using NHibernate.Cfg.MappingSchema; +using NHibernate.Linq; +using NHibernate.Mapping.ByCode; + +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH2500 +{ + public class Foo + { + public virtual Guid Id { get; set; } + + public virtual string Name { get; set; } + } + + + [TestFixture] + public class Fixture : TestCaseMappingByCode + { + protected override HbmMapping GetMappings() + { + var mapper = new ConventionModelMapper(); + mapper.BeforeMapClass += (mi, t, x) => x.Id(map => map.Generator(Generators.Guid)); + return mapper.CompileMappingFor(new[] { typeof(Foo) }); + } + + protected override void OnSetUp() + { + base.OnSetUp(); + + using (ISession session = Sfi.OpenSession()) + using (ITransaction transaction = session.BeginTransaction()) + { + session.Persist(new Foo { Name = "Banana" }); + session.Persist(new Foo { Name = "Egg" }); + transaction.Commit(); + } + } + + protected override void OnTearDown() + { + using (ISession session = Sfi.OpenSession()) + using (ITransaction transaction = session.BeginTransaction()) + { + session.CreateQuery("delete from Foo").ExecuteUpdate(); + transaction.Commit(); + } + + base.OnTearDown(); + } + + private int count; + + [Test] + public void TestLinqProjectionExpressionDoesntCacheParameters() + { + using (ISession session = Sfi.OpenSession()) + using (ITransaction transaction = session.BeginTransaction()) + { + this.count = 1; + + var foos1 = session.Query() + .Where(x => x.Name == "Banana") + .Select(x => new + { + x.Name, + count, + User = "abc" + }).First(); + + count = 2; + + var foos2 = session.Query() + .Where(x => x.Name == "Egg") + .Select(x => new + { + x.Name, + count, + User = "def" + }).First(); + + Assert.AreEqual(1, foos1.count); + Assert.AreEqual(2, foos2.count); + Assert.AreEqual("abc", foos1.User); + Assert.AreEqual("def", foos2.User); + + transaction.Commit(); + } + } + } +} diff --git a/src/NHibernate.Test/NHibernate.Test.csproj b/src/NHibernate.Test/NHibernate.Test.csproj index 73595b31ed8..b8d0998da9d 100644 --- a/src/NHibernate.Test/NHibernate.Test.csproj +++ b/src/NHibernate.Test/NHibernate.Test.csproj @@ -653,6 +653,7 @@ + From ab3c45bec4c45187b4243278d0439aba700207dc Mon Sep 17 00:00:00 2001 From: Ayende Rahien Date: Tue, 24 Jan 2012 19:05:49 +0200 Subject: [PATCH 2/4] Modified the way we are generating the query plan key to take into account that there might be constants in the select clause (which get compiled into lambdas) that we need to account for. --- .../NHSpecificTest/NH2500/Fixture.cs | 30 +++++++++---------- .../Linq/Visitors/ExpressionKeyVisitor.cs | 20 ++++++++++++- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/NHibernate.Test/NHSpecificTest/NH2500/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH2500/Fixture.cs index 61454912471..26a3e2fe217 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH2500/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH2500/Fixture.cs @@ -65,24 +65,24 @@ public void TestLinqProjectionExpressionDoesntCacheParameters() this.count = 1; var foos1 = session.Query() - .Where(x => x.Name == "Banana") - .Select(x => new - { - x.Name, - count, - User = "abc" - }).First(); + .Where(x => x.Name == "Banana") + .Select(x => new + { + x.Name, + count, + User = "abc" + }).First(); - count = 2; + this.count = 2; var foos2 = session.Query() - .Where(x => x.Name == "Egg") - .Select(x => new - { - x.Name, - count, - User = "def" - }).First(); + .Where(x => x.Name == "Egg") + .Select(x => new + { + x.Name, + count, + User = "def" + }).First(); Assert.AreEqual(1, foos1.count); Assert.AreEqual(2, foos2.count); diff --git a/src/NHibernate/Linq/Visitors/ExpressionKeyVisitor.cs b/src/NHibernate/Linq/Visitors/ExpressionKeyVisitor.cs index fb3d18ce13e..d5a57832c0a 100644 --- a/src/NHibernate/Linq/Visitors/ExpressionKeyVisitor.cs +++ b/src/NHibernate/Linq/Visitors/ExpressionKeyVisitor.cs @@ -78,7 +78,7 @@ protected override Expression VisitConstantExpression(ConstantExpression express { NamedParameter param; - if (_constantToParameterMap.TryGetValue(expression, out param)) + if (_constantToParameterMap.TryGetValue(expression, out param) && insideSelectClause == false) { // Nulls generate different query plans. X = variable generates a different query depending on if variable is null or not. if (param.Value == null) @@ -169,8 +169,25 @@ protected override MemberBinding VisitMemberMemberBinding(MemberMemberBinding bi return base.VisitMemberMemberBinding(binding); } + private bool insideSelectClause; protected override Expression VisitMethodCallExpression(MethodCallExpression expression) { + var old = insideSelectClause; + + switch (expression.Method.Name) + { + case "First": + case "FirstOrDefault": + case "Single": + case "SingleOrDefault": + case "Select": + insideSelectClause = true; + break; + default: + insideSelectClause = false; + break; + } + VisitExpression(expression.Object); _string.Append('.'); VisitMethod(expression.Method); @@ -178,6 +195,7 @@ protected override Expression VisitMethodCallExpression(MethodCallExpression exp VisitList(expression.Arguments, AppendCommas); _string.Append(')'); + insideSelectClause = old; return expression; } From 32cdfaa33b6f633fc63856d802dd2a6cb05eb69b Mon Sep 17 00:00:00 2001 From: Ayende Rahien Date: Tue, 24 Jan 2012 19:13:13 +0200 Subject: [PATCH 3/4] Adding test to show that NH-2419 works --- .../Linq/ByMethod/OrderByTests.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/NHibernate.Test/Linq/ByMethod/OrderByTests.cs b/src/NHibernate.Test/Linq/ByMethod/OrderByTests.cs index 0cd66d27df5..16b8daa3993 100644 --- a/src/NHibernate.Test/Linq/ByMethod/OrderByTests.cs +++ b/src/NHibernate.Test/Linq/ByMethod/OrderByTests.cs @@ -6,6 +6,23 @@ namespace NHibernate.Test.Linq.ByMethod [TestFixture] public class OrderByTests : LinqTestCase { + [Test] + public void GroupByThenOrderBy() + { + var query = from c in db.Customers + group c by c.Address.Country into g + orderby g.Key + select new { p0 = g.Key, p1 = g.Count() }; + + var ids = query.ToList(); + Assert.NotNull(ids); + if (ids.Count > 1) + { + Assert.Less(ids[0].p0, ids[1].p0); + } + + } + [Test] public void AscendingOrderByClause() { From 6ebd3e8db807862b5fb091d7d2e1907088d1dd18 Mon Sep 17 00:00:00 2001 From: Ayende Rahien Date: Wed, 25 Jan 2012 11:48:52 +0200 Subject: [PATCH 4/4] Better assert --- src/NHibernate.Test/Linq/ByMethod/OrderByTests.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/NHibernate.Test/Linq/ByMethod/OrderByTests.cs b/src/NHibernate.Test/Linq/ByMethod/OrderByTests.cs index 16b8daa3993..fa1e4319ac7 100644 --- a/src/NHibernate.Test/Linq/ByMethod/OrderByTests.cs +++ b/src/NHibernate.Test/Linq/ByMethod/OrderByTests.cs @@ -12,15 +12,11 @@ public void GroupByThenOrderBy() var query = from c in db.Customers group c by c.Address.Country into g orderby g.Key - select new { p0 = g.Key, p1 = g.Count() }; + select new { Country = g.Key, Count = g.Count() }; var ids = query.ToList(); Assert.NotNull(ids); - if (ids.Count > 1) - { - Assert.Less(ids[0].p0, ids[1].p0); - } - + AssertOrderedBy.Ascending(ids, arg => arg.Country); } [Test]