ESQL: Fix ROUND() with unsigned longs throwing in some edge cases#119536
ESQL: Fix ROUND() with unsigned longs throwing in some edge cases#119536ivancea merged 18 commits intoelastic:mainfrom
Conversation
ivancea
added
>bug
Team:Analytics
|
Documentation preview: |
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
Hi @ivancea, I've created a changelog YAML for you. |
In a follow up change I think it'd be pretty reasonable to limit the inputs to |
...n/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/Round.java
Outdated
Show resolved
Hide resolved
I should have started there... I'm removing the warnExceptions and adding the condition instead, returning 0 |
|
Updated the PR by:
|
| if (decimals <= -20) { | ||
| // Unsigned long max value is 2^64 - 1, which has 20 digits | ||
| return longToUnsignedLong(0, false); | ||
| } |
💚 Backport successful
|
…19536) (#120381) There were different error cases with `ROUND(number, decimals)`: - Decimals accepted unsigned longs, but threw a 500 with a `can't process [unsigned_long -> long]` in the cast evaluator - Fixed by improving the `resolveType()` - If the number was a BigInteger unsigned long, there were 2 cases throwing an exception: 1. Negative decimals outside the range of integer: Error 2. Negative decimals insie the range of integer, but "big enough" for `BigInteger.TEN.pow(...)` to throw a `BigInteger would overflow supported range` 3. -19 decimals with big unsigned longs like `18446744073709551615` was throwing an `unsigned_long overflow` Also, when the number is a BigInteger and the decimals is a big negative (but not big enough to throw), it may be **very** slow. Taking _many_ seconds for a single computation (It tries to calculate a `10^(big number)`. I didn't do anything here, but I wonder if we should limit it. To solve most of the cases, a warnExceptions was added for the overflow case, and a guard clause to return 0 for <-19 decimals on unsigned longs. Another issue is that rounding to a number like 7 to -1 returns 0 instead of 10, which may be considered an error. But it's consistent, so I'm leaving it to another PR
5 participants
There were different error cases with
ROUND(number, decimals):can't process [unsigned_long -> long]in the cast evaluatorresolveType()BigInteger.TEN.pow(...)to throw aBigInteger would overflow supported range18446744073709551615was throwing anunsigned_long overflowAlso, when the number is a BigInteger and the decimals is a big negative (but not big enough to throw), it may be very slow. Taking many seconds for a single computation (It tries to calculate a
10^(big number). I didn't do anything here, but I wonder if we should limit it.To solve most of the cases, a warnExceptions was added for the overflow case, and a guard clause to return 0 for <-19 decimals on unsigned longs.
Another issue is that rounding to a number like 7 to -1 returns 0 instead of 10, which may be considered an error. But it's consistent, so I'm leaving it to another PR