Skip to content

Commit d43d4b2

Browse files
axnetgaxnetg
authored
Bugfix for culture invariant decimal types (#423)
1 parent c58713a commit d43d4b2

File tree

9 files changed

+116
-22
lines changed

9 files changed

+116
-22
lines changed

‎README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,7 @@ We'd love your contributions! If you want to contribute please read our [Contrib
469469
* [@AndersenGans](https://github.com/AndersenGans)
470470
* [@mdrakib](https://github.com/mdrakib)
471471
* [@jrpavoncello](https://github.com/jrpavoncello)
472+
* [@axnetg](https://github.com/axnetg)
472473

473474
<!-- Logo -->
474475
[Logo]: images/logo.svg

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

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -57,32 +57,32 @@ protected override void ValidateAndPushOperand(Expression expression, Stack<stri
5757

5858
if (binaryExpression.Right is ConstantExpression constantExpression)
5959
{
60-
stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, constantExpression));
60+
var constVal = ExpressionParserUtilities.GetOperandString(constantExpression);
61+
stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, constVal));
6162
}
6263
else if (binaryExpression.Right is UnaryExpression uni)
6364
{
6465
switch (uni.Operand)
6566
{
6667
case ConstantExpression c:
67-
stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, c));
68+
var constVal = ExpressionParserUtilities.GetOperandString(c);
69+
stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, constVal));
6870
break;
6971
case MemberExpression mem:
70-
{
7172
var val = ExpressionParserUtilities.GetOperandString(mem);
72-
stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, System.Linq.Expressions.Expression.Constant(val)));
73+
stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, val));
7374
break;
74-
}
7575
}
7676
}
7777
else if (binaryExpression.Right is MemberExpression mem)
7878
{
7979
var val = ExpressionParserUtilities.GetOperandString(mem);
80-
stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, System.Linq.Expressions.Expression.Constant(val)));
80+
stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, val));
8181
}
8282
else
8383
{
8484
var val = ExpressionParserUtilities.GetOperandStringForQueryArgs(binaryExpression.Right, new List<object>()); // hack - will need to revisit when integrating vectors into aggregations.
85-
stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, System.Linq.Expressions.Expression.Constant(val)));
85+
stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, val));
8686
}
8787
}
8888
else if (expression is ConstantExpression c
@@ -169,7 +169,7 @@ protected override void SplitBinaryExpression(BinaryExpression expression, Stack
169169
}
170170
}
171171

