Skip to content

Conversation

ivancea
Copy link
Contributor

@ivancea ivancea commented Jan 17, 2025

Backports the following commits to 8.x:

…astic#119536)

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
@ivancea ivancea added :Analytics/ES|QL AKA ESQL >bug auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels Jan 17, 2025
Copy link
Contributor

Documentation preview:

@elasticsearchmachine elasticsearchmachine merged commit d1f9a0a into elastic:8.x Jan 17, 2025
15 checks passed
@ivancea ivancea deleted the backport/8.x/pr-119536 branch January 17, 2025 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.18.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants