From 461a5d567a1e9d6308d72a6018d94d72923e209b Mon Sep 17 00:00:00 2001 From: Duncan M Date: Tue, 23 Aug 2016 15:27:02 -0600 Subject: [PATCH] NH-3901, NH-3846 - Correct IndexOfGenerator 0-based to 1-based index translation --- .../Hql/SimpleFunctionsTest.cs | 2 +- src/NHibernate.Test/Linq/FunctionTests.cs | 51 ++++++++++++++++--- .../Dialect/Function/CharIndexFunction.cs | 36 ++++++++----- .../Function/PositionSubstringFunction.cs | 45 ++++++++-------- .../Linq/Functions/StringGenerator.cs | 22 +++++--- .../Visitors/ExpressionParameterVisitor.cs | 9 +++- 6 files changed, 116 insertions(+), 49 deletions(-) diff --git a/src/NHibernate.Test/Hql/SimpleFunctionsTest.cs b/src/NHibernate.Test/Hql/SimpleFunctionsTest.cs index 30b849e4da0..98241d22b5a 100644 --- a/src/NHibernate.Test/Hql/SimpleFunctionsTest.cs +++ b/src/NHibernate.Test/Hql/SimpleFunctionsTest.cs @@ -131,7 +131,7 @@ public void PositionSubstring() args.Add("'a'"); args.Add("va2"); args.Add("2"); - Assert.AreEqual("(position('a' in substring(va2, 2))+2-1)", psf.Render(args, factoryImpl).ToString()); + Assert.AreEqual("(case position('a' in substring(va2, 2)) when 0 then 0 else (position('a' in substring(va2, 2))+2-1) end)", psf.Render(args, factoryImpl).ToString()); } [Test] diff --git a/src/NHibernate.Test/Linq/FunctionTests.cs b/src/NHibernate.Test/Linq/FunctionTests.cs index fce5c0d1470..0e17f4f4df2 100644 --- a/src/NHibernate.Test/Linq/FunctionTests.cs +++ b/src/NHibernate.Test/Linq/FunctionTests.cs @@ -131,10 +131,35 @@ public void CharIndexFunction() if (!TestDialect.SupportsLocate) Assert.Ignore("Locate function not supported."); + var raw = (from e in db.Employees select e.FirstName).ToList(); + var expected = raw.Select(x => x.ToLower()).Where(x => x.IndexOf('a') == 0).ToList(); + var query = from e in db.Employees - where e.FirstName.IndexOf('A') == 1 - select e.FirstName; + let lowerName = e.FirstName.ToLower() + where lowerName.IndexOf('a') == 0 + select lowerName; + var result = query.ToList(); + Assert.That(result, Is.EqualTo(expected), "Expected {0} but was {1}", string.Join("|", expected), string.Join("|", result)); + ObjectDumper.Write(query); + } + + [Test] + public void CharIndexOffsetNegativeFunction() + { + if (!TestDialect.SupportsLocate) + Assert.Ignore("Locate function not supported."); + + var raw = (from e in db.Employees select e.FirstName).ToList(); + var expected = raw.Select(x => x.ToLower()).Where(x => x.IndexOf('a', 2) == -1).ToList(); + + var query = from e in db.Employees + let lowerName = e.FirstName.ToLower() + where lowerName.IndexOf('a', 2) == -1 + select lowerName; + var result = query.ToList(); + + Assert.That(result, Is.EqualTo(expected), "Expected {0} but was {1}", string.Join("|", expected), string.Join("|", result)); ObjectDumper.Write(query); } @@ -144,10 +169,16 @@ public void IndexOfFunctionExpression() if (!TestDialect.SupportsLocate) Assert.Ignore("Locate function not supported."); + var raw = (from e in db.Employees select e.FirstName).ToList(); + var expected = raw.Select(x => x.ToLower()).Where(x => x.IndexOf("an") == 0).ToList(); + var query = from e in db.Employees - where e.FirstName.IndexOf("An") == 1 - select e.FirstName; + let lowerName = e.FirstName.ToLower() + where lowerName.IndexOf("an") == 0 + select lowerName; + var result = query.ToList(); + Assert.That(result, Is.EqualTo(expected), "Expected {0} but was {1}", string.Join("|", expected), string.Join("|", result)); ObjectDumper.Write(query); } @@ -156,11 +187,17 @@ public void IndexOfFunctionProjection() { if (!TestDialect.SupportsLocate) Assert.Ignore("Locate function not supported."); - + + var raw = (from e in db.Employees select e.FirstName).ToList(); + var expected = raw.Select(x => x.ToLower()).Where(x => x.Contains("a")).Select(x => x.IndexOf("a", 1)).ToList(); + var query = from e in db.Employees - where e.FirstName.Contains("a") - select e.FirstName.IndexOf('A', 3); + let lowerName = e.FirstName.ToLower() + where lowerName.Contains("a") + select lowerName.IndexOf("a", 1); + var result = query.ToList(); + Assert.That(result, Is.EqualTo(expected), "Expected {0} but was {1}", string.Join("|", expected), string.Join("|", result)); ObjectDumper.Write(query); } diff --git a/src/NHibernate/Dialect/Function/CharIndexFunction.cs b/src/NHibernate/Dialect/Function/CharIndexFunction.cs index 5398a5f5a72..1cdfafbff8c 100644 --- a/src/NHibernate/Dialect/Function/CharIndexFunction.cs +++ b/src/NHibernate/Dialect/Function/CharIndexFunction.cs @@ -43,26 +43,38 @@ public SqlString Render(IList args, ISessionFactoryImplementor factory) object start = threeArgs ? args[2] : null; SqlStringBuilder buf = new SqlStringBuilder(); - buf.Add("charindex(") - .AddObject(pattern) - .Add(", "); if (threeArgs) { - buf.Add("right("); + buf.Add("(case "); + RenderPositionInSubstring(buf, pattern, orgString, start); + buf.Add(" when 0 then 0 else ("); + RenderPositionInSubstring(buf, pattern, orgString, start); + buf.Add("+(") + .AddObject(start) + .Add("-1)) end)"); } - buf.AddObject(orgString); - if (threeArgs) + else { - buf.Add(", char_length(") - .AddObject(orgString) - .Add(")-(") - .AddObject(start) - .Add("-1))"); + buf.Add("charindex(") + .AddObject(pattern) + .Add(", ") + .AddObject(orgString) + .Add(")"); } - buf.Add(")"); return buf.ToSqlString(); } + private static void RenderPositionInSubstring(SqlStringBuilder buf, object pattern, object orgString, object start) + { + buf.Add("charindex(") + .AddObject(pattern) + .Add(", right(") + .AddObject(orgString) + .Add(", char_length(") + .AddObject(start) + .Add(")))"); + } + #endregion } } diff --git a/src/NHibernate/Dialect/Function/PositionSubstringFunction.cs b/src/NHibernate/Dialect/Function/PositionSubstringFunction.cs index 2fe33254af1..9a89bda39c4 100644 --- a/src/NHibernate/Dialect/Function/PositionSubstringFunction.cs +++ b/src/NHibernate/Dialect/Function/PositionSubstringFunction.cs @@ -1,6 +1,7 @@ using System; using System.Collections; using System.Text; +using Antlr.Runtime; using NHibernate.Engine; using NHibernate.SqlCommand; using NHibernate.Type; @@ -49,32 +50,36 @@ public SqlString Render(IList args, ISessionFactoryImplementor factory) SqlStringBuilder buf = new SqlStringBuilder(); if (threeArgs) { - buf.Add("("); - } - buf.Add("position(") - .AddObject(pattern) - .Add(" in "); - if (threeArgs) - { - buf.Add("substring("); - } - buf.AddObject(orgString); - if (threeArgs) - { - buf.Add(", ") - .AddObject(start) - .Add(")"); + buf.Add("(case "); + RenderPositionInSubstring(buf, pattern, orgString, start); + buf.Add(" when 0 then 0 else ("); + RenderPositionInSubstring(buf, pattern, orgString, start); + buf.Add("+") + .AddObject(start) + .Add("-1) end)"); } - buf.Add(")"); - if (threeArgs) + else { - buf.Add("+") - .AddObject(start) - .Add("-1)"); + buf.Add("position(") + .AddObject(pattern) + .Add(" in ") + .AddObject(orgString) + .Add(")"); } return buf.ToSqlString(); } + private static void RenderPositionInSubstring(SqlStringBuilder buf, object pattern, object orgString, object start) + { + buf.Add("position(") + .AddObject(pattern) + .Add(" in substring(") + .AddObject(orgString) + .Add(", ") + .AddObject(start) + .Add("))"); + } + #endregion } } diff --git a/src/NHibernate/Linq/Functions/StringGenerator.cs b/src/NHibernate/Linq/Functions/StringGenerator.cs index 77c37c007ef..f70654acfdb 100644 --- a/src/NHibernate/Linq/Functions/StringGenerator.cs +++ b/src/NHibernate/Linq/Functions/StringGenerator.cs @@ -196,17 +196,23 @@ public IndexOfGenerator() } public override HqlTreeNode BuildHql(MethodInfo method, Expression targetObject, ReadOnlyCollection arguments, HqlTreeBuilder treeBuilder, IHqlExpressionVisitor visitor) { + HqlMethodCall locate; if (arguments.Count == 1) { - return treeBuilder.MethodCall("locate", - visitor.Visit(arguments[0]).AsExpression(), - visitor.Visit(targetObject).AsExpression());//, - //treeBuilder.Constant(0)); - } - return treeBuilder.MethodCall("locate", + locate = treeBuilder.MethodCall("locate", visitor.Visit(arguments[0]).AsExpression(), - visitor.Visit(targetObject).AsExpression(), - visitor.Visit(arguments[1]).AsExpression()); + visitor.Visit(targetObject).AsExpression()); //, + //treeBuilder.Constant(0)); + } + else + { + var start = treeBuilder.Add(visitor.Visit(arguments[1]).AsExpression(), treeBuilder.Constant(1)); + locate = treeBuilder.MethodCall("locate", + visitor.Visit(arguments[0]).AsExpression(), + visitor.Visit(targetObject).AsExpression(), + start); + } + return treeBuilder.Subtract(locate,treeBuilder.Constant(1)); } } diff --git a/src/NHibernate/Linq/Visitors/ExpressionParameterVisitor.cs b/src/NHibernate/Linq/Visitors/ExpressionParameterVisitor.cs index 4674d0dc7d1..ec543e9af48 100644 --- a/src/NHibernate/Linq/Visitors/ExpressionParameterVisitor.cs +++ b/src/NHibernate/Linq/Visitors/ExpressionParameterVisitor.cs @@ -86,6 +86,7 @@ protected override Expression VisitConstantExpression(ConstantExpression express if (!_parameters.ContainsKey(expression) && !typeof(IQueryable).IsAssignableFrom(expression.Type) && !IsNullObject(expression)) { // We use null for the type to indicate that the caller should let HQL figure it out. + object value = expression.Value; IType type = null; // We have a bit more information about the null parameter value. @@ -103,12 +104,18 @@ protected override Expression VisitConstantExpression(ConstantExpression express } } + // Constant characters should be sent as strings + if (expression.Type == typeof(char)) + { + value = value.ToString(); + } + // There is more information available in the Linq expression than to HQL directly. // In some cases it might be advantageous to use the extra info. Assuming this // comes up, it would be nice to combine the HQL parameter type determination code // and the Expression information. - _parameters.Add(expression, new NamedParameter("p" + (_parameters.Count + 1), expression.Value, type)); + _parameters.Add(expression, new NamedParameter("p" + (_parameters.Count + 1), value, type)); } return base.VisitConstantExpression(expression);