Skip to content

Commit de40f42

Browse files
GumpacGGumpacG
authored
Modified returning NaN to NULL (#225) (#1341)
Signed-off-by: Guian Gumpac <[email protected]>
1 parent 72547f4 commit de40f42

File tree

3 files changed

+123
-22
lines changed

3 files changed

+123
-22
lines changed

‎core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,8 @@ private static DefaultFunctionResolver floor() {
240240
*/
241241
private static DefaultFunctionResolver ln() {
242242
return baseMathFunction(BuiltinFunctionName.LN.getName(),
243-
v -> new ExprDoubleValue(Math.log(v.doubleValue())), DOUBLE);
243+
v -> v.doubleValue() <= 0 ? ExprNullValue.of() :
244+
new ExprDoubleValue(Math.log(v.doubleValue())), DOUBLE);
244245
}
245246

246247
/**
@@ -255,15 +256,17 @@ private static DefaultFunctionResolver log() {
255256
// build unary log(x), SHORT/INTEGER/LONG/FLOAT/DOUBLE -> DOUBLE
256257
for (ExprType type : ExprCoreType.numberTypes()) {
257258
builder.add(FunctionDSL.impl(FunctionDSL
258-
.nullMissingHandling(v -> new ExprDoubleValue(Math.log(v.doubleValue()))),
259+
.nullMissingHandling(v -> v.doubleValue() <= 0 ? ExprNullValue.of() :
260+
new ExprDoubleValue(Math.log(v.doubleValue()))),
259261
DOUBLE, type));
260262
}
261263

262264
// build binary function log(b, x)
263265
for (ExprType baseType : ExprCoreType.numberTypes()) {
264266
for (ExprType numberType : ExprCoreType.numberTypes()) {
265267
builder.add(FunctionDSL.impl(FunctionDSL
266-
.nullMissingHandling((b, x) -> new ExprDoubleValue(
268+
.nullMissingHandling((b, x) -> b.doubleValue() <= 0 || x.doubleValue() <= 0
269+
? ExprNullValue.of() : new ExprDoubleValue(
267270
Math.log(x.doubleValue()) / Math.log(b.doubleValue()))),
268271
DOUBLE, baseType, numberType));
269272
}
@@ -278,7 +281,8 @@ private static DefaultFunctionResolver log() {
278281
*/
279282
private static DefaultFunctionResolver log10() {
280283
return baseMathFunction(BuiltinFunctionName.LOG10.getName(),
281-
v -> new ExprDoubleValue(Math.log10(v.doubleValue())), DOUBLE);
284+
v -> v.doubleValue() <= 0 ? ExprNullValue.of() :
285+
new ExprDoubleValue(Math.log10(v.doubleValue())), DOUBLE);
282286
}
283287

284288
/**
@@ -287,7 +291,8 @@ private static DefaultFunctionResolver log10() {
287291
*/
288292
private static DefaultFunctionResolver log2() {
289293
return baseMathFunction(BuiltinFunctionName.LOG2.getName(),
290-
v -> new ExprDoubleValue(Math.log(v.doubleValue()) / Math.log(2)), DOUBLE);
294+
v -> v.doubleValue() <= 0 ? ExprNullValue.of() :
295+
new ExprDoubleValue(Math.log(v.doubleValue()) / Math.log(2)), DOUBLE);
291296
}
292297

