From 275f4ba5828b63647957bdab00da9e8e4807552e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= Date: Mon, 2 Oct 2017 19:31:00 +0200 Subject: [PATCH 1/2] NH-3787 - Decimal truncation in some Linq queries * Fixes #1335 --- .../NHSpecificTest/NH3787/TestFixture.cs | 131 ++++++++++++++++++ .../NHSpecificTest/NH3787/Mappings.hbm.xml | 12 ++ .../NHSpecificTest/NH3787/RateDto.cs | 7 + .../NHSpecificTest/NH3787/TestEntity.cs | 10 ++ .../NHSpecificTest/NH3787/TestFixture.cs | 124 +++++++++++++++++ src/NHibernate/Dialect/Dialect.cs | 1 + .../Function/TransparentCastFunction.cs | 16 +++ .../ANTLR/SessionFactoryHelperExtensions.cs | 2 +- src/NHibernate/Hql/Ast/HqlTreeBuilder.cs | 11 ++ src/NHibernate/Hql/Ast/HqlTreeNode.cs | 13 ++ .../Visitors/HqlGeneratorExpressionVisitor.cs | 10 +- 11 files changed, 332 insertions(+), 5 deletions(-) create mode 100644 src/NHibernate.Test/Async/NHSpecificTest/NH3787/TestFixture.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/NH3787/Mappings.hbm.xml create mode 100644 src/NHibernate.Test/NHSpecificTest/NH3787/RateDto.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/NH3787/TestEntity.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/NH3787/TestFixture.cs create mode 100644 src/NHibernate/Dialect/Function/TransparentCastFunction.cs diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH3787/TestFixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH3787/TestFixture.cs new file mode 100644 index 00000000000..ea45ad9a861 --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH3787/TestFixture.cs @@ -0,0 +1,131 @@ +//------------------------------------------------------------------------------ +// +// 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.Linq; +using NHibernate.Criterion; +using NHibernate.Transform; +using NUnit.Framework; +using NHibernate.Linq; + +namespace NHibernate.Test.NHSpecificTest.NH3787 +{ + using System.Threading.Tasks; + [TestFixture] + public class TestFixtureAsync : BugTestCase + { + private const decimal _testRate = 12345.123456789M; + + protected override void OnSetUp() + { + base.OnSetUp(); + + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var testEntity = new TestEntity + { + UsePreviousRate = true, + PreviousRate = _testRate, + Rate = 54321.123456789M + }; + s.Save(testEntity); + t.Commit(); + } + } + + protected override void OnTearDown() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + s.CreateQuery("delete from TestEntity").ExecuteUpdate(); + t.Commit(); + } + } + + [Test] + public async Task TestLinqQueryAsync() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var queryResult = await (s + .Query() + .Where(e => e.PreviousRate == _testRate) + .ToListAsync()); + + Assert.That(queryResult.Count, Is.EqualTo(1)); + Assert.That(queryResult[0].PreviousRate, Is.EqualTo(_testRate)); + await (t.CommitAsync()); + } + } + + [Test] + public async Task TestLinqProjectionAsync() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var queryResult = await ((from test in s.Query() + select new RateDto { Rate = test.UsePreviousRate ? test.PreviousRate : test.Rate }).ToListAsync()); + + // Check it has not been truncated to the default 5 positions of NHibernate. + Assert.That(queryResult[0].Rate, Is.EqualTo(_testRate)); + await (t.CommitAsync()); + } + } + + [Test] + public async Task TestLinqQueryOnExpressionAsync() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var queryResult = await (s + .Query() + .Where(e => (e.UsePreviousRate ? e.PreviousRate : e.Rate) == _testRate) + .ToListAsync()); + + Assert.That(queryResult.Count, Is.EqualTo(1)); + Assert.That(queryResult[0].PreviousRate, Is.EqualTo(_testRate)); + await (t.CommitAsync()); + } + } + + [Test] + public async Task TestQueryOverProjectionAsync() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + TestEntity testEntity = null; + + var rateDto = new RateDto(); + //Generated sql + //exec sp_executesql N'SELECT (case when this_.UsePreviousRate = @p0 then this_.PreviousRate else this_.Rate end) as y0_ FROM [TestEntity] this_',N'@p0 bit',@p0=1 + var query = s + .QueryOver(() => testEntity) + .Select( + Projections.Alias( + Projections.Conditional( + Restrictions.Eq(Projections.Property(() => testEntity.UsePreviousRate), true), + Projections.Property(() => testEntity.PreviousRate), + Projections.Property(() => testEntity.Rate)), + "Rate") + .WithAlias(() => rateDto.Rate)); + + var queryResult = await (query.TransformUsing(Transformers.AliasToBean()).ListAsync()); + + Assert.That(queryResult[0].Rate, Is.EqualTo(_testRate)); + await (t.CommitAsync()); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH3787/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/NH3787/Mappings.hbm.xml new file mode 100644 index 00000000000..56a52e4eb78 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3787/Mappings.hbm.xml @@ -0,0 +1,12 @@ + + + + + + + + + + + diff --git a/src/NHibernate.Test/NHSpecificTest/NH3787/RateDto.cs b/src/NHibernate.Test/NHSpecificTest/NH3787/RateDto.cs new file mode 100644 index 00000000000..a36b9439733 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3787/RateDto.cs @@ -0,0 +1,7 @@ +namespace NHibernate.Test.NHSpecificTest.NH3787 +{ + public class RateDto + { + public decimal Rate { get; set; } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH3787/TestEntity.cs b/src/NHibernate.Test/NHSpecificTest/NH3787/TestEntity.cs new file mode 100644 index 00000000000..6bad2e9c918 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3787/TestEntity.cs @@ -0,0 +1,10 @@ +namespace NHibernate.Test.NHSpecificTest.NH3787 +{ + public class TestEntity + { + public virtual int Id { get; set; } + public virtual bool UsePreviousRate { get; set; } + public virtual decimal Rate { get; set; } + public virtual decimal PreviousRate { get; set; } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH3787/TestFixture.cs b/src/NHibernate.Test/NHSpecificTest/NH3787/TestFixture.cs new file mode 100644 index 00000000000..a05b3a41f5a --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3787/TestFixture.cs @@ -0,0 +1,124 @@ +using System.Linq; +using NHibernate.Criterion; +using NHibernate.Linq; +using NHibernate.Transform; +using NHibernate.Type; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH3787 +{ + [TestFixture] + public class TestFixture : BugTestCase + { + private const decimal _testRate = 12345.1234567890123M; + + protected override void OnSetUp() + { + base.OnSetUp(); + + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var testEntity = new TestEntity + { + UsePreviousRate = true, + PreviousRate = _testRate, + Rate = 54321.1234567890123M + }; + s.Save(testEntity); + t.Commit(); + } + } + + protected override void OnTearDown() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + s.CreateQuery("delete from TestEntity").ExecuteUpdate(); + t.Commit(); + } + } + + [Test] + public void TestLinqQuery() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var queryResult = s + .Query() + .Where(e => e.PreviousRate == _testRate) + .ToList(); + + Assert.That(queryResult.Count, Is.EqualTo(1)); + Assert.That(queryResult[0].PreviousRate, Is.EqualTo(_testRate)); + t.Commit(); + } + } + + [Test] + public void TestLinqProjection() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var queryResult = (from test in s.Query() + select new RateDto { Rate = test.UsePreviousRate ? test.PreviousRate : test.Rate }).ToList(); + + // Check it has not been truncated to the default scale (10) of NHibernate. + Assert.That(queryResult[0].Rate, Is.EqualTo(_testRate)); + t.Commit(); + } + } + + [Test] + public void TestLinqQueryOnExpression() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var queryResult = s + .Query() + .Where( + // Without MappedAs, the test fails for SQL Server because it would restrict its parameter to the dialect's default scale. + e => (e.UsePreviousRate ? e.PreviousRate : e.Rate) == _testRate.MappedAs(TypeFactory.Basic("decimal(18,13)"))) + .ToList(); + + Assert.That(queryResult.Count, Is.EqualTo(1)); + Assert.That(queryResult[0].PreviousRate, Is.EqualTo(_testRate)); + t.Commit(); + } + } + + [Test] + public void TestQueryOverProjection() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + TestEntity testEntity = null; + + var rateDto = new RateDto(); + //Generated sql + //exec sp_executesql N'SELECT (case when this_.UsePreviousRate = @p0 then this_.PreviousRate else this_.Rate end) as y0_ FROM [TestEntity] this_',N'@p0 bit',@p0=1 + var query = s + .QueryOver(() => testEntity) + .Select( + Projections + .Alias( + Projections.Conditional( + Restrictions.Eq(Projections.Property(() => testEntity.UsePreviousRate), true), + Projections.Property(() => testEntity.PreviousRate), + Projections.Property(() => testEntity.Rate)), + "Rate") + .WithAlias(() => rateDto.Rate)); + + var queryResult = query.TransformUsing(Transformers.AliasToBean()).List(); + + Assert.That(queryResult[0].Rate, Is.EqualTo(_testRate)); + t.Commit(); + } + } + } +} diff --git a/src/NHibernate/Dialect/Dialect.cs b/src/NHibernate/Dialect/Dialect.cs index a4b5075f13c..1ce0af30de1 100644 --- a/src/NHibernate/Dialect/Dialect.cs +++ b/src/NHibernate/Dialect/Dialect.cs @@ -98,6 +98,7 @@ protected Dialect() RegisterFunction("upper", new StandardSQLFunction("upper")); RegisterFunction("lower", new StandardSQLFunction("lower")); RegisterFunction("cast", new CastFunction()); + RegisterFunction("transparentcast", new TransparentCastFunction()); RegisterFunction("extract", new AnsiExtractFunction()); RegisterFunction("concat", new VarArgsSQLFunction(NHibernateUtil.String, "(", "||", ")")); diff --git a/src/NHibernate/Dialect/Function/TransparentCastFunction.cs b/src/NHibernate/Dialect/Function/TransparentCastFunction.cs new file mode 100644 index 00000000000..3572fc422ce --- /dev/null +++ b/src/NHibernate/Dialect/Function/TransparentCastFunction.cs @@ -0,0 +1,16 @@ +using System; + +namespace NHibernate.Dialect.Function +{ + /// + /// A HQL only cast for helping HQL knowing the type. Does not generates any actual cast in SQL code. + /// + [Serializable] + public class TransparentCastFunction : CastFunction + { + protected override bool CastingIsRequired(string sqlType) + { + return false; + } + } +} diff --git a/src/NHibernate/Hql/Ast/ANTLR/SessionFactoryHelperExtensions.cs b/src/NHibernate/Hql/Ast/ANTLR/SessionFactoryHelperExtensions.cs index d2dc73c2934..142c8a0c4ab 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/SessionFactoryHelperExtensions.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/SessionFactoryHelperExtensions.cs @@ -77,7 +77,7 @@ public IType FindFunctionReturnType(String functionName, IASTNode first) if (first != null) { - if (functionName == "cast") + if (sqlFunction is CastFunction) { argumentType = TypeFactory.HeuristicType(first.NextSibling.Text); } diff --git a/src/NHibernate/Hql/Ast/HqlTreeBuilder.cs b/src/NHibernate/Hql/Ast/HqlTreeBuilder.cs index bbe3517c776..e49cac5a5b3 100755 --- a/src/NHibernate/Hql/Ast/HqlTreeBuilder.cs +++ b/src/NHibernate/Hql/Ast/HqlTreeBuilder.cs @@ -301,6 +301,17 @@ public HqlCast Cast(HqlExpression expression, System.Type type) return new HqlCast(_factory, expression, type); } + /// + /// Generate a cast node intended solely to hint HQL at the resulting type, without issuing an actual SQL cast. + /// + /// The expression to cast. + /// The resulting type. + /// A node. + public HqlTransparentCast TransparentCast(HqlExpression expression, System.Type type) + { + return new HqlTransparentCast(_factory, expression, type); + } + public HqlBitwiseNot BitwiseNot() { return new HqlBitwiseNot(_factory); diff --git a/src/NHibernate/Hql/Ast/HqlTreeNode.cs b/src/NHibernate/Hql/Ast/HqlTreeNode.cs index b44efbe4c84..fba59d196dd 100755 --- a/src/NHibernate/Hql/Ast/HqlTreeNode.cs +++ b/src/NHibernate/Hql/Ast/HqlTreeNode.cs @@ -701,6 +701,19 @@ public HqlCast(IASTFactory factory, HqlExpression expression, System.Type type) } } + /// + /// Cast node intended solely to hint HQL at the resulting type, without issuing an actual SQL cast. + /// + public class HqlTransparentCast : HqlExpression + { + public HqlTransparentCast(IASTFactory factory, HqlExpression expression, System.Type type) + : base(HqlSqlWalker.METHOD_CALL, "method", factory) + { + AddChild(new HqlIdent(factory, "transparentcast")); + AddChild(new HqlExpressionList(factory, expression, new HqlIdent(factory, type))); + } + } + public class HqlCoalesce : HqlExpression { public HqlCoalesce(IASTFactory factory, HqlExpression lhs, HqlExpression rhs) diff --git a/src/NHibernate/Linq/Visitors/HqlGeneratorExpressionVisitor.cs b/src/NHibernate/Linq/Visitors/HqlGeneratorExpressionVisitor.cs index cb4ec1cca66..6a0e7eda8d9 100644 --- a/src/NHibernate/Linq/Visitors/HqlGeneratorExpressionVisitor.cs +++ b/src/NHibernate/Linq/Visitors/HqlGeneratorExpressionVisitor.cs @@ -8,7 +8,6 @@ using NHibernate.Param; using NHibernate.Util; using Remotion.Linq.Clauses.Expressions; -using Remotion.Linq.Clauses.ResultOperators; namespace NHibernate.Linq.Visitors { @@ -538,9 +537,12 @@ protected HqlTreeNode VisitConditionalExpression(ConditionalExpression expressio HqlExpression @case = _hqlTreeBuilder.Case(new[] {_hqlTreeBuilder.When(test, ifTrue)}, ifFalse); - return (expression.Type == typeof (bool) || expression.Type == (typeof (bool?))) - ? @case - : _hqlTreeBuilder.Cast(@case, expression.Type); + // If both operands are parameters, HQL will not be able to determine the resulting type before + // parameters binding. But it has to compute result set columns type before parameters are bound, + // so an artificial cast is introduced to hint HQL at the resulting type. + return expression.Type == typeof(bool) || expression.Type == typeof(bool?) + ? @case + : _hqlTreeBuilder.TransparentCast(@case, expression.Type); } protected HqlTreeNode VisitSubQueryExpression(SubQueryExpression expression) From b82fbf4b6ffde16dc31fb9ce04ecaf3d6cec40f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= Date: Tue, 28 Nov 2017 00:36:27 +0100 Subject: [PATCH 2/2] NH-3787 - SQLite requires the cast in SQL And SQLite cannot perform the NH-3787 test with so many digits (it does not have a true decimal type anyway). To be squashed --- .../NHSpecificTest/NH3787/TestFixture.cs | 35 ++++++++++++------- .../NHSpecificTest/NH3787/TestFixture.cs | 5 +++ src/NHibernate/Dialect/SQLiteDialect.cs | 3 ++ 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH3787/TestFixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH3787/TestFixture.cs index ea45ad9a861..0cc7243bf40 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/NH3787/TestFixture.cs +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH3787/TestFixture.cs @@ -10,9 +10,10 @@ using System.Linq; using NHibernate.Criterion; +using NHibernate.Linq; using NHibernate.Transform; +using NHibernate.Type; using NUnit.Framework; -using NHibernate.Linq; namespace NHibernate.Test.NHSpecificTest.NH3787 { @@ -20,7 +21,12 @@ namespace NHibernate.Test.NHSpecificTest.NH3787 [TestFixture] public class TestFixtureAsync : BugTestCase { - private const decimal _testRate = 12345.123456789M; + private const decimal _testRate = 12345.1234567890123M; + + protected override bool AppliesTo(Dialect.Dialect dialect) + { + return !TestDialect.HasBrokenDecimalType; + } protected override void OnSetUp() { @@ -33,7 +39,7 @@ protected override void OnSetUp() { UsePreviousRate = true, PreviousRate = _testRate, - Rate = 54321.123456789M + Rate = 54321.1234567890123M }; s.Save(testEntity); t.Commit(); @@ -76,7 +82,7 @@ public async Task TestLinqProjectionAsync() var queryResult = await ((from test in s.Query() select new RateDto { Rate = test.UsePreviousRate ? test.PreviousRate : test.Rate }).ToListAsync()); - // Check it has not been truncated to the default 5 positions of NHibernate. + // Check it has not been truncated to the default scale (10) of NHibernate. Assert.That(queryResult[0].Rate, Is.EqualTo(_testRate)); await (t.CommitAsync()); } @@ -90,9 +96,11 @@ public async Task TestLinqQueryOnExpressionAsync() { var queryResult = await (s .Query() - .Where(e => (e.UsePreviousRate ? e.PreviousRate : e.Rate) == _testRate) + .Where( + // Without MappedAs, the test fails for SQL Server because it would restrict its parameter to the dialect's default scale. + e => (e.UsePreviousRate ? e.PreviousRate : e.Rate) == _testRate.MappedAs(TypeFactory.Basic("decimal(18,13)"))) .ToListAsync()); - + Assert.That(queryResult.Count, Is.EqualTo(1)); Assert.That(queryResult[0].PreviousRate, Is.EqualTo(_testRate)); await (t.CommitAsync()); @@ -113,13 +121,14 @@ public async Task TestQueryOverProjectionAsync() var query = s .QueryOver(() => testEntity) .Select( - Projections.Alias( - Projections.Conditional( - Restrictions.Eq(Projections.Property(() => testEntity.UsePreviousRate), true), - Projections.Property(() => testEntity.PreviousRate), - Projections.Property(() => testEntity.Rate)), - "Rate") - .WithAlias(() => rateDto.Rate)); + Projections + .Alias( + Projections.Conditional( + Restrictions.Eq(Projections.Property(() => testEntity.UsePreviousRate), true), + Projections.Property(() => testEntity.PreviousRate), + Projections.Property(() => testEntity.Rate)), + "Rate") + .WithAlias(() => rateDto.Rate)); var queryResult = await (query.TransformUsing(Transformers.AliasToBean()).ListAsync()); diff --git a/src/NHibernate.Test/NHSpecificTest/NH3787/TestFixture.cs b/src/NHibernate.Test/NHSpecificTest/NH3787/TestFixture.cs index a05b3a41f5a..91f6bbd514f 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH3787/TestFixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH3787/TestFixture.cs @@ -12,6 +12,11 @@ public class TestFixture : BugTestCase { private const decimal _testRate = 12345.1234567890123M; + protected override bool AppliesTo(Dialect.Dialect dialect) + { + return !TestDialect.HasBrokenDecimalType; + } + protected override void OnSetUp() { base.OnSetUp(); diff --git a/src/NHibernate/Dialect/SQLiteDialect.cs b/src/NHibernate/Dialect/SQLiteDialect.cs index 03748e0b022..737b8ce31e2 100644 --- a/src/NHibernate/Dialect/SQLiteDialect.cs +++ b/src/NHibernate/Dialect/SQLiteDialect.cs @@ -86,6 +86,9 @@ protected virtual void RegisterFunctions() RegisterFunction("cast", new SQLiteCastFunction()); RegisterFunction("round", new StandardSQLFunction("round")); + + // NH-3787: SQLite requires the cast in SQL too for not defaulting to string. + RegisterFunction("transparentcast", new CastFunction()); } #region private static readonly string[] DialectKeywords = { ... }