ncalc / ncalc

NCalc is a fast and lightweight expression evaluator library for .NET, designed for flexibility and high performance. It supports a wide range of mathematical and logical operations.
https://ncalc.github.io/ncalc/
MIT License
610 stars 85 forks source link

Regression bug - OverflowException with double values #190

Closed Bykiev closed 5 months ago

Bykiev commented 5 months ago

I've found that in LogicalExpressionParser the result variable type is decimal, but I think it should be double and only when ExpressionOptions.DecimalAsDefault (UseDecimalsAsDefault) specified it should be converted to decimal.

https://github.com/ncalc/ncalc/blob/e6c8c5f47932810a9f2f00397a95d31d058b4722/src/NCalc/Parser/LogicalExpressionParser.cs#L88-L95

Because of this such expression cause overflow:

var expr = new Expression($"Floor({double.MaxValue.ToString(CultureInfo.InvariantCulture)})");
var res = expr.Evaluate();

Seems to be a regression bug since 4.0

gumbarros commented 5 months ago

Not related to Parlot, but still don't fix my bug at #188 but solves your bug:

    [Fact]
    public void Overflow_Issue_190()
    {
        const decimal minValue = decimal.MinValue;
        var expr = new Expression(minValue.ToString(CultureInfo.InvariantCulture), CultureInfo.InvariantCulture);
        Assert.Equal(minValue,expr.Evaluate());
    }
 var decimalNumber = Terms.Decimal().Then<LogicalExpression>(d=> new ValueExpression(d));
        var doubleNumber = Terms.Double().Then<LogicalExpression>(d=> new ValueExpression(d));

        // primary => NUMBER | "[" identifier "]" | DateTime | string | function | boolean | "(" expression ")";
        var primary = OneOf(
            number,
            decimalNumber,
            doubleNumber,
            booleanTrue,
            booleanFalse,
            dateTime,
            stringValue,
            function,
            groupExpression,
            identifierExpression);

Edit: My bug was caused by culture, not overflow, also fixed.