172-
private static string BuildEqualityPredicate(MemberInfo member, ConstantExpression expression, string memberStr, bool negated = false)
172+
private static string BuildEqualityPredicate(MemberInfo member, string queryValue, string memberStr, bool negated = false)
173173
{
174174
var sb = new StringBuilder();
175175
var fieldAttribute = member.GetCustomAttribute<SearchFieldAttribute>();
@@ -190,13 +190,13 @@ private static string BuildEqualityPredicate(MemberInfo member, ConstantExpressi
190190
switch (searchFieldType)
191191
{
192192
case SearchFieldType.TAG:
193-
sb.Append($"{{{ExpressionParserUtilities.EscapeTagField(expression.Value.ToString())}}}");
193+
sb.Append($"{{{ExpressionParserUtilities.EscapeTagField(queryValue)}}}");
194194
break;
195195
case SearchFieldType.TEXT:
196-
sb.Append(expression.Value);
196+
sb.Append(queryValue);
197197
break;
198198
case SearchFieldType.NUMERIC:
199-
sb.Append($"[{expression.Value} {expression.Value}]");
199+
sb.Append($"[{queryValue} {queryValue}]");
200200
break;
201201
default:
202202
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
205205
return sb.ToString();
206206
}
207207

208-
private string BuildQueryPredicate(ExpressionType expType, MemberExpression member, ConstantExpression constExpression)
208+
private string BuildQueryPredicate(ExpressionType expType, MemberExpression member, string queryValue)
209209
{
210210
var memberStr = ExpressionParserUtilities.GetOperandString(member);
211211
var queryPredicate = expType switch
212212
{
213-
ExpressionType.GreaterThan => $"{memberStr}:[({constExpression.Value} inf]",
214-
ExpressionType.LessThan => $"{memberStr}:[-inf ({constExpression.Value}]",
215-
ExpressionType.GreaterThanOrEqual => $"{memberStr}:[{constExpression.Value} inf]",
216-
ExpressionType.LessThanOrEqual => $"{memberStr}:[-inf {constExpression.Value}]",
217-
ExpressionType.Equal => BuildEqualityPredicate(member.Member, constExpression, memberStr),
218-
ExpressionType.NotEqual => BuildEqualityPredicate(member.Member, constExpression, memberStr, true),
213+
ExpressionType.GreaterThan => $"{memberStr}:[({queryValue} inf]",
214+
ExpressionType.LessThan => $"{memberStr}:[-inf ({queryValue}]",
215+
ExpressionType.GreaterThanOrEqual => $"{memberStr}:[{queryValue} inf]",
216+
ExpressionType.LessThanOrEqual => $"{memberStr}:[-inf {queryValue}]",
217+
ExpressionType.Equal => BuildEqualityPredicate(member.Member, queryValue, memberStr),
218+
ExpressionType.NotEqual => BuildEqualityPredicate(member.Member, queryValue, memberStr, true),
219219
_ => string.Empty
220220
};
221221
return queryPredicate;

‎src/Redis.OM/Common/ExpressionParserUtilities.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ internal static string GetOperandStringForQueryArgs(Expression exp, List<object>
8383
{
8484
var res = exp switch
8585
{
86-
ConstantExpression constExp => $"{constExp.Value}",
86+
ConstantExpression constExp => ValueToString(constExp.Value),
8787
MemberExpression member => GetOperandStringForMember(member, treatEnumsAsInt),
8888
MethodCallExpression method => TranslateMethodStandardQuerySyntax(method, parameters),
8989
UnaryExpression unary => GetOperandStringForQueryArgs(unary.Operand, parameters, treatEnumsAsInt, unary.NodeType == ExpressionType.Not),
@@ -960,6 +960,16 @@ private static string ValueToString(object value)
960960
return ((double)value).ToString(CultureInfo.InvariantCulture);
961961
}
962962

963+
if (valueType == typeof(float) || Nullable.GetUnderlyingType(valueType) == typeof(float))
964+
{
965+
return ((float)value).ToString(CultureInfo.InvariantCulture);
966+
}
967+
968+
if (valueType == typeof(decimal) || Nullable.GetUnderlyingType(valueType) == typeof(decimal))
969+
{
970+
return ((decimal)value).ToString(CultureInfo.InvariantCulture);
971+
}
972+
963973
if (value is DateTimeOffset dto)
964974
{
965975
return dto.ToUnixTimeMilliseconds().ToString(CultureInfo.InvariantCulture);

‎src/Redis.OM/Modeling/JsonDiff.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
using System;
2-
using System.Linq;
2+
using System.Globalization;
33
using System.Text.Json;
44
using System.Web;
55
using Newtonsoft.Json.Linq;
@@ -37,6 +37,7 @@ public string[] SerializeScriptArgs()
3737
return _value.Type switch
3838
{
3939
JTokenType.String => new[] { _operation, _path, $"\"{HttpUtility.JavaScriptStringEncode(_value.ToString())}\"" },
40+
JTokenType.Float => new[] { _operation, _path, ((JValue)_value).ToString(CultureInfo.InvariantCulture) },
4041
JTokenType.Boolean => new[] { _operation, _path, _value.ToString().ToLower() },
4142
JTokenType.Date => SerializeAsDateTime(),
4243
_ => new[] { _operation, _path, _value.ToString() }

‎src/Redis.OM/RedisObjectHandler.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ internal static IDictionary<string, object> BuildHashSet(this object obj)
350350
var val = property.GetValue(obj);
351351
if (val != null)
352352
{
353-
hash.Add(propertyName, val.ToString());
353+
hash.Add(propertyName, Convert.ToString(val, CultureInfo.InvariantCulture));
354354
}
355355
}
356356
else if (type.IsEnum)

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,5 +587,35 @@ public void DateTimeQuery()
587587
_ = collection.Where(query).ToList();
588588
_substitute.Received().Execute("FT.AGGREGATE", "objectwithdatetime-idx", $"@TimestampOffset:[({dtMs} inf]", "WITHCURSOR", "COUNT", "10000");
589589
}
590+
591+
[Fact]
592+
public void TestDecimalQuery()
593+
{
594+
var collection = new RedisAggregationSet<Person>(_substitute, true, chunkSize: 10000);
595+
_substitute.Execute("FT.AGGREGATE", Arg.Any<object[]>()).Returns(MockedResult);
596+
_substitute.Execute("FT.CURSOR", Arg.Any<object[]>()).Returns(MockedResultCursorEnd);
597+
598+
var y = 30.55M;
599+
Expression<Func<AggregationResult<Person>, bool>> query = a => a.RecordShell!.Salary > y;
600+
_ = collection.Where(query).ToList();
601+
_substitute.Received().Execute("FT.AGGREGATE", "person-idx", "@Salary:[(30.55 inf]", "WITHCURSOR", "COUNT", "10000");
602+
603+
query = a => a.RecordShell!.Salary > 85.99M;
604+
_ = collection.Where(query).ToList();
605+
_substitute.Received().Execute("FT.AGGREGATE", "person-idx", "@Salary:[(85.99 inf]", "WITHCURSOR", "COUNT", "10000");
606+
607+
query = a => a.RecordShell!.Salary == 70.5M;
608+
_ = collection.Where(query).ToList();
609+
_substitute.Received().Execute("FT.AGGREGATE", "person-idx", "@Salary:[70.5 70.5]", "WITHCURSOR", "COUNT", "10000");
610+
}
611+
612+
[Theory]
613+
[InlineData("en-DE")]
614+
[InlineData("it-IT")]
615+
[InlineData("es-ES")]
616+
public void TestDecimalQueryTestingInvariantCultureCompliance(string lcid)
617+
{
618+
Helper.RunTestUnderDifferentCulture(lcid, _ => TestDecimalQuery());
619+
}
590620
}
591621
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ public partial class Person
3232
[Indexed(Sortable = true)]
3333
public double? Height { get; set; }
3434

35+
[Indexed(Sortable = true)]
36+
public decimal? Salary { get; set; }
37+
3538
[ListType]
3639
[Indexed]
3740
public string[] NickNames { get; set; }

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

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public void TestFirstOrDefaultWithMixedLocals()
119119
}
120120

121121
[Fact]
122-
public void TestBasicQueryWithExactNumericMatch()
122+
public void TestBasicQueryWithExactIntegerMatch()
123123
{
124124
_substitute.ClearSubstitute();
125125
_substitute.Execute(Arg.Any<string>(), Arg.Any<object[]>()).Returns(_mockReply);
@@ -135,6 +135,32 @@ public void TestBasicQueryWithExactNumericMatch()
135135
"100");
136136
}
137137

138+
[Fact]
139+
public void TestBasicQueryWithExactDecimalMatch()
140+
{
141+
_substitute.ClearSubstitute();
142+
_substitute.Execute(Arg.Any<string>(), Arg.Any<object[]>()).Returns(_mockReply);
143+
var y = 90.5M;
144+
var collection = new RedisCollection<Person>(_substitute);
145+
_ = collection.Where(x => x.Salary == y).ToList();
146+
_substitute.Received().Execute(
147+
"FT.SEARCH",
148+
"person-idx",
149+
"(@Salary:[90.5 90.5])",
150+
"LIMIT",
151+
"0",
152+
"100");
153+
}
154+
155+
[Theory]
156+
[InlineData("en-DE")]
157+
[InlineData("it-IT")]
158+
[InlineData("es-ES")]
159+
public void TestBasicQueryWithExactDecimalMatchTestingInvariantCultureCompliance(string lcid)
160+
{
161+
Helper.RunTestUnderDifferentCulture(lcid, _ => TestBasicQueryWithExactDecimalMatch());
162+
}
163+
138164
[Fact]
139165
public void TestBasicFirstOrDefaultQuery()
140166
{
@@ -1057,6 +1083,15 @@ public async Task TestUpdateJsonWithDouble()
10571083
Scripts.ShaCollection.Clear();
10581084
}
10591085

1086+
[Theory]
1087+
[InlineData("en-DE")]
1088+
[InlineData("it-IT")]
1089+
[InlineData("es-ES")]
1090+
public void TestUpdateJsonWithDoubleTestingInvariantCultureCompliance(string lcid)
1091+
{
1092+
Helper.RunTestUnderDifferentCulture(lcid, async _ => await TestUpdateJsonWithDouble());
1093+
}
1094+
10601095
[Fact]
10611096
public async Task TestDeleteAsync()
10621097
{
@@ -3346,6 +3381,7 @@ public void NonNullableNumericFieldContains()
33463381
var doubles = new [] { 22.5, 23, 24 };
33473382
var floats = new [] { 25.5F, 26, 27 };
33483383
var ushorts = new ushort[] { 28, 29, 30 };
3384+
var decimals = new decimal[] { 31.5M, 32, 33 };
33493385
_substitute.ClearSubstitute();
33503386
_substitute.Execute(Arg.Any<string>(), Arg.Any<object[]>()).Returns(_mockReply);
33513387
var collection = new RedisCollection<ObjectWithNumerics>(_substitute).Where(x => ints.Contains(x.Integer));
@@ -3457,6 +3493,17 @@ public void NonNullableNumericFieldContains()
34573493
"LIMIT",
34583494
"0",
34593495
"100");
3496+
3497+
collection = new RedisCollection<ObjectWithNumerics>(_substitute).Where(x => decimals.Contains(x.Decimal));
3498+
_ = collection.ToList();
3499+
expected = $"@{nameof(ObjectWithNumerics.Decimal)}:[31.5 31.5]|@{nameof(ObjectWithNumerics.Decimal)}:[32 32]|@{nameof(ObjectWithNumerics.Decimal)}:[33 33]";
3500+
_substitute.Received().Execute(
3501+
"FT.SEARCH",
3502+
"objectwithnumerics-idx",
3503+
expected,
3504+
"LIMIT",
3505+
"0",
3506+
"100");
34603507
}
34613508

34623509
[Fact]

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,6 @@ public class ObjectWithNumerics
2525
public double Double { get; set; }
2626
[Indexed]
2727
public float Float { get; set; }
28+
[Indexed]
29+
public decimal Decimal { get; set; }
2830
}

0 commit comments

Comments
 (0)