Support BigDecimals with SpEL [SPR-9164]

[spring-projects-issues]

Oliver Becker opened SPR-9164 and commented

When doing number arithmetic in SpEL the result type is apparently one of double, long or int.
This has the unwanted effect that for example float or BigDecimal values will be changed to int.

new java.math.BigDecimal("12.34")
evaluates to 12.34

-(new java.math.BigDecimal("12.34"))
evaluates to -12

see org.springframework.expression.spel.ast
OpPlus, OpMinus, OpMultiply, OpDivide

---

Affects: 3.1.1

Reference URL: #80

Attachments:

• SpELFloatLiteralTest.java (1.24 kB)

Issue Links:

• Expression language not compare BigDecimals with integers [SPR-8716] #13358 Expression language not compare BigDecimals with integers ("is duplicated by")
• SpEL: OpEQ should use equals() [SPR-9194] #13832 SpEL: OpEQ should use equals()
• Downgrade accidental use of Java 1.7 APIs [SPR-11319] #15943 Downgrade accidental use of Java 1.7 APIs
• Add SpEL support for float literals [SPR-9486] #14121 Add SpEL support for float literals
• SpEL's arithmetic operations should explicitly detect BigInteger/Short/Byte and fall back to double handling for unknown Number subtypes [SPR-9913] #14546 SpEL's arithmetic operations should explicitly detect BigInteger/Short/Byte and fall back to double handling for unknown Number subtypes

1 votes, 7 watchers

[spring-projects-issues]

Chris Beams commented

Added Andy Clement as watcher

@Andy, could you take a look at this? Any comments would be appreciated; please assign to yourself if you can spare the time to fix it directly.

Thanks!

[spring-projects-issues]

Oliver Becker commented

I think the algorithm should be something along

• if one of the types is double or float then use doubleValue
• else if one of the types is byte, short, int or long then use longValue
• else either add BigDecimal/BigInteger support or fallback to double

[spring-projects-issues]

Andy Clement commented

This does need doing. I feel like widening byte/short/int to long value if not necessary might be overkill but I don't feel strongly enough that it shouldn't be done. Supporting BigDecimal/BigInteger in those referenced Op* classes should be added. Although not hard, care would need taking to do it in a way such that the code doesn't get too bloated with instanceof checks. Also needs test coverage, ideally for all cases/combinations. Don't think we have to take the further step of handling custom Number subtypes, that could be looked at when the scenario comes up.

[spring-projects-issues]

Chris Beams commented

Andy, is this something you're willing to commit to for 3.2 M1 / M2? I'll assign to you if so.

[spring-projects-issues]

Stevo Slavić commented

#13358 is somewhat related issue.

[spring-projects-issues]

Giovanni Dall'Oglio Risso commented

Hi...

I'm the reporter for the #13358 issue (related to this).

We managed to solve this problem, adding BigDecimals; we also made a pull request: #80

You can take that pull request, or start by that code (we added quite test cases).

PS: maybe you can consider to add also the BigIntegers support...

[spring-projects-issues]

Chris Beams commented

I've asked Giovanni to touch up his pull request per the contributor guidelines. We can start with that support at least, which covers BigDecimal.

[spring-projects-issues]

Satyapal Reddy commented

I have two functions:

public class FunctionsClass {
public int abs(int value) {
return Math.abs(value);
}
public float abs(float value) {
return Math.abs(value);
}
}

and when I execute the test with following code:

FunctionsClass testObject = new FunctionsClass();
StandardEvaluationContext context = new StandardEvaluationContext();
org.springframework.expression.Expression expression =  parser.parseExpression("abs(-10.2f)");
Number result = expression.getValue(context, testObject, Number.class);

it fails with:

Caused by: org.springframework.expression.spel.SpelEvaluationException: EL1033E:(pos 0): Method call of 'abs' is ambiguous, supported type conversions allow multiple variants to match

[spring-projects-issues]

Satyapal Reddy commented

When I have two functions with different inputs int and float, eventhough I pass float, it is complaining about this being ambiguous

[spring-projects-issues]

Satyapal Reddy commented

test case

[spring-projects-issues]

Giovanni Dall'Oglio Risso commented

@Satyapal

I think that your problem (SpEL unable to find the correct method) should be described in a new issue.
This report deals with the problems with numeric operators, but you speak of indecisiveness in the resolution of the method to be applied.

Regarding your problem, it seems that (at first glance) SPEL tries to find a function with the required signature, if not, try to find one that might fit (using converters), finds two candidates and does not know which one to apply.

You might think to put in place a priority system depending on the type to convert.
For example, when I have a float, I try first to see if there is a function that accepts a double, and if it finds it then use that, otherwise I try to see if there is one that accept long, int, etc..

...but pay attention, imagine you have this situation:

public int someFunction (int a, double b);
public int someFunction (double a, int b);

Then you go and evaluate

"someFunction (1f, 1f)"

Which one of the two functions would you choose?

So I think that it is necessary to open a new issue, and discuss it carefully in there.

[spring-projects-issues]

Satyapal Reddy commented

@Giovanni

Thanks for the feedback. I will open a new bug. The possible fix that I tried locally (and that resolved my issue) involved introducing FloatLiteral.java and adding additional instanceof for Float.

Satyapal

[spring-projects-issues]

Andy Clement commented

I just reviewed the PR Giovanni again - made a couple of comments on it but in principal it is in reasonable shape. There seems to be some code duplication in the tests and the coding standards aren't quite right for the if blocks, but it works and all the existing expression tests are passing for me. Chris/Phil - would you expect the submitter to fix the code duplication and styling thing in the PR and resubmit or would you do that?

[spring-projects-issues]

Giovanni Dall'Oglio Risso commented

This weekend I'm out, if you can wait, the first days of the next week I'll rebase and clean everything...

Thanks

[spring-projects-issues]

Giovanni Dall'Oglio Risso commented

Hello.
Everything is now OK, plus some additions:

• From the original pull request to now, SpEL advanced [operator++, between function].
  • Now is everything covered (the new features), updated and tested.
• In addition, some polish (like Phil Webb commit e83bdda)

I rebased on master, hope is OK...
Thanks

[spring-projects-issues]

Oliver Becker commented

Big thanks to Giovanni and Andy for fixing this!