293298
/**

‎core/src/test/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunctionTest.java‎

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ private static Stream<Arguments> testLogDoubleArguments() {
8181
return builder.add(Arguments.of(2D, 2D)).build();
8282
}
8383

84+
private static Stream<Arguments> testLogInvalidDoubleArguments() {
85+
return Stream.of(Arguments.of(0D, -2D),
86+
Arguments.of(0D, 2D),
87+
Arguments.of(2D, 0D));
88+
}
89+
8490
private static Stream<Arguments> trigonometricArguments() {
8591
Stream.Builder<Arguments> builder = Stream.builder();
8692
return builder
@@ -725,7 +731,7 @@ public void floor_missing_value() {
725731
* Test ln with integer value.
726732
*/
727733
@ParameterizedTest(name = "ln({0})")
728-
@ValueSource(ints = {2, -2})
734+
@ValueSource(ints = {2, 3})
729735
public void ln_int_value(Integer value) {
730736
FunctionExpression ln = DSL.ln(DSL.literal(value));
731737
assertThat(
@@ -738,7 +744,7 @@ public void ln_int_value(Integer value) {
738744
* Test ln with long value.
739745
*/
740746
@ParameterizedTest(name = "ln({0})")
741-
@ValueSource(longs = {2L, -2L})
747+
@ValueSource(longs = {2L, 3L})
742748
public void ln_long_value(Long value) {
743749
FunctionExpression ln = DSL.ln(DSL.literal(value));
744750
assertThat(
@@ -751,7 +757,7 @@ public void ln_long_value(Long value) {
751757
* Test ln with float value.
752758
*/
753759
@ParameterizedTest(name = "ln({0})")
754-
@ValueSource(floats = {2F, -2F})
760+
@ValueSource(floats = {2F, 3F})
755761
public void ln_float_value(Float value) {
756762
FunctionExpression ln = DSL.ln(DSL.literal(value));
757763
assertThat(
@@ -764,7 +770,7 @@ public void ln_float_value(Float value) {
764770
* Test ln with double value.
765771
*/
766772
@ParameterizedTest(name = "ln({0})")
767-
@ValueSource(doubles = {2D, -2D})
773+
@ValueSource(doubles = {2D, 3D})
768774
public void ln_double_value(Double value) {
769775
FunctionExpression ln = DSL.ln(DSL.literal(value));
770776
assertThat(
@@ -773,6 +779,17 @@ public void ln_double_value(Double value) {
773779
assertEquals(String.format("ln(%s)", value.toString()), ln.toString());
774780
}
775781

782+
/**
783+
* Test ln with invalid value.
784+
*/
785+
@ParameterizedTest(name = "ln({0})")
786+
@ValueSource(doubles = {0D, -3D})
787+
public void ln_invalid_value(Double value) {
788+
FunctionExpression ln = DSL.ln(DSL.literal(value));
789+
assertEquals(DOUBLE, ln.type());
790+
assertTrue(ln.valueOf(valueEnv()).isNull());
791+
}
792+
776793
/**
777794
* Test ln with null value.
778795
*/
@@ -853,6 +870,17 @@ public void log_double_value(Double v) {
853870
assertEquals(String.format("log(%s)", v.toString()), log.toString());
854871
}
855872

873+
/**
874+
* Test log with 1 invalid value.
875+
*/
876+
@ParameterizedTest(name = "log({0})")
877+
@ValueSource(doubles = {0D, -3D})
878+
public void log_invalid_value(Double value) {
879+
FunctionExpression log = DSL.log(DSL.literal(value));
880+
assertEquals(DOUBLE, log.type());
881+
assertTrue(log.valueOf(valueEnv()).isNull());
882+
}
883+
856884
/**
857885
* Test log with 1 null value argument.
858886
*/
@@ -931,6 +959,17 @@ public void log_two_double_value(Double v1, Double v2) {
931959
assertEquals(String.format("log(%s, %s)", v1.toString(), v2.toString()), log.toString());
932960
}
933961

962+
/**
963+
* Test log with 2 invalid double arguments.
964+
*/
965+
@ParameterizedTest(name = "log({0}, {2})")
966+
@MethodSource("testLogInvalidDoubleArguments")
967+
public void log_two_invalid_double_value(Double v1, Double v2) {
968+
FunctionExpression log = DSL.log(DSL.literal(v1), DSL.literal(v2));
969+
assertEquals(log.type(), DOUBLE);
970+
assertTrue(log.valueOf(valueEnv()).isNull());
971+
}
972+
934973
/**
935974
* Test log with 2 null value arguments.
936975
*/
@@ -1051,6 +1090,17 @@ public void log10_double_value(Double v) {
10511090
assertEquals(String.format("log10(%s)", v.toString()), log.toString());
10521091
}
10531092

1093+
/**
1094+
* Test log10 with 1 invalid double argument.
1095+
*/
1096+
@ParameterizedTest(name = "log10({0})")
1097+
@ValueSource(doubles = {0D, -3D})
1098+
public void log10_two_invalid_value(Double v) {
1099+
FunctionExpression log = DSL.log10(DSL.literal(v));
1100+
assertEquals(log.type(), DOUBLE);
1101+
assertTrue(log.valueOf(valueEnv()).isNull());
1102+
}
1103+
10541104
/**
10551105
* Test log10 with null value.
10561106
*/
@@ -1133,6 +1183,17 @@ public void log2_double_value(Double v) {
11331183
assertEquals(String.format("log2(%s)", v.toString()), log.toString());
11341184
}
11351185

1186+
/**
1187+
* Test log2 with an invalid double value.
1188+
*/
1189+
@ParameterizedTest(name = "log2({0})")
1190+
@ValueSource(doubles = {0D, -2D})
1191+
public void log2_invalid_double_value(Double v) {
1192+
FunctionExpression log = DSL.log2(DSL.literal(v));
1193+
assertEquals(log.type(), DOUBLE);
1194+
assertTrue(log.valueOf(valueEnv()).isNull());
1195+
}
1196+
11361197
/**
11371198
* Test log2 with null value.
11381199
*/

‎integ-test/src/test/java/org/opensearch/sql/sql/MathematicalFunctionIT.java‎

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -197,19 +197,6 @@ public void testAtan() throws IOException {
197197
verifyDataRows(result, rows(Math.atan2(2, 3)));
198198
}
199199

200-
protected JSONObject executeQuery(String query) throws IOException {
201-
Request request = new Request("POST", QUERY_API_ENDPOINT);
202-
request.setJsonEntity(String.format(Locale.ROOT, "{\n" + " \"query\": \"%s\"\n" + "}", query));
203-
204-
RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder();
205-
restOptionsBuilder.addHeader("Content-Type", "application/json");
206-
request.setOptions(restOptionsBuilder);
207-
208-
Response response = client().performRequest(request);
209-
return new JSONObject(getResponseBody(response));
210-
}
211-
212-
213200
@Test
214201
public void testCbrt() throws IOException {
215202
JSONObject result = executeQuery("select cbrt(8)");
@@ -224,4 +211,52 @@ public void testCbrt() throws IOException {
224211
verifySchema(result, schema("cbrt(-27)", "double"));
225212
verifyDataRows(result, rows(-3.0));
226213
}
214+
215+
@Test
216+
public void testLnReturnsNull() throws IOException {
217+
JSONObject result = executeQuery("select ln(0), ln(-2)");
218+
verifySchema(result,
219+
schema("ln(0)", "double"),
220+
schema("ln(-2)", "double"));
221+
verifyDataRows(result, rows(null, null));
222+
}
223+
224+
@Test
225+
public void testLogReturnsNull() throws IOException {
226+
JSONObject result = executeQuery("select log(0), log(-2)");
227+
verifySchema(result,
228+
schema("log(0)", "double"),
229+
schema("log(-2)", "double"));
230+
verifyDataRows(result, rows(null, null));
231+
}
232+
233+
@Test
234+
public void testLog10ReturnsNull() throws IOException {
235+
JSONObject result = executeQuery("select log10(0), log10(-2)");
236+
verifySchema(result,
237+
schema("log10(0)", "double"),
238+
schema("log10(-2)", "double"));
239+
verifyDataRows(result, rows(null, null));
240+
}
241+
242+
@Test
243+
public void testLog2ReturnsNull() throws IOException {
244+
JSONObject result = executeQuery("select log2(0), log2(-2)");
245+
verifySchema(result,
246+
schema("log2(0)", "double"),
247+
schema("log2(-2)", "double"));
248+
verifyDataRows(result, rows(null, null));
249+
}
250+
251+
protected JSONObject executeQuery(String query) throws IOException {
252+
Request request = new Request("POST", QUERY_API_ENDPOINT);
253+
request.setJsonEntity(String.format(Locale.ROOT, "{\n" + " \"query\": \"%s\"\n" + "}", query));
254+
255+
RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder();
256+
restOptionsBuilder.addHeader("Content-Type", "application/json");
257+
request.setOptions(restOptionsBuilder);
258+
259+
Response response = client().performRequest(request);
260+
return new JSONObject(getResponseBody(response));
261+
}
227262
}

0 commit comments

Comments
 (0)