Skip to content

Commit e6ff84d

Browse files
not-napoleonalbertzaharovits
authored andcommitted
[ESQL] Fix parsing of large magnitude negative numbers (#110665)
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`.
1 parent 01b1770 commit e6ff84d

File tree

5 files changed

+34
-1
lines changed

5 files changed

+34
-1
lines changed

docs/changelog/110665.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 110665
2+
summary: "[ESQL] Fix parsing of large magnitude negative numbers"
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 104323

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/StringUtils.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,9 @@ public static Number parseIntegral(String string) throws InvalidArgumentExceptio
354354
}
355355
return bi;
356356
}
357+
if (bi.compareTo(BigInteger.valueOf(Long.MIN_VALUE)) < 0) {
358+
throw new InvalidArgumentException("Magnitude of negative number [{}] is too large", string);
359+
}
357360
// try to downsize to int if possible (since that's the most common type)
358361
if (bi.intValue() == bi.longValue()) { // ternary operator would always promote to Long
359362
return bi.intValueExact();

x-pack/plugin/esql/qa/testFixtures/src/main/resources/floats.csv-spec

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
11
// Floating point types-specific tests
2+
parseLargeMagnitudeValues
3+
required_capability: fix_parsing_large_negative_numbers
4+
row a = 92233720368547758090, b = -9223372036854775809;
5+
6+
a:double | b:double
7+
9.223372036854776E+19 | -9.223372036854776E+18
8+
;
29

310
inDouble
411
from employees | keep emp_no, height, height.float, height.half_float, height.scaled_float | where height in (2.03, 2.0299999713897705, 2.029296875, 2.0300000000000002) | sort emp_no;

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,13 @@ public enum Cap {
127127
/**
128128
* Fix for union-types when aggregating over an inline conversion with conversion function. Done in #110652.
129129
*/
130-
UNION_TYPES_INLINE_FIX;
130+
UNION_TYPES_INLINE_FIX,
131+
132+
/**
133+
* Fix a parsing issue where numbers below Long.MIN_VALUE threw an exception instead of parsing as doubles.
134+
* see <a href="https://github.com/elastic/elasticsearch/issues/104323"> Parsing large numbers is inconsistent #104323 </a>
135+
*/
136+
FIX_PARSING_LARGE_NEGATIVE_NUMBERS;
131137

132138
private final boolean snapshotOnly;
133139

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,17 @@ public void testRowCommandHugeInt() {
105105
);
106106
}
107107

108+
public void testRowCommandHugeNegativeInt() {
109+
assertEquals(
110+
new Row(EMPTY, List.of(new Alias(EMPTY, "c", literalDouble(-92233720368547758080d)))),
111+
statement("row c = -92233720368547758080")
112+
);
113+
assertEquals(
114+
new Row(EMPTY, List.of(new Alias(EMPTY, "c", literalDouble(-18446744073709551616d)))),
115+
statement("row c = -18446744073709551616")
116+
);
117+
}
118+
108119
public void testRowCommandDouble() {
109120
assertEquals(new Row(EMPTY, List.of(new Alias(EMPTY, "c", literalDouble(1.0)))), statement("row c = 1.0"));
110121
}

0 commit comments

Comments
 (0)