Check the scale before converting xcontent long values, rather than the absolute value #111538
Conversation
elasticsearchmachine
added
Team:Core/Infra
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
thecoop
added
v8.15.0
auto-backport
thecoop
force-pushed
the
long-scale-check
branch
from
01b3c94
to
d0e172b
Compare
💚 Backport successful
|
…he absolute value (elastic#111538) Large numbers are rejected, small numbers rounded to zero (if rounding enabled)
…he absolute value (elastic#111538) Large numbers are rejected, small numbers rounded to zero (if rounding enabled)
| // large scale -> very small number | ||
| if (bigDecimalValue.scale() > 19) { | ||
| if (coerce) { | ||
| bigIntegerValue = BigInteger.ZERO; |
There was a problem hiding this comment.
Note to self: a BigDecimal with a scale of 19 can have a value as large as 0.9. When such a number reaches toBigInteger it will round down to zero. Hence, this logic is correct.
| if (coerce) { | ||
| bigIntegerValue = BigInteger.ZERO; | ||
| } else { | ||
| throw new ArithmeticException("Number has a decimal part"); |
There was a problem hiding this comment.
Note to self: the other IllegalArgumentException reports the actual number, but in this case, the conversion to string would itself cause the problem we're trying to avoid, so we can't really offer any more hints for the user.
| builder.field("decimal", "5.5"); | ||
| builder.field("expInRange", "5e18"); | ||
| builder.field("expTooBig", "2e100"); | ||
| builder.field("expTooSmall", "2e-100"); |
There was a problem hiding this comment.
@thecoop - we should also test the negative versions of all these values.
There was a problem hiding this comment.
I've been playing with it, and 1e-2147483648 throws an error instead of going to zero. Is that what we want?
There was a problem hiding this comment.
I made #111641 in case that's helpful.
There was a problem hiding this comment.
Thats a limitation of BigDecimal, the scale is far too small. I don't think this is a case we need to worry about...
…he absolute value (#111538) Large numbers are rejected, small numbers rounded to zero (if rounding enabled)
…he absolute value (elastic#111538) Large numbers are rejected, small numbers rounded to zero (if rounding enabled)
Check the BigDecimal scale before trying to convert to a long - this is a faster check than converting to BigInteger and checking the resulting value