Skip to content

Commit 3da7c18

Browse files
authored
Fix #5163: Return null for double overflow to Infinity in arithmetic (#5202)
* Fix #5163: Return null for double overflow to Infinity in arithmetic operations Signed-off-by: Peng Huo <penghuo@gmail.com> * Address review feedback for #5163: complete test coverage and use isFinite() - Add 6 missing unit tests: divide/modulus double, add/subtract/divide/modulus float - Add NaN tests for ExprValueUtils.fromObjectValue (Double.NaN, Float.NaN) - Add yamlRestTest reproducing the bug's PPL query with Double.MAX_VALUE - Use Double.isFinite()/Float.isFinite() instead of isInfinite()||isNaN() Signed-off-by: Peng Huo <penghuo@gmail.com> * Fix spotless formatting in ArithmeticFunctions and tests Signed-off-by: Peng Huo <penghuo@gmail.com> * Revert V2 ArithmeticFunctions changes; keep ExprValueUtils fix for Calcite path Signed-off-by: Peng Huo <penghuo@gmail.com> --------- Signed-off-by: Peng Huo <penghuo@gmail.com>
1 parent c00b8b7 commit 3da7c18

3 files changed

Lines changed: 74 additions & 2 deletions

File tree

core/src/main/java/org/opensearch/sql/data/model/ExprValueUtils.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public static ExprValue fromObjectValue(Object o) {
133133
} else if (o instanceof Boolean) {
134134
return booleanValue((Boolean) o);
135135
} else if (o instanceof Double d) {
136-
if (Double.isNaN(d)) {
136+
if (!Double.isFinite(d)) {
137137
return LITERAL_NULL;
138138
}
139139
return doubleValue(d);
@@ -144,7 +144,7 @@ public static ExprValue fromObjectValue(Object o) {
144144
} else if (o instanceof String) {
145145
return stringValue((String) o);
146146
} else if (o instanceof Float f) {
147-
if (Float.isNaN(f)) {
147+
if (!Float.isFinite(f)) {
148148
return LITERAL_NULL;
149149
}
150150
return floatValue(f);

core/src/test/java/org/opensearch/sql/data/model/ExprValueUtilsTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import static org.junit.jupiter.api.Assertions.assertEquals;
1010
import static org.junit.jupiter.api.Assertions.assertNotEquals;
1111
import static org.junit.jupiter.api.Assertions.assertThrows;
12+
import static org.junit.jupiter.api.Assertions.assertTrue;
1213
import static org.opensearch.sql.data.model.ExprValueUtils.integerValue;
1314
import static org.opensearch.sql.data.type.ExprCoreType.ARRAY;
1415
import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN;
@@ -243,6 +244,28 @@ public void constructDateAndTimeValue() {
243244
ExprValueUtils.fromObjectValue("2012-07-07 01:01:01", TIMESTAMP));
244245
}
245246

247+
@Test
248+
public void fromObjectValue_double_infinity_returns_null() {
249+
assertTrue(ExprValueUtils.fromObjectValue(Double.POSITIVE_INFINITY).isNull());
250+
assertTrue(ExprValueUtils.fromObjectValue(Double.NEGATIVE_INFINITY).isNull());
251+
}
252+
253+
@Test
254+
public void fromObjectValue_float_infinity_returns_null() {
255+
assertTrue(ExprValueUtils.fromObjectValue(Float.POSITIVE_INFINITY).isNull());
256+
assertTrue(ExprValueUtils.fromObjectValue(Float.NEGATIVE_INFINITY).isNull());
257+
}
258+
259+
@Test
260+
public void fromObjectValue_double_nan_returns_null() {
261+
assertTrue(ExprValueUtils.fromObjectValue(Double.NaN).isNull());
262+
}
263+
264+
@Test
265+
public void fromObjectValue_float_nan_returns_null() {
266+
assertTrue(ExprValueUtils.fromObjectValue(Float.NaN).isNull());
267+
}
268+
246269
@Test
247270
public void hashCodeTest() {
248271
assertEquals(new ExprByteValue(1).hashCode(), new ExprByteValue(1).hashCode());
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
setup:
2+
- do:
3+
query.settings:
4+
body:
5+
transient:
6+
plugins.calcite.enabled: true
7+
- do:
8+
indices.create:
9+
index: bounty-types
10+
body:
11+
mappings:
12+
properties:
13+
"int_field":
14+
type: integer
15+
"double_field":
16+
type: double
17+
- do:
18+
bulk:
19+
index: bounty-types
20+
refresh: true
21+
body:
22+
- '{"index": {"_id": "1"}}'
23+
- '{"int_field": 2147483647, "double_field": 1.7976931348623157E308}'
24+
25+
---
26+
teardown:
27+
- do:
28+
query.settings:
29+
body:
30+
transient:
31+
plugins.calcite.enabled: false
32+
33+
---
34+
"Double overflow to Infinity returns null instead of crashing":
35+
- skip:
36+
features:
37+
- headers
38+
- allowed_warnings
39+
- do:
40+
allowed_warnings:
41+
- 'Loading the fielddata on the _id field is deprecated and will be removed in future versions. If you require sorting or aggregating on this field you should also include the id in the body of your documents, and map this field as a keyword field that has [doc_values] enabled'
42+
headers:
43+
Content-Type: 'application/json'
44+
ppl:
45+
body:
46+
query: source=bounty-types | where int_field = 2147483647 | eval double_mul = double_field * 2 | fields double_field, double_mul
47+
48+
- match: { total: 1 }
49+
- match: { "datarows": [[1.7976931348623157E308, null]] }

0 commit comments

Comments
 (0)