[ 
https://issues.apache.org/jira/browse/JEXL-366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17529782#comment-17529782
 ] 

Hussachai Puripunpinyo commented on JEXL-366:
---------------------------------------------

Got it :) . That's the reason why I use toBigDecimal which has the following 
logic that uses a configured math context.

 
{code:java}
return roundBigDecimal(new BigDecimal(string, getMathContext()));{code}
 

I'd say the logic I want to add is kinda equivalent to the existing one.

For example:  '1.0' == 1b

This expression will be evaluated by the first check because the right operand 
is BigDecimal

 
{code:java}
if (left instanceof BigDecimal || right instanceof BigDecimal) {
    final BigDecimal l = toBigDecimal(left);
    final BigDecimal r = toBigDecimal(right);
    return l.compareTo(r);
} {code}
As you can see, the right operand satisfies to the right check which means this 
'1.0' == 1b will be computed the same way as the logic I introduced.

 

The difference in my case is that the int will be widen to BigDecimal and the 
following code will be used to convert the numerable to BigDecimal.

It actually doesn't really do anything because numerable won't have any decimal 
value. I think that's the reason why the code for Character doesn't even care 
to apply the configured math context.
{code:java}
if (val instanceof Number) {
    return roundBigDecimal(new BigDecimal(val.toString(), getMathContext()));
} 

or

if (val instanceof Character) {
    final int i = ((Character) val);
    return new BigDecimal(i);
}{code}
 

Anyway, the caveat here is that the string "1.00001" can be equal to 1 when the 
math context is configured like
{code:java}
new MathContext(1, RoundingMode.FLOOR) {code}
But, this will fail the existing code as well.

The existing logic will say that this is true '1.00001' == 1b if the math 
context is configured as above.

So, I'd say the logic I want to add pretty conforms to the existing behavior 
except that it won't fail when you do '1.0' == 1.

> Fail to evaluate string and number comparison
> ---------------------------------------------
>
>                 Key: JEXL-366
>                 URL: https://issues.apache.org/jira/browse/JEXL-366
>             Project: Commons JEXL
>          Issue Type: Improvement
>    Affects Versions: 3.2.1
>            Reporter: Hussachai Puripunpinyo
>            Priority: Major
>
> The comparison logic doesn't cover the case when one operand is a string and 
> another operand is a numerable type (int, short, long,..).
> The expected result for '1.0' == 1 should be true but it fails because the 
> string comparison check is after the numerable type check. JEXL tries to 
> parse '1.0' using toLong function and it fails with this error message `For 
> input string: "1.0"`
> Moving a string comparison up before other number type checks will not cover 
> some corner cases such as
> '1.00' == 1.0 // String comparison will yield false but it obviously doesn't 
> make sense.
> The proposed change is to add the following code to handle the corner case 
> when one operand is string and another operand is numerable. To cover this 
> corner case, we can apply toBigDecimal to both *lhs* and *rhs* since it 
> should cover any arbitrary number in a string form, and it handles other 
> number types well.
> {code:java}
> if (isNumberable(left) || isNumberable(right)) {
>     if (left instanceof String || right instanceof String) {
>         final BigDecimal l = toBigDecimal(left);
>         final BigDecimal r = toBigDecimal(right);
>         return l.compareTo(r);
>     } else {
>         // this code block remains the same
>     }
>     return 0;
> } {code}
> JEXL syntax is very similar to ECMA script except a few small set that are 
> not the same. So, I think following the ECMA spec for this comparison check 
> makes sense.
> The following code is JavaScript and it can be used in the JEXL test to make 
> sure that the behavior of comparison are the same. 
> Note that '1.0' == 1 yields true
> {code:java}
> function assert(condition, source) {
>     if (!condition) {
>         throw `Assertion failed for ${source}`;
>     }
> }
>   // Basic compare
> let exprs = [
>   "1 == 1", true,
>   "1 != 1", false,
>   "1 != 2", true,
>   "1 > 2", false,
>   "1 >= 2", false,
>   "1 < 2", true,
>   "1 <= 2", true,
>   // Int <-> Float Coercion
>   "1.0 == 1", true,
>   "1 == 1.0", true,
>   "1.1 != 1", true,
>   "1.1 < 2", true,
>   // numbers and strings
>   "'1' == 1", true,
>   "'' == 0", true, // empty string is coerced to zero (ECMA compliance)
>   "1.0 >= '1'", true,
>   "1.0 > '1'", false
> ];for (e = 0; e < exprs.length; e += 2) {
>   let stext = exprs[e];
>   let expected = exprs[e + 1];
>   assert(eval(stext) == expected, stext);
>   
> } {code}
>  



--
This message was sent by Atlassian Jira
(v8.20.7#820007)

Reply via email to