Bugfix for culture invariant decimal types (redis#423)

README.md

@@ -469,6 +469,7 @@ We'd love your contributions! If you want to contribute please read our [Contrib
 * [@AndersenGans](https://github.com/AndersenGans)
 * [@mdrakib](https://github.com/mdrakib)
 * [@jrpavoncello](https://github.com/jrpavoncello)
+* [@axnetg](https://github.com/axnetg)
 
 <!-- Logo -->
 [Logo]: images/logo.svg

src/Redis.OM/Aggregation/AggregationPredicates/QueryPredicate.cs

@@ -57,32 +57,32 @@ protected override void ValidateAndPushOperand(Expression expression, Stack<stri
 
                 if (binaryExpression.Right is ConstantExpression constantExpression)
                 {
-                    stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, constantExpression));
+                    var constVal = ExpressionParserUtilities.GetOperandString(constantExpression);
+                    stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, constVal));
                 }
                 else if (binaryExpression.Right is UnaryExpression uni)
                 {
                     switch (uni.Operand)
                     {
                         case ConstantExpression c:
-                            stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, c));
+                            var constVal = ExpressionParserUtilities.GetOperandString(c);
+                            stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, constVal));
                             break;
                         case MemberExpression mem:
-                        {
                             var val = ExpressionParserUtilities.GetOperandString(mem);
-                            stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, System.Linq.Expressions.Expression.Constant(val)));
+                            stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, val));
                             break;
-                        }
                     }
                 }
                 else if (binaryExpression.Right is MemberExpression mem)
                 {
                     var val = ExpressionParserUtilities.GetOperandString(mem);
-                    stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, System.Linq.Expressions.Expression.Constant(val)));
+                    stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, val));
                 }
                 else
                 {
                     var val = ExpressionParserUtilities.GetOperandStringForQueryArgs(binaryExpression.Right, new List<object>()); // hack - will need to revisit when integrating vectors into aggregations.
-                    stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, System.Linq.Expressions.Expression.Constant(val)));
+                    stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, val));
                 }
             }
             else if (expression is ConstantExpression c
@@ -169,7 +169,7 @@ protected override void SplitBinaryExpression(BinaryExpression expression, Stack
             }
         }
 
