Skip to content

[8.15] [ESQL] Fix parsing of large magnitude negative numbers #110758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

not-napoleon
Copy link
Member

Resolves #104323

This fixes and adds tests for the first of the two bullets in the linked issue. ExpressionBuilder#visitIntegerValue will attempt to parse a string as an integral value, and return a Literal of the appropriate type. The actual parsing happens in StringUtils#parseIntegral. That function has special handling for values that are larger than Long.MAX_VALUE where it attempts to turn them into unsigned longs, and if the number is still out of range, throw InvalidArgumentException. ExpressionBuilder catches that InvalidArgumentException and tries to parse a double instead. If, on the other hand, the value is smaller than Long.MIN_VALUE, StringUtils never enters the unsigned long path and just calls intValueExact, which throws ArithmeticException. This PR solves the issue by catching that ArithmeticException and rethrowing it as an InvalidArgumentException.

Resolves elastic#104323

This fixes and adds tests for the first of the two bullets in the linked
issue.  `ExpressionBuilder#visitIntegerValue` will attempt to parse a
string as an integral value, and return a Literal of the appropriate
type.  The actual parsing happens in `StringUtils#parseIntegral`.  That
function has special handling for values that are larger than
`Long.MAX_VALUE` where it attempts to turn them into unsigned longs, and
if the number is still out of range, throw `InvalidArgumentException`.
`ExpressionBuilder` catches that `InvalidArgumentException` and tries to
parse a `double` instead.  If, on the other hand, the value is smaller
than `Long.MIN_VALUE`, `StringUtils` never enters the unsigned long path
and just calls `intValueExact`, which throws `ArithmeticException`.
This PR solves the issue by catching that `ArithmeticException` and
rethrowing it as an `InvalidArgumentException`.
@not-napoleon not-napoleon added >bug backport auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Analytics/ES|QL AKA ESQL v8.15.0 labels Jul 11, 2024
@elasticsearchmachine elasticsearchmachine merged commit 0ee975b into elastic:8.15 Jul 11, 2024
15 checks passed
@not-napoleon not-napoleon deleted the backport-110665-to-8.15 branch July 11, 2024 14:53
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 v8.15.0 v8.15.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants