Skip to content

Commit

Permalink
fix: culture invariant decimals for aggregation queries
Browse files Browse the repository at this point in the history
  • Loading branch information
@axnetg
axnetg committed Dec 4, 2023
1 parent 40e8a7f commit 4b97f18
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 18 deletions.
36 changes: 18 additions & 18 deletions src/Redis.OM/Aggregation/AggregationPredicates/QueryPredicate.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
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
Expand Down Expand Up @@ -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>();
Expand All @@ -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");
Expand All @@ -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;
Expand Down
30 changes: 30 additions & 0 deletions test/Redis.OM.Unit.Tests/RediSearchTests/AggregationSetTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string[]>()).Returns(MockedResult);
_substitute.Execute("FT.CURSOR", Arg.Any<string[]>()).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());
}
}
}

0 comments on commit 4b97f18

Please sign in to comment.