-        private static string BuildEqualityPredicate(MemberInfo member, ConstantExpression expression, string memberStr, bool negated = false)
+        private static string BuildEqualityPredicate(MemberInfo member, string queryValue, string memberStr, bool negated = false)
         {
             var sb = new StringBuilder();
             var fieldAttribute = member.GetCustomAttribute<SearchFieldAttribute>();
@@ -190,13 +190,13 @@ private static string BuildEqualityPredicate(MemberInfo member, ConstantExpressi
             switch (searchFieldType)
             {
                 case SearchFieldType.TAG:
-                    sb.Append($"{{{ExpressionParserUtilities.EscapeTagField(expression.Value.ToString())}}}");
+                    sb.Append($"{{{ExpressionParserUtilities.EscapeTagField(queryValue)}}}");
                     break;
                 case SearchFieldType.TEXT:
-                    sb.Append(expression.Value);
+                    sb.Append(queryValue);
                     break;
                 case SearchFieldType.NUMERIC:
-                    sb.Append($"[{expression.Value} {expression.Value}]");
+                    sb.Append($"[{queryValue} {queryValue}]");
                     break;
                 default:
                     throw new InvalidOperationException("Could not translate query equality searches only supported for Tag, text, and numeric fields");
@@ -205,17 +205,17 @@ private static string BuildEqualityPredicate(MemberInfo member, ConstantExpressi
             return sb.ToString();
         }
 
-        private string BuildQueryPredicate(ExpressionType expType, MemberExpression member, ConstantExpression constExpression)
+        private string BuildQueryPredicate(ExpressionType expType, MemberExpression member, string queryValue)
         {
             var memberStr = ExpressionParserUtilities.GetOperandString(member);
             var queryPredicate = expType switch
             {
-                ExpressionType.GreaterThan => $"{memberStr}:[({constExpression.Value} inf]",
-                ExpressionType.LessThan => $"{memberStr}:[-inf ({constExpression.Value}]",
-                ExpressionType.GreaterThanOrEqual => $"{memberStr}:[{constExpression.Value} inf]",
-                ExpressionType.LessThanOrEqual => $"{memberStr}:[-inf {constExpression.Value}]",
-                ExpressionType.Equal => BuildEqualityPredicate(member.Member, constExpression, memberStr),
-                ExpressionType.NotEqual => BuildEqualityPredicate(member.Member, constExpression, memberStr, true),
+                ExpressionType.GreaterThan => $"{memberStr}:[({queryValue} inf]",
+                ExpressionType.LessThan => $"{memberStr}:[-inf ({queryValue}]",
+                ExpressionType.GreaterThanOrEqual => $"{memberStr}:[{queryValue} inf]",
+                ExpressionType.LessThanOrEqual => $"{memberStr}:[-inf {queryValue}]",
+                ExpressionType.Equal => BuildEqualityPredicate(member.Member, queryValue, memberStr),
+                ExpressionType.NotEqual => BuildEqualityPredicate(member.Member, queryValue, memberStr, true),
                 _ => string.Empty
             };
             return queryPredicate;

src/Redis.OM/Common/ExpressionParserUtilities.cs

@@ -83,7 +83,7 @@ internal static string GetOperandStringForQueryArgs(Expression exp, List<object>
         {
             var res = exp switch
             {
-                ConstantExpression constExp => $"{constExp.Value}",
+                ConstantExpression constExp => ValueToString(constExp.Value),
                 MemberExpression member => GetOperandStringForMember(member, treatEnumsAsInt),
                 MethodCallExpression method => TranslateMethodStandardQuerySyntax(method, parameters),
                 UnaryExpression unary => GetOperandStringForQueryArgs(unary.Operand, parameters, treatEnumsAsInt, unary.NodeType == ExpressionType.Not),
@@ -960,6 +960,16 @@ private static string ValueToString(object value)
                 return ((double)value).ToString(CultureInfo.InvariantCulture);
             }
 
+            if (valueType == typeof(float) || Nullable.GetUnderlyingType(valueType) == typeof(float))
+            {
+                return ((float)value).ToString(CultureInfo.InvariantCulture);
+            }
+
+            if (valueType == typeof(decimal) || Nullable.GetUnderlyingType(valueType) == typeof(decimal))
+            {
+                return ((decimal)value).ToString(CultureInfo.InvariantCulture);
+            }
+
             if (value is DateTimeOffset dto)
             {
                 return dto.ToUnixTimeMilliseconds().ToString(CultureInfo.InvariantCulture);

src/Redis.OM/Modeling/JsonDiff.cs

@@ -1,5 +1,5 @@
 using System;
-using System.Linq;
+using System.Globalization;
 using System.Text.Json;
 using System.Web;
 using Newtonsoft.Json.Linq;
@@ -37,6 +37,7 @@ public string[] SerializeScriptArgs()
             return _value.Type switch
             {
                 JTokenType.String => new[] { _operation, _path, $"\"{HttpUtility.JavaScriptStringEncode(_value.ToString())}\"" },
+                JTokenType.Float => new[] { _operation, _path, ((JValue)_value).ToString(CultureInfo.InvariantCulture) },
                 JTokenType.Boolean => new[] { _operation, _path, _value.ToString().ToLower() },
                 JTokenType.Date => SerializeAsDateTime(),
                 _ => new[] { _operation, _path, _value.ToString() }

src/Redis.OM/RedisObjectHandler.cs

@@ -350,7 +350,7 @@ internal static IDictionary<string, object> BuildHashSet(this object obj)
                     var val = property.GetValue(obj);
                     if (val != null)
                     {
-                        hash.Add(propertyName, val.ToString());
+                        hash.Add(propertyName, Convert.ToString(val, CultureInfo.InvariantCulture));
                     }
                 }
                 else if (type.IsEnum)

test/Redis.OM.Unit.Tests/RediSearchTests/AggregationSetTests.cs

@@ -587,5 +587,35 @@ public void DateTimeQuery()
             _ = collection.Where(query).ToList();
             _substitute.Received().Execute("FT.AGGREGATE", "objectwithdatetime-idx", $"@TimestampOffset:[({dtMs} inf]", "WITHCURSOR", "COUNT", "10000");
         }
+
+        [Fact]
+        public void TestDecimalQuery()
+        {
+            var collection = new RedisAggregationSet<Person>(_substitute, true, chunkSize: 10000);
+            _substitute.Execute("FT.AGGREGATE", Arg.Any<object[]>()).Returns(MockedResult);
+            _substitute.Execute("FT.CURSOR", Arg.Any<object[]>()).Returns(MockedResultCursorEnd);
+
+            var y = 30.55M;
+            Expression<Func<AggregationResult<Person>, bool>> query = a => a.RecordShell!.Salary > y;
+            _ = collection.Where(query).ToList();
+            _substitute.Received().Execute("FT.AGGREGATE", "person-idx", "@Salary:[(30.55 inf]", "WITHCURSOR", "COUNT", "10000");
+
+            query = a => a.RecordShell!.Salary > 85.99M;
+            _ = collection.Where(query).ToList();
+            _substitute.Received().Execute("FT.AGGREGATE", "person-idx", "@Salary:[(85.99 inf]", "WITHCURSOR", "COUNT", "10000");
+
+            query = a => a.RecordShell!.Salary == 70.5M;
+            _ = collection.Where(query).ToList();
+            _substitute.Received().Execute("FT.AGGREGATE", "person-idx", "@Salary:[70.5 70.5]", "WITHCURSOR", "COUNT", "10000");
+        }
+
+        [Theory]
+        [InlineData("en-DE")]
+        [InlineData("it-IT")]
+        [InlineData("es-ES")]
+        public void TestDecimalQueryTestingInvariantCultureCompliance(string lcid)
+        {
+            Helper.RunTestUnderDifferentCulture(lcid, _ => TestDecimalQuery());
+        }
     }
 }

test/Redis.OM.Unit.Tests/RediSearchTests/Person.cs

@@ -32,6 +32,9 @@ public partial class Person
         [Indexed(Sortable = true)]
         public double? Height { get; set; }
 
+        [Indexed(Sortable = true)]
+        public decimal? Salary { get; set; }
+
         [ListType]
         [Indexed]
         public string[] NickNames { get; set; }

test/Redis.OM.Unit.Tests/RediSearchTests/SearchTests.cs

@@ -119,7 +119,7 @@ public void TestFirstOrDefaultWithMixedLocals()
         }
 
         [Fact]
-        public void TestBasicQueryWithExactNumericMatch()
+        public void TestBasicQueryWithExactIntegerMatch()
         {
             _substitute.ClearSubstitute();
             _substitute.Execute(Arg.Any<string>(), Arg.Any<object[]>()).Returns(_mockReply);
@@ -135,6 +135,32 @@ public void TestBasicQueryWithExactNumericMatch()
                 "100");
         }
 
+        [Fact]
+        public void TestBasicQueryWithExactDecimalMatch()
+        {
+            _substitute.ClearSubstitute();
+            _substitute.Execute(Arg.Any<string>(), Arg.Any<object[]>()).Returns(_mockReply);
+            var y = 90.5M;
+            var collection = new RedisCollection<Person>(_substitute);
+            _ = collection.Where(x => x.Salary == y).ToList();
+            _substitute.Received().Execute(
+                "FT.SEARCH",
+                "person-idx",
+                "(@Salary:[90.5 90.5])",
+                "LIMIT",
+                "0",
+                "100");
+        }
+
+        [Theory]
+        [InlineData("en-DE")]
+        [InlineData("it-IT")]
+        [InlineData("es-ES")]
+        public void TestBasicQueryWithExactDecimalMatchTestingInvariantCultureCompliance(string lcid)
+        {
+            Helper.RunTestUnderDifferentCulture(lcid, _ => TestBasicQueryWithExactDecimalMatch());
+        }
+
         [Fact]
         public void TestBasicFirstOrDefaultQuery()
         {
@@ -1057,6 +1083,15 @@ public async Task TestUpdateJsonWithDouble()
             Scripts.ShaCollection.Clear();
         }
 
+        [Theory]
+        [InlineData("en-DE")]
+        [InlineData("it-IT")]
+        [InlineData("es-ES")]
+        public void TestUpdateJsonWithDoubleTestingInvariantCultureCompliance(string lcid)
+        {
+            Helper.RunTestUnderDifferentCulture(lcid, async _ => await TestUpdateJsonWithDouble());
+        }
+
         [Fact]
         public async Task TestDeleteAsync()
         {
@@ -3346,6 +3381,7 @@ public void NonNullableNumericFieldContains()
             var doubles = new [] { 22.5, 23, 24 };
             var floats = new [] { 25.5F, 26, 27 };
             var ushorts = new ushort[] { 28, 29, 30 };
+            var decimals = new decimal[] { 31.5M, 32, 33 };
             _substitute.ClearSubstitute();
             _substitute.Execute(Arg.Any<string>(), Arg.Any<object[]>()).Returns(_mockReply);
             var collection = new RedisCollection<ObjectWithNumerics>(_substitute).Where(x => ints.Contains(x.Integer));
@@ -3457,6 +3493,17 @@ public void NonNullableNumericFieldContains()
                 "LIMIT",
                 "0",
                 "100");
+
+            collection = new RedisCollection<ObjectWithNumerics>(_substitute).Where(x => decimals.Contains(x.Decimal));
+            _ = collection.ToList();
+            expected = $"@{nameof(ObjectWithNumerics.Decimal)}:[31.5 31.5]|@{nameof(ObjectWithNumerics.Decimal)}:[32 32]|@{nameof(ObjectWithNumerics.Decimal)}:[33 33]";
+            _substitute.Received().Execute(
+                "FT.SEARCH",
+                "objectwithnumerics-idx",
+                expected,
+                "LIMIT",
+                "0",
+                "100");
         }
 
         [Fact]

test/Redis.OM.Unit.Tests/Serialization/ObjectWithNumerics.cs

@@ -25,4 +25,6 @@ public class ObjectWithNumerics
     public double Double { get; set; }
     [Indexed]
     public float Float { get; set; }
+    [Indexed]
+    public decimal Decimal { get; set; }